threads / patch / 35599

v3, 2 partsfetch --prune: Run prune before fetching

Subject: [PATCH V3 2/2] fetch --prune: Run prune before fetching

## tl;dr

3 messages between Jan 3, 2014 and Jan 3, 2014. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Tom Miller· Jan 3, 2014, 02:28 UTC · lore

[PATCH V3 1/2] fetch --prune: Always print header url

If "fetch --prune" is run with no new refs to fetch, but it has refs to prune. Then, the header url is not printed as it would if there were new refs to fetch.

Output before this patch:
	$ git fetch --prune remote-with-no-new-refs
	 x [deleted]         (none)     -> origin/world
Output after this patch:
	$ git fetch --prune remote-with-no-new-refs
	From https://github.com/git/git
	 x [deleted]         (none)     -> origin/test
Signed-off-by: Tom Miller <jackerran@gmail.com>
---

I decided it is not worth writing a function to format the header url that is printed by fetch. Instead, I will duplicate the formatting logic for now because I plan on following up this patch set with another patch to stop striping the trailing "/" and ".git" from the url. When that patch is finished it will remove the majority of the duplicated logic.

 builtin/fetch.c  | 32 +++++++++++++++++++++++++++-----
 t/t5510-fetch.sh | 12 ++++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)
Show changes to 2 files +39 −5

builtin/fetch.c, t/t5510-fetch.sh

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1e7d617..1b81cf9 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -44,6 +44,7 @@ static struct transport *gtransport;
 static struct transport *gsecondary;
 static const char *submodule_prefix = "";
 static const char *recurse_submodules_default;
+static int shown_url = 0;
 
 static int option_parse_recurse_submodules(const struct option *opt,
 				   const char *arg, int unset)
@@ -535,7 +536,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 {
 	FILE *fp;
 	struct commit *commit;
-	int url_len, i, shown_url = 0, rc = 0;
+	int url_len, i, rc = 0;
 	struct strbuf note = STRBUF_INIT;
 	const char *what, *kind;
 	struct ref *rm;
@@ -708,17 +709,36 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
 	return ret;
 }
 
-static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
+		const char *raw_url)
 {
-	int result = 0;
+	int url_len, i, result = 0;
 	struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);
+	char *url;
 	const char *dangling_msg = dry_run
 		? _("   (%s will become dangling)")
 		: _("   (%s has become dangling)");
 
+	if (raw_url)
+		url = transport_anonymize_url(raw_url);
+	else
+		url = xstrdup("foreign");
+
+	url_len = strlen(url);
+	for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
+		;
+
+	url_len = i + 1;
+	if (4 < i && !strncmp(".git", url + i - 3, 4))
+		url_len = i - 3;
+
 	for (ref = stale_refs; ref; ref = ref->next) {
 		if (!dry_run)
 			result |= delete_ref(ref->name, NULL, 0);
+		if (verbosity >= 0 && !shown_url) {
+			fprintf(stderr, _("From %.*s\n"), url_len, url);
+			shown_url = 1;
+		}
 		if (verbosity >= 0) {
 			fprintf(stderr, " x %-*s %-*s -> %s\n",
 				TRANSPORT_SUMMARY(_("[deleted]")),
@@ -726,6 +746,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
 			warn_dangling_symref(stderr, dangling_msg, ref->name);
 		}
 	}
+	free(url);
 	free_refs(stale_refs);
 	return result;
 }
