[PATCHv3 0/4] Be more careful when prunning

[PATCHv3 0/4] Be more careful when prunning

[Carlos Martín Nieto]

Hello,

The first patch is not that big a deal, but it's better if we're
freeing the refspecs, we might as well free all of them.

The second patch introduces expected failures for the features that
this series fixes.

The third patch changes prune_resf and get_stale_heads so the caller
has to decide which refspecs are the appropriate ones to use. For
example, running

    git fetch --prune origin refs/heads/master:refs/heads/master

doesn't remove the other branches anymore. For a more interesting (and
believable) example, let's take

    git fetch --prune origin refs/heads/b/*:refs/heads/b/*

because you want to prune the refs inside the b/ namespace
only. Currently git will delete all the refs that aren't under that
namespace. With the second patch applied, git won't remove any refs
outside the b/ namespace.

What is probably the most usual case is covered by the forth patch,
which pretends that a "refs/tags/*:refs/tags/*" refspec was given on
the command-line. That fixes the

    git fetch --prune --tags origin

case. The non-tag refs are kept now.

Cheers,
   cmn

Carlos Martín Nieto (4):
  fetch: free all the additional refspecs
  t5510: add tests for fetch --prune
  fetch: honor the user-provided refspecs when pruning refs
  fetch: treat --tags like refs/tags/*:refs/tags/* when pruning

 builtin/fetch.c  |   33 +++++++++++++++++++++++----
 builtin/remote.c |    3 +-
 remote.c         |   66 ++++++++++++++++++++++++++++++++++++++++++++++-------
 remote.h         |    2 +-
 t/t5510-fetch.sh |   50 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 138 insertions(+), 16 deletions(-)

-- 
1.7.5.2.354.g349bf

[PATCH 1/4] fetch: free all the additional refspecs

[Carlos Martín Nieto]

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7a4e41c..30b485e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -883,7 +883,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	atexit(unlock_pack);
 	refspec = parse_fetch_refspec(ref_nr, refs);
 	exit_code = do_fetch(transport, refspec, ref_nr);
-	free(refspec);
+	free_refspec(ref_nr, refspec);
 	transport_disconnect(transport);
 	transport = NULL;
 	return exit_code;
-- 
1.7.5.2.354.g349bf

[PATCH 2/4] t5510: add tests for fetch --prune

[Carlos Martín Nieto]

The failures will be fixed in later commits.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 t/t5510-fetch.sh |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 7e433b1..8b5e925 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -76,6 +76,56 @@ test_expect_success "fetch test for-merge" '
 	cut -f -2 .git/FETCH_HEAD >actual &&
 	test_cmp expected actual'
 
+test_expect_success 'fetch --prune on its own works as expected' '
+	cd "$D" &&
+	git clone . prune &&
+	cd prune &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune origin &&
+	test_must_fail git rev-parse origin/extrabranch
+'
+
+test_expect_failure 'fetch --prune with a branch name keeps branches' '
+	cd "$D" &&
+	git clone . prune-branch &&
+	cd prune-branch &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune origin master &&
+	git rev-parse origin/extrabranch
+'
+
+test_expect_failure 'fetch --prune with a namespace keeps other namespaces' '
+	cd "$D" &&
+	git clone . prune-namespace &&
+	cd prune-namespace &&
+
+	git fetch --prune origin refs/heads/a/*:refs/remotes/origin/a/* &&
+	git rev-parse origin/master
+'
+
+test_expect_failure 'fetch --prune --tags does not delete the remote-tracking branches' '
+	cd "$D" &&
+	git clone . prune-tags &&
+	cd prune-tags &&
+	git fetch origin refs/heads/master:refs/tags/sometag &&
+
+	git fetch --prune --tags origin &&
+	git rev-parse origin/master &&
+	test_must_fail git rev-parse somebranch
+'
+
+test_expect_failure 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
+	cd "$D" &&
+	git clone . prune-tags-branch &&
+	cd prune-tags-branch &&
+	git fetch origin refs/heads/master:refs/remotes/origin/extrabranch &&
+
+	git fetch --prune --tags origin master &&
+	git rev-parse origin/extrabranch
+'
+
 test_expect_success 'fetch tags when there is no tags' '
 
     cd "$D" &&
-- 
1.7.5.2.354.g349bf

[PATCH 3/4] fetch: honor the user-provided refspecs when pruning refs

[Carlos Martín Nieto]

If the user gave us refspecs on the command line, we should use those
when deciding whether to prune a ref instead of relying on the
refspecs in the config.

Previously, running

    git fetch --prune origin refs/heads/master:refs/remotes/origin/master

would delete every other ref under the origin namespace because we
were using the refspec to filter the available refs but using the
configured refspec to figure out if a ref had been deleted on the
remote. This is clearly the wrong thing to do.

Change prune_refs and get_stale_heads to simply accept a list of
references and a list of refspecs. The caller of either function needs
to decide what refspecs should be used to decide whether a ref is
stale.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c  |   12 ++++++---
 builtin/remote.c |    3 +-
 remote.c         |   66 ++++++++++++++++++++++++++++++++++++++++++++++-------
 remote.h         |    2 +-
 t/t5510-fetch.sh |    4 +-
 5 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 30b485e..041f79e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -505,10 +505,10 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
 	return ret;
 }
 
-static int prune_refs(struct transport *transport, struct ref *ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
 {
 	int result = 0;
-	struct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);
+	struct ref *ref, *stale_refs = get_stale_heads(ref_map, refs, ref_count);
 	const char *dangling_msg = dry_run
 		? _("   (%s will become dangling)\n")
 		: _("   (%s has become dangling)\n");
@@ -699,8 +699,12 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
-	if (prune)
-		prune_refs(transport, ref_map);
+	if (prune) {
+		if (ref_count)
+			prune_refs(refs, ref_count, ref_map);
+		else
+			prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);
+	}
 	free_refs(ref_map);
 
 	/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/builtin/remote.c b/builtin/remote.c
index f2a9c26..79d898b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -349,7 +349,8 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 		else
 			string_list_append(&states->tracked, abbrev_branch(ref->name));
 	}
-	stale_refs = get_stale_heads(states->remote, fetch_map);
+	stale_refs = get_stale_heads(fetch_map, states->remote->fetch,
+				     states->remote->fetch_refspec_nr);
 	for (ref = stale_refs; ref; ref = ref->next) {
 		struct string_list_item *item =
 			string_list_append(&states->stale, abbrev_branch(ref->name));
diff --git a/remote.c b/remote.c
index b8ecfa5..13c9153 100644
--- a/remote.c
+++ b/remote.c
@@ -1681,36 +1681,84 @@ struct ref *guess_remote_head(const struct ref *head,
 }
 
 struct stale_heads_info {
-	struct remote *remote;
 	struct string_list *ref_names;
 	struct ref **stale_refs_tail;
+	struct refspec *refs;
+	int ref_count;
 };
 
+/* Returns 0 on success, -1 if it couldn't find a match in the refspecs. */
+static int find_in_refs(struct refspec *refs, int ref_count, struct refspec *query)
+{
+	int i;
+	struct refspec *refspec;
+
+	for (i = 0; i < ref_count; ++i) {
+		refspec = &refs[i];
+
+		/* No dst means it can't be used for prunning. */
+		if (!refspec->dst)
+			continue;
+
+		/*
+		 * No '*' means that it must match exactly. If it does
+		 * have it, try to match it against the pattern. If
+		 * the refspec matches, store the ref name as it would
+		 * appear in the server in query->src.
+		 */
+		if (!strchr(refspec->dst, '*')) {
+			if (!strcmp(query->dst, refspec->dst)) {
+				query->src = xstrdup(refspec->src);
+				return 0;
+			}
+		} else if (match_name_with_pattern(refspec->dst, query->dst,
+						    refspec->src, &query->src)) {
+			return 0;
+		}
+	}
+
+	return -1;
+}
+
 static int get_stale_heads_cb(const char *refname,
 	const unsigned char *sha1, int flags, void *cb_data)
 {
 	struct stale_heads_info *info = cb_data;
 	struct refspec refspec;
+	int ret;
 	memset(&refspec, 0, sizeof(refspec));
 	refspec.dst = (char *)refname;
-	if (!remote_find_tracking(info->remote, &refspec)) {
-		if (!((flags & REF_ISSYMREF) ||
-		    string_list_has_string(info->ref_names, refspec.src))) {
-			struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
-			hashcpy(ref->new_sha1, sha1);
-		}
+
+	ret = find_in_refs(info->refs, info->ref_count, &refspec);
+
+	/* No matches */
+	if (ret)
+		return 0;
+
+	/*
+	 * If we did find a suitable refspec and it's not a symref and
+	 * it's not in the list of refs that currently exist in that
+	 * remote we consider it to be stale.
+	 */
+	if (!((flags & REF_ISSYMREF) ||
+	      string_list_has_string(info->ref_names, refspec.src))) {
+		struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
+		hashcpy(ref->new_sha1, sha1);
 	}
+
+	free(refspec.src);
 	return 0;
 }
 
-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
+struct ref *get_stale_heads(struct ref *fetch_map, struct refspec *refs, int ref_count)
 {
 	struct ref *ref, *stale_refs = NULL;
 	struct string_list ref_names = STRING_LIST_INIT_NODUP;
 	struct stale_heads_info info;
-	info.remote = remote;
 	info.ref_names = &ref_names;
 	info.stale_refs_tail = &stale_refs;
+	info.refs = refs;
+	info.ref_count = ref_count;
 	for (ref = fetch_map; ref; ref = ref->next)
 		string_list_append(&ref_names, ref->name);
 	sort_string_list(&ref_names);
diff --git a/remote.h b/remote.h
index 9a30a9d..5d70aff 100644
--- a/remote.h
+++ b/remote.h
@@ -164,6 +164,6 @@ struct ref *guess_remote_head(const struct ref *head,
 			      int all);
 
 /* Return refs which no longer exist on remote */
-struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map);
+struct ref *get_stale_heads(struct ref *fetch_map, struct refspec *refs, int ref_count);
 
 #endif
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 8b5e925..581049b 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -86,7 +86,7 @@ test_expect_success 'fetch --prune on its own works as expected' '
 	test_must_fail git rev-parse origin/extrabranch
 '
 
-test_expect_failure 'fetch --prune with a branch name keeps branches' '
+test_expect_success 'fetch --prune with a branch name keeps branches' '
 	cd "$D" &&
 	git clone . prune-branch &&
 	cd prune-branch &&
@@ -96,7 +96,7 @@ test_expect_failure 'fetch --prune with a branch name keeps branches' '
 	git rev-parse origin/extrabranch
 '
 
-test_expect_failure 'fetch --prune with a namespace keeps other namespaces' '
+test_expect_success 'fetch --prune with a namespace keeps other namespaces' '
 	cd "$D" &&
 	git clone . prune-namespace &&
 	cd prune-namespace &&
-- 
1.7.5.2.354.g349bf

Re: [PATCH 3/4] fetch: honor the user-provided refspecs when pruning refs

[Junio C Hamano]

Carlos Martín Nieto <cmn@elego.de> writes:

> -static int prune_refs(struct transport *transport, struct ref *ref_map)
> +static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
>  {
>  	int result = 0;
> -	struct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);
> +	struct ref *ref, *stale_refs = get_stale_heads(ref_map, refs, ref_count);

So in short, get_state_heads() used to take a ref_map and a remote. The
ref_map is what we actually observed from the remote after talking
ls-remote with it. It tried to see if any existing ref in our refspace may
have come from that remote by inspecting the fetch refspec associated with
that remote (and the ones that does not exist anymore are queued in the
stale ref list).

Now get_state_heads() takes a ref_map and <refs, ref_count> (you made the
patch unnecessarily harder to read by swapping the order of parameters).
The latter "pair" roughly corresponds to what the "remote" parameter used
to mean, but instead of using the refspec associated with that remote, we
would use the refspec used for this particular fetch to determine which
refs we have are stale.

> @@ -699,8 +699,12 @@ static int do_fetch(struct transport *transport,
>  		free_refs(ref_map);
>  		return 1;
>  	}
> -	if (prune)
> -		prune_refs(transport, ref_map);
> +	if (prune) {
> +		if (ref_count)
> +			prune_refs(refs, ref_count, ref_map);
> +		else
> +			prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);
> +	}

