[PATCH 0/4] Teach fetch --prune and --dry run options

DORMANTno replies

Revision v1 of 2 in this series.

5 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH 0/4] Teach fetch --prune and --dry run options

From: Jay Soffian <hidden>
Date: 2016-06-15 22:47:41

(Darn it, I left the SOB off. This is a resend of the series with SOB. Sorry
for the noise...)

This series builds on bg/fetch-multi, by teaching fetch --prune, re-enabling
"remote update --prune" and ensuring its corresponding test now passes.

As a convenience to users used to "remote prune" supporting the "--dry-run"
option, we also teach this option to fetch since it's trivial to do so.

I've left out re-implementing "remote prune" as a synonym for "update --prune"
since it's not a 1:1 match. And as per Junio's comments, teaching fetch a 
"--prune-only" option would be a UI wart.

Jay Soffian (4):
  remote: refactor some logic into get_stale_heads()
  teach warn_dangling_symref to take a FILE argument
  builtin-fetch: add --prune option
  builtin-fetch: add --dry-run option

 Documentation/fetch-options.txt |    9 ++++++++
 builtin-fetch.c                 |   42 +++++++++++++++++++++++++++++++++++---
 builtin-remote.c                |   38 +++++++++++------------------------
 refs.c                          |    7 +++--
 refs.h                          |    2 +-
 remote.c                        |   40 +++++++++++++++++++++++++++++++++++++
 remote.h                        |    3 ++
 t/t5505-remote.sh               |    2 +-
 8 files changed, 108 insertions(+), 35 deletions(-)

[PATCH 1/4] remote: refactor some logic into get_stale_heads()

From: Jay Soffian <hidden>
Date: 2016-06-15 22:47:41

Move the logic in builtin-remote.c which determines which local heads are stale
to remote.c so it can be used by other builtins.

Signed-off-by: Jay Soffian <redacted>
---
 builtin-remote.c |   32 ++++++++------------------------
 remote.c         |   40 ++++++++++++++++++++++++++++++++++++++++
 remote.h         |    3 +++
 3 files changed, 51 insertions(+), 24 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index fd7e0b2..0ea4e60 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c

@@ -227,32 +227,10 @@ struct ref_states {
 	int queried;
 };
 
-static int handle_one_branch(const char *refname,
-	const unsigned char *sha1, int flags, void *cb_data)
-{
-	struct ref_states *states = cb_data;
-	struct refspec refspec;
-
-	memset(&refspec, 0, sizeof(refspec));
-	refspec.dst = (char *)refname;
-	if (!remote_find_tracking(states->remote, &refspec)) {
-		struct string_list_item *item;
-		const char *name = abbrev_branch(refspec.src);
-		/* symbolic refs pointing nowhere were handled already */
-		if ((flags & REF_ISSYMREF) ||
-		    string_list_has_string(&states->tracked, name) ||
-		    string_list_has_string(&states->new, name))
-			return 0;
-		item = string_list_append(name, &states->stale);
-		item->util = xstrdup(refname);
-	}
-	return 0;
-}
-
 static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
 {
 	struct ref *fetch_map = NULL, **tail = &fetch_map;
-	struct ref *ref;
+	struct ref *ref, *stale_refs;
 	int i;
 
 	for (i = 0; i < states->remote->fetch_refspec_nr; i++)

@@ -268,11 +246,17 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 		else
 			string_list_append(abbrev_branch(ref->name), &states->tracked);
 	}
+	stale_refs = get_stale_heads(states->remote, fetch_map);
+	for (ref = stale_refs; ref; ref = ref->next) {
+		struct string_list_item *item =
+			string_list_append(abbrev_branch(ref->name), &states->stale);
+		item->util = xstrdup(ref->name);
+	}
+	free_refs(stale_refs);
 	free_refs(fetch_map);
 
 	sort_string_list(&states->new);
 	sort_string_list(&states->tracked);
-	for_each_ref(handle_one_branch, states);
 	sort_string_list(&states->stale);
 
 	return 0;