@@ -854,11 +875,12 @@ static int do_fetch(struct transport *transport,
 		 * don't care whether --tags was specified.
 		 */
 		if (ref_count) {
-			prune_refs(refs, ref_count, ref_map);
+			prune_refs(refs, ref_count, ref_map, transport->url);
 		} else {
 			prune_refs(transport->remote->fetch,
 				   transport->remote->fetch_refspec_nr,
-				   ref_map);
+				   ref_map,
+				   transport->url);
 		}
 	}
 	free_refs(ref_map);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..87e896d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,16 @@ test_expect_success 'all boundary commits are excluded' '
 	test_bundle_object_count .git/objects/pack/pack-${pack##pack	}.pack 3
 '
 
+test_expect_success 'fetch --prune prints the remotes url' '
+	git branch goodbye &&
+	git clone . only-prunes &&
+	git branch -D goodbye &&
+	(
+		cd only-prunes &&
+		git fetch --prune origin 2>&1 | head -n1 >../actual
+	) &&
+	echo "From ${D}/." >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.5.2.231.g4834e63.dirty
Tom Miller· Jan 3, 2014, 02:28 UTC · re: Tom Miller · lore

When we have a remote-tracking branch named "frotz/nitfol" from a previous fetch, and the upstream now has a branch named "frotz". Prior to this patch fetch would fail to remove "frotz/nitfol" with a "git fetch --prune" from the upstream. git would inform the user to use "git remote prune" to fix the problem.

This patch changes the way "fetch --prune" works by moving the pruning operation before the fetching operation. Instead of warning the user of a conflict, it autmatically fixes it.

Signed-off-by: Tom Miller <jackerran@gmail.com>
Tested-by: Thomas Rast <tr@thomasrast.ch>
Acked-by: Junio C Hamano <gitster@pobox.com>
---

I did change the commit message according to Junio's suggestion in the first patch.

 builtin/fetch.c  | 10 +++++-----
 t/t5510-fetch.sh | 14 ++++++++++++++
 2 files changed, 19 insertions(+), 5 deletions(-)
Show changes to 2 files +19 −5

builtin/fetch.c, t/t5510-fetch.sh

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1b81cf9..09825c8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -863,11 +863,6 @@ static int do_fetch(struct transport *transport,
 
 	if (tags == TAGS_DEFAULT && autotags)
 		transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
-	if (fetch_refs(transport, ref_map)) {
-		free_refs(ref_map);
-		retcode = 1;
-		goto cleanup;
-	}
 	if (prune) {
 		/*
 		 * We only prune based on refspecs specified
@@ -883,6 +878,11 @@ static int do_fetch(struct transport *transport,
 				   transport->url);
 		}
 	}
+	if (fetch_refs(transport, ref_map)) {
+		free_refs(ref_map);
+		retcode = 1;
+		goto cleanup;
+	}
 	free_refs(ref_map);
 
 	/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 87e896d..12674ac 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -626,4 +626,18 @@ test_expect_success 'fetch --prune prints the remotes url' '
 	test_cmp expect actual
 '
 
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+	git branch dir/file &&
+	git clone . prune-df-conflict &&
+	git branch -D dir/file &&
+	git branch dir &&
+	(
+		cd prune-df-conflict &&
+		git fetch --prune &&
+		git rev-parse origin/dir >../actual
+	) &&
+	git rev-parse dir >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.5.2.231.g4834e63.dirty
Junio C Hamano· Jan 3, 2014, 18:17 UTC · re: Tom Miller · lore

Re: [PATCH V3 1/2] fetch --prune: Always print header url

Tom Miller <jackerran@gmail.com> writes:
Show 16 quoted lines
> If "fetch --prune" is run with no new refs to fetch, but it has refs
> to prune. Then, the header url is not printed as it would if there were
> new refs to fetch.
>
> Output before this patch:
>
> 	$ git fetch --prune remote-with-no-new-refs
> 	 x [deleted]         (none)     -> origin/world
>
> Output after this patch:
>
> 	$ git fetch --prune remote-with-no-new-refs
> 	From https://github.com/git/git
> 	 x [deleted]         (none)     -> origin/test
>
> Signed-off-by: Tom Miller <jackerran@gmail.com>

And (needless to say) when there are refs to be fetched and also pruned, the "shown_url" logic prevents us from giving two identical headers "From <there>". Sounds sane.

Show 7 quoted lines
> ---
>
> I decided it is not worth writing a function to format the header url
> that is printed by fetch. Instead, I will duplicate the formatting logic
> for now because I plan on following up this patch set with another patch
> to stop striping the trailing "/" and ".git" from the url. When that
> patch is finished it will remove the majority of the duplicated logic.
OK, let's see how it goes.

Will replace what has been queued on 'pu'. The incremental change relative to the old one looked sensible and I think this one is ready for 'next'.

Thanks.
Show 108 quoted lines
>  builtin/fetch.c  | 32 +++++++++++++++++++++++++++-----
>  t/t5510-fetch.sh | 12 ++++++++++++
>  2 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 1e7d617..1b81cf9 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -44,6 +44,7 @@ static struct transport *gtransport;
>  static struct transport *gsecondary;
>  static const char *submodule_prefix = "";
>  static const char *recurse_submodules_default;
> +static int shown_url = 0;
>  
>  static int option_parse_recurse_submodules(const struct option *opt,
>  				   const char *arg, int unset)
> @@ -535,7 +536,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>  {
>  	FILE *fp;
>  	struct commit *commit;
> -	int url_len, i, shown_url = 0, rc = 0;
> +	int url_len, i, rc = 0;
>  	struct strbuf note = STRBUF_INIT;
>  	const char *what, *kind;
>  	struct ref *rm;
> @@ -708,17 +709,36 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
>  	return ret;
>  }
>  
> -static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
> +static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
> +		const char *raw_url)
>  {
> -	int result = 0;
> +	int url_len, i, result = 0;
>  	struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);
> +	char *url;
>  	const char *dangling_msg = dry_run
>  		? _("   (%s will become dangling)")
>  		: _("   (%s has become dangling)");
>  
> +	if (raw_url)
> +		url = transport_anonymize_url(raw_url);
> +	else
> +		url = xstrdup("foreign");
> +
> +	url_len = strlen(url);
> +	for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
> +		;
> +
> +	url_len = i + 1;
> +	if (4 < i && !strncmp(".git", url + i - 3, 4))
> +		url_len = i - 3;
> +
>  	for (ref = stale_refs; ref; ref = ref->next) {
>  		if (!dry_run)
>  			result |= delete_ref(ref->name, NULL, 0);
> +		if (verbosity >= 0 && !shown_url) {
> +			fprintf(stderr, _("From %.*s\n"), url_len, url);
> +			shown_url = 1;
> +		}
>  		if (verbosity >= 0) {
>  			fprintf(stderr, " x %-*s %-*s -> %s\n",
>  				TRANSPORT_SUMMARY(_("[deleted]")),
> @@ -726,6 +746,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
>  			warn_dangling_symref(stderr, dangling_msg, ref->name);
>  		}
>  	}
> +	free(url);
>  	free_refs(stale_refs);
>  	return result;
>  }
> @@ -854,11 +875,12 @@ static int do_fetch(struct transport *transport,
>  		 * don't care whether --tags was specified.
>  		 */
>  		if (ref_count) {
> -			prune_refs(refs, ref_count, ref_map);
> +			prune_refs(refs, ref_count, ref_map, transport->url);
>  		} else {
>  			prune_refs(transport->remote->fetch,
>  				   transport->remote->fetch_refspec_nr,
> -				   ref_map);
> +				   ref_map,
> +				   transport->url);
>  		}
>  	}
>  	free_refs(ref_map);
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index 5d4581d..87e896d 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -614,4 +614,16 @@ test_expect_success 'all boundary commits are excluded' '
>  	test_bundle_object_count .git/objects/pack/pack-${pack##pack	}.pack 3
>  '
>  
> +test_expect_success 'fetch --prune prints the remotes url' '
> +	git branch goodbye &&
> +	git clone . only-prunes &&
> +	git branch -D goodbye &&
> +	(
> +		cd only-prunes &&
> +		git fetch --prune origin 2>&1 | head -n1 >../actual
> +	) &&
> +	echo "From ${D}/." >expect &&
> +	test_cmp expect actual
> +'
> +
>  test_done

← back to recent threads