And this is consistent to my two paragraph commentary above.

> diff --git a/builtin/remote.c b/builtin/remote.c
> index f2a9c26..79d898b 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -349,7 +349,8 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
>  		else
>  			string_list_append(&states->tracked, abbrev_branch(ref->name));
>  	}
> -	stale_refs = get_stale_heads(states->remote, fetch_map);
> +	stale_refs = get_stale_heads(fetch_map, states->remote->fetch,
> +				     states->remote->fetch_refspec_nr);

So is this.

> diff --git a/remote.c b/remote.c
> index b8ecfa5..13c9153 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1681,36 +1681,84 @@ struct ref *guess_remote_head(const struct ref *head,
>  }
>  
>  struct stale_heads_info {
> -	struct remote *remote;
>  	struct string_list *ref_names;
>  	struct ref **stale_refs_tail;
> +	struct refspec *refs;
> +	int ref_count;
>  };
>  
> +/* Returns 0 on success, -1 if it couldn't find a match in the refspecs. */
> +static int find_in_refs(struct refspec *refs, int ref_count, struct refspec *query)
> +{
> +	int i;
> +	struct refspec *refspec;

This function replaces the role remote_find_tracking() used to play in the
old code and the difference in the behaviour (except the obvious lack of
"find_src/find_dst") feels gratuitous.

The original code in remote_find_tracking() uses "->pattern" to see if a
pattern match is necessary, but this scans the refspec for an asterisk,
assuring a breakage when the refspec language is updated to understand
other glob magic in the future. Why isn't refspec->pattern used here?

Can't these two functions share more logic?  It appears to me that by
enhancing the logic here a little bit, it may be possible to implement
remote_find_tracking() ed in terms of this function as a helper.

> +	for (i = 0; i < ref_count; ++i) {
> +		refspec = &refs[i];
> +
> +		/* No dst means it can't be used for prunning. */
> +		if (!refspec->dst)
> +			continue;
> +
> +		/*
> +		 * No '*' means that it must match exactly. If it does
> +		 * have it, try to match it against the pattern. If
> +		 * the refspec matches, store the ref name as it would
> +		 * appear in the server in query->src.
> +		 */
> +		if (!strchr(refspec->dst, '*')) {
> +			if (!strcmp(query->dst, refspec->dst)) {
> +				query->src = xstrdup(refspec->src);
> +				return 0;
> +			}
> +		} else if (match_name_with_pattern(refspec->dst, query->dst,
> +						    refspec->src, &query->src)) {
> +			return 0;
> +		}
> +	}
> +
> +	return -1;
> +}