diff --git a/remote.c b/remote.c
index beaf9fb..eae5866 100644
--- a/remote.c
+++ b/remote.c

@@ -6,6 +6,7 @@
 #include "revision.h"
 #include "dir.h"
 #include "tag.h"
+#include "string-list.h"
 
 static struct refspec s_tag_refspec = {
 	0,

@@ -1587,3 +1588,42 @@ struct ref *guess_remote_head(const struct ref *head,
 
 	return list;
 }
+
+struct stale_heads_info {
+	struct remote *remote;
+	struct string_list *ref_names;
+	struct ref **stale_refs_tail;
+};
+
+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;
+	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);
+		}
+	}
+	return 0;
+}
+
+struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
+{
+	struct ref *ref, *stale_refs = NULL;
+	struct string_list ref_names = { NULL, 0, 0, 0 };
+	struct stale_heads_info info;
+	info.remote = remote;
+	info.ref_names = &ref_names;
+	info.stale_refs_tail = &stale_refs;
+	for (ref = fetch_map; ref; ref = ref->next)
+		string_list_append(ref->name, &ref_names);
+	sort_string_list(&ref_names);
+	for_each_ref(get_stale_heads_cb, &info);
+	string_list_clear(&ref_names, 0);
+	return stale_refs;
+}

diff --git a/remote.h b/remote.h
index 5db8420..d0aba81 100644
--- a/remote.h
+++ b/remote.h

@@ -154,4 +154,7 @@ struct ref *guess_remote_head(const struct ref *head,
 			      const struct ref *refs,
 			      int all);
 
+/* Return refs which no longer exist on remote */
+struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map);
+
 #endif

-- 
1.6.4.2

[PATCH 4/4] builtin-fetch: add --dry-run option

From: Jay Soffian <hidden>
Date: 2016-06-15 22:47:41

Teach fetch --dry-run as users of "git remote prune" switching to "git fetch
--prune" may expect it. Unfortunately OPT__DRY_RUN() cannot be used as fetch
already uses "-n" for something else.

Signed-off-by: Jay Soffian <redacted>
---
 Documentation/fetch-options.txt |    5 +++++
 builtin-fetch.c                 |   14 ++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 500637a..ab6419f 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt

@@ -12,6 +12,11 @@
 	`git clone` with `--depth=<depth>` option (see linkgit:git-clone[1])
 	by the specified number of commits.
 
+ifndef::git-pull[]
+--dry-run::
+	Show what would be done, without making any changes.
+endif::git-pull[]
+
 -f::
 --force::
 	When 'git-fetch' is used with `<rbranch>:<lbranch>`

diff --git a/builtin-fetch.c b/builtin-fetch.c
index fd31072..8082d36 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c

@@ -26,7 +26,7 @@ enum {
 	TAGS_SET = 2
 };
 
-static int all, append, force, keep, multiple, prune, update_head_ok, verbosity;
+static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;