Re: [PATCH 3/4] fetch: honor the user-provided refspecs when pruning refs

[Carlos Martín Nieto]

[-- Attachment #1: Type: text/plain, Size: 4652 bytes --]

On Wed, 2011-10-12 at 14:39 -0700, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
> 
> > -static int prune_refs(struct transport *transport, struct ref *ref_map)
> > +static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
> >  {
> >  	int result = 0;
> > -	struct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);
> > +	struct ref *ref, *stale_refs = get_stale_heads(ref_map, refs, ref_count);
> 
> So in short, get_state_heads() used to take a ref_map and a remote. The
> ref_map is what we actually observed from the remote after talking
> ls-remote with it. It tried to see if any existing ref in our refspace may
> have come from that remote by inspecting the fetch refspec associated with
> that remote (and the ones that does not exist anymore are queued in the
> stale ref list).
> 
> Now get_state_heads() takes a ref_map and <refs, ref_count> (you made the
> patch unnecessarily harder to read by swapping the order of parameters).
> The latter "pair" roughly corresponds to what the "remote" parameter used
> to mean, but instead of using the refspec associated with that remote, we
> would use the refspec used for this particular fetch to determine which
> refs we have are stale.

Right. The only reason that the remote was passed was in order to use
its refspec. The order reversal wasn't on purpose, I'll change that.

> 
> > @@ -699,8 +699,12 @@ static int do_fetch(struct transport *transport,
> >  		free_refs(ref_map);
> >  		return 1;
> >  	}
> > -	if (prune)
> > -		prune_refs(transport, ref_map);
> > +	if (prune) {
> > +		if (ref_count)
> > +			prune_refs(refs, ref_count, ref_map);
> > +		else
> > +			prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);
> > +	}
> 
> And this is consistent to my two paragraph commentary above.
> 
> > diff --git a/builtin/remote.c b/builtin/remote.c
> > index f2a9c26..79d898b 100644
> > --- a/builtin/remote.c
> > +++ b/builtin/remote.c
> > @@ -349,7 +349,8 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
> >  		else
> >  			string_list_append(&states->tracked, abbrev_branch(ref->name));
> >  	}
> > -	stale_refs = get_stale_heads(states->remote, fetch_map);
> > +	stale_refs = get_stale_heads(fetch_map, states->remote->fetch,
> > +				     states->remote->fetch_refspec_nr);
> 
> So is this.
> 
> > diff --git a/remote.c b/remote.c
> > index b8ecfa5..13c9153 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -1681,36 +1681,84 @@ struct ref *guess_remote_head(const struct ref *head,
> >  }
> >  
> >  struct stale_heads_info {
> > -	struct remote *remote;
> >  	struct string_list *ref_names;
> >  	struct ref **stale_refs_tail;
> > +	struct refspec *refs;
> > +	int ref_count;
> >  };
> >  
> > +/* Returns 0 on success, -1 if it couldn't find a match in the refspecs. */
> > +static int find_in_refs(struct refspec *refs, int ref_count, struct refspec *query)
> > +{
> > +	int i;
> > +	struct refspec *refspec;
> 
> This function replaces the role remote_find_tracking() used to play in the
> old code and the difference in the behaviour (except the obvious lack of
> "find_src/find_dst") feels gratuitous.

remote_find_tracking wants a remote, and that's what we don't have
anymore. The main reason was that it does "too much". The previous
versions had the callback doing more by itself, so I overlooked the
possibilities of remote_find_tracking when rewriting it. Looking at the
code again, it does look like what we want.

> 
> The original code in remote_find_tracking() uses "->pattern" to see if a
> pattern match is necessary, but this scans the refspec for an asterisk,
> assuring a breakage when the refspec language is updated to understand
> other glob magic in the future. Why isn't refspec->pattern used here?

Trees, forest etc. I noticed that a bit late. I have a patch on top of
this one that does use ->pattern, which I was going to ask you to squash
in, but it's moot now, as I need to rewrite the patch anyway.

> 
> Can't these two functions share more logic?  It appears to me that by
> enhancing the logic here a little bit, it may be possible to implement
> remote_find_tracking() ed in terms of this function as a helper.

Yes, remote_find_tracking should use a version of this function; or
probably better, its loop should become the next version of
find_in_refs, so remote_find_tracking is just a wrapper for when we want
to use the remote's fetch refspec.


I'll resend the series with these changes.

   cmn


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCH 4/4] fetch: treat --tags like refs/tags/*:refs/tags/* when pruning

[Carlos Martín Nieto]

If --tags is specified, add that refspec to the list given to
prune_refs so it knows to treat it as a filter on what refs to
should consider for prunning. This way

    git fetch --prune --tags origin

only prunes tags and doesn't delete the branch refs.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---
 builtin/fetch.c  |   23 +++++++++++++++++++++--
 t/t5510-fetch.sh |    4 ++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 041f79e..0f8170c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -700,10 +700,29 @@ static int do_fetch(struct transport *transport,
 		return 1;
 	}
 	if (prune) {
-		if (ref_count)
+		/* If --tags was specified, pretend the user gave us the canonical tags refspec */
+		if (tags == TAGS_SET) {
+			const char *tags_str = "refs/tags/*:refs/tags/*";
+			struct refspec *tags_refspec, *refspec;
+
+			/* Copy the refspec and add the tags to it */
+			refspec = xcalloc(ref_count + 1, sizeof(struct refspec));
+			tags_refspec = parse_fetch_refspec(1, &tags_str);
+			memcpy(refspec, refs, ref_count * sizeof(struct refspec));
+			memcpy(&refspec[ref_count], tags_refspec, sizeof(struct refspec));
+			ref_count++;
+
+			prune_refs(refspec, ref_count, ref_map);
+
+			ref_count--;
+			/* The rest of the strings belong to fetch_one */
+			free_refspec(1, tags_refspec);
+			free(refspec);
+		} else if (ref_count) {
 			prune_refs(refs, ref_count, ref_map);
-		else
+		} else {
 			prune_refs(transport->remote->fetch, transport->remote->fetch_refspec_nr, ref_map);
+		}
 	}
 	free_refs(ref_map);
 
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 581049b..e0af4c4 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -105,7 +105,7 @@ test_expect_success 'fetch --prune with a namespace keeps other namespaces' '
 	git rev-parse origin/master
 '
 
-test_expect_failure 'fetch --prune --tags does not delete the remote-tracking branches' '
+test_expect_success 'fetch --prune --tags does not delete the remote-tracking branches' '
 	cd "$D" &&
 	git clone . prune-tags &&
 	cd prune-tags &&
@@ -116,7 +116,7 @@ test_expect_failure 'fetch --prune --tags does not delete the remote-tracking br
 	test_must_fail git rev-parse somebranch
 '
 
-test_expect_failure 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
+test_expect_success 'fetch --prune --tags with branch does not delete other remote-tracking branches' '
 	cd "$D" &&
 	git clone . prune-tags-branch &&
 	cd prune-tags-branch &&
-- 
1.7.5.2.354.g349bf