@@ -51,6 +51,8 @@ static struct option builtin_fetch_options[] = {
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
 	OPT_BOOLEAN('p', "prune", &prune,
 		    "prune tracking branches no longer on remote"),
+	OPT_BOOLEAN(0, "dry-run", &dry_run,
+		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
 	OPT_BOOLEAN('u', "update-head-ok", &update_head_ok,
 		    "allow updating of HEAD ref"),

@@ -187,6 +189,8 @@ static int s_update_ref(const char *action,
 	char *rla = getenv("GIT_REFLOG_ACTION");
 	static struct ref_lock *lock;
 
+	if (dry_run)
+		return 0;
 	if (!rla)
 		rla = default_rla.buf;
 	snprintf(msg, sizeof(msg), "%s: %s", rla, action);

@@ -312,7 +316,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	char note[1024];
 	const char *what, *kind;
 	struct ref *rm;
-	char *url, *filename = git_path("FETCH_HEAD");
+	char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
 
 	fp = fopen(filename, "a");
 	if (!fp)

@@ -658,7 +662,7 @@ static int do_fetch(struct transport *transport,
 		die("Don't know how to fetch from %s", transport->url);
 
 	/* if not appending, truncate FETCH_HEAD */
-	if (!append) {
+	if (!append && !dry_run) {
 		char *filename = git_path("FETCH_HEAD");
 		FILE *fp = fopen(filename, "w");
 		if (!fp)

@@ -766,9 +770,11 @@ static int add_remote_or_group(const char *name, struct string_list *list)
 static int fetch_multiple(struct string_list *list)
 {
 	int i, result = 0;
-	const char *argv[] = { "fetch", NULL, NULL, NULL, NULL, NULL };
+	const char *argv[] = { "fetch", NULL, NULL, NULL, NULL, NULL, NULL };
 	int argc = 1;
 
+	if (dry_run)
+		argv[argc++] = "--dry-run";
 	if (prune)
 		argv[argc++] = "--prune";
 	if (verbosity >= 2)

-- 
1.6.4.2

[PATCH 3/4] builtin-fetch: add --prune option

From: Jay Soffian <hidden>
Date: 2016-06-15 22:47:41

Teach fetch to cull stale remote tracking branches after fetching via --prune.

We can now enable the "--prune" option to "remote update", which was recently
re-implemented as an invocation of git fetch.

Signed-off-by: Jay Soffian <redacted>
---
 Documentation/fetch-options.txt |    4 ++++
 builtin-fetch.c                 |   32 ++++++++++++++++++++++++++++++--
 builtin-remote.c                |    4 +++-
 t/t5505-remote.sh               |    2 +-
 4 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 8b0cf58..500637a 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt

@@ -28,6 +28,10 @@ ifndef::git-pull[]
 --multiple::
 	Allow several <repository> and <group> arguments to be
 	specified. No <refspec>s may be specified.
+
+--prune::
+	After fetching, remove any remote tracking branches which
+	no longer exist	on the remote.
 endif::git-pull[]
 
 ifdef::git-pull[]

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 945dfd8..fd31072 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c

@@ -26,7 +26,7 @@ enum {
 	TAGS_SET = 2
 };
 
-static int all, append, force, keep, multiple, update_head_ok, verbosity;
+static int all, append, force, keep, multiple, prune, update_head_ok, verbosity;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;

@@ -49,6 +49,8 @@ static struct option builtin_fetch_options[] = {
 		    "fetch all tags and associated objects", TAGS_SET),
 	OPT_SET_INT('n', NULL, &tags,
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
+	OPT_BOOLEAN('p', "prune", &prune,
+		    "prune tracking branches no longer on remote"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
 	OPT_BOOLEAN('u', "update-head-ok", &update_head_ok,
 		    "allow updating of HEAD ref"),

@@ -492,6 +494,28 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
 	return ret;
 }
 
+static int prune_refs(struct transport *transport, struct ref *ref_map)
+{
+	int result = 0;
+	struct ref *ref, *stale_refs = get_stale_heads(transport->remote, ref_map);
+	const char *dangling_msg = dry_run
+		? "   (%s will become dangling)\n"
+		: "   (%s has become dangling)\n";
+
+	for (ref = stale_refs; ref; ref = ref->next) {
+		if (!dry_run)
+			result |= delete_ref(ref->name, NULL, 0);
+		if (verbosity >= 0) {
+			fprintf(stderr, " x %-*s %-*s -> %s\n",
+				SUMMARY_WIDTH, "[deleted]",
+				REFCOL_WIDTH, "(none)", prettify_refname(ref->name));
+			warn_dangling_symref(stderr, dangling_msg, ref->name);
+		}
+	}
+	free_refs(stale_refs);
+	return result;
+}
+
 static int add_existing(const char *refname, const unsigned char *sha1,
 			int flag, void *cbdata)
 {

@@ -657,6 +681,8 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
+	if (prune)
+		prune_refs(transport, ref_map);
 	free_refs(ref_map);
 
 	/* if neither --no-tags nor --tags was specified, do automated tag

@@ -740,9 +766,11 @@ static int add_remote_or_group(const char *name, struct string_list *list)
 static int fetch_multiple(struct string_list *list)
 {
 	int i, result = 0;
-	const char *argv[] = { "fetch", NULL, NULL, NULL, NULL };
+	const char *argv[] = { "fetch", NULL, NULL, NULL, NULL, NULL };
 	int argc = 1;
 
+	if (prune)
+		argv[argc++] = "--prune";
 	if (verbosity >= 2)
 		argv[argc++] = "-v";
 	if (verbosity >= 1)

diff --git a/builtin-remote.c b/builtin-remote.c
index e67b89f..fb0d66d 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c

@@ -1195,13 +1195,15 @@ static int update(int argc, const char **argv)
 	int fetch_argc = 0;
 	int default_defined = 0;
 
-	fetch_argv = xmalloc(sizeof(char *) * (argc+4));
+	fetch_argv = xmalloc(sizeof(char *) * (argc+5));
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
 			     PARSE_OPT_KEEP_ARGV0);
 
 	fetch_argv[fetch_argc++] = "fetch";
 
+	if (prune)
+		fetch_argv[fetch_argc++] = "--prune";
 	if (verbose)
 		fetch_argv[fetch_argc++] = "-v";
 	if (argc < 2) {

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 562cca2..e931ce6 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh

@@ -365,7 +365,7 @@ test_expect_success 'update with arguments' '
 
 '
 
-test_expect_failure 'update --prune' '
+test_expect_success 'update --prune' '
 
 	(cd one &&
 	 git branch -m side2 side3) &&

-- 
1.6.4.2

[PATCH 2/4] teach warn_dangling_symref to take a FILE argument

From: Jay Soffian <hidden>
Date: 2016-06-15 22:47:41

Different callers of warn_dangling_symref() may want to control whether its
output goes to stdout or stderr so let it take a FILE argument.

Signed-off-by: Jay Soffian <redacted>
---
 builtin-remote.c |    2 +-
 refs.c           |    7 ++++---
 refs.h           |    2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 0ea4e60..e67b89f 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c

@@ -1166,7 +1166,7 @@ static int prune_remote(const char *remote, int dry_run)
 
 		printf(" * [%s] %s\n", dry_run ? "would prune" : "pruned",
 		       abbrev_ref(refname, "refs/remotes/"));
-		warn_dangling_symref(dangling_msg, refname);
+		warn_dangling_symref(stdout, dangling_msg, refname);
 	}
 
 	free_remote_ref_states(&states);

diff --git a/refs.c b/refs.c
index 808f56b..3e73a0a 100644
--- a/refs.c
+++ b/refs.c

@@ -286,6 +286,7 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)
 }
 
 struct warn_if_dangling_data {
+	FILE *fp;
 	const char *refname;
 	const char *msg_fmt;
 };

@@ -304,13 +305,13 @@ static int warn_if_dangling_symref(const char *refname, const unsigned char *sha
 	if (!resolves_to || strcmp(resolves_to, d->refname))
 		return 0;
 
-	printf(d->msg_fmt, refname);
+	fprintf(d->fp, d->msg_fmt, refname);
 	return 0;
 }
 
-void warn_dangling_symref(const char *msg_fmt, const char *refname)
+void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
 {
-	struct warn_if_dangling_data data = { refname, msg_fmt };
+	struct warn_if_dangling_data data = { fp, refname, msg_fmt };
 	for_each_rawref(warn_if_dangling_symref, &data);
 }

diff --git a/refs.h b/refs.h
index 777b5b7..e141991 100644
--- a/refs.h
+++ b/refs.h

@@ -29,7 +29,7 @@ extern int for_each_replace_ref(each_ref_fn, void *);
 /* can be used to learn about broken ref and symref */
 extern int for_each_rawref(each_ref_fn, void *);
 
-extern void warn_dangling_symref(const char *msg_fmt, const char *refname);
+extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);
 
 /*
  * Extra refs will be listed by for_each_ref() before any actual refs

-- 
1.6.4.2

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help