Web lists-archives.com

[PATCH 1/1] Fix %(push:track) in ref-filter




In ref-filter.c, when processing the atom %(push:track), the
ahead/behind values are computed using `stat_tracking_info` which refers
to the upstream branch.

Fix that by introducing a new function `stat_push_info` in remote.c
(exported in remote.h), which does the same thing but for the push
branch. Factorise the ahead/behind computation of `stat_tracking_info` into
`stat_compare_info` so that it can be reused for `stat_push_info`.

This bug was not detected in t/t6300-for-each-ref.sh because in the test
for push:track, both the upstream and the push branch were ahead by 1.
Change the test so that the upstream branch is ahead by 2 while the push
branch is ahead by 1, this allow us to test that %(push:track) refer to
the correct branch.

This change the expected value of some following tests, so update them
too.

Signed-off-by: Damien Robert <damien.olivier.robert+git@xxxxxxxxx>
---
 ref-filter.c            |  7 ++--
 remote.c                | 79 +++++++++++++++++++++++++++++++----------
 remote.h                |  2 ++
 t/t6300-for-each-ref.sh | 13 ++++++-
 4 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 3aca105307..82e277222b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1391,8 +1391,11 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 	if (atom->u.remote_ref.option == RR_REF)
 		*s = show_ref(&atom->u.remote_ref.refname, refname);
 	else if (atom->u.remote_ref.option == RR_TRACK) {
-		if (stat_tracking_info(branch, &num_ours, &num_theirs,
-				       NULL, AHEAD_BEHIND_FULL) < 0) {
+		if ((atom->u.remote_ref.push ?
+		     stat_push_info(branch, &num_ours, &num_theirs,
+				    NULL, AHEAD_BEHIND_FULL) :
+		     stat_tracking_info(branch, &num_ours, &num_theirs,
+					NULL, AHEAD_BEHIND_FULL)) < 0) {
 			*s = xstrdup(msgs.gone);
 		} else if (!num_ours && !num_theirs)
 			*s = xstrdup("");
diff --git a/remote.c b/remote.c
index 9cc3b07d21..b2b37d1e8d 100644
--- a/remote.c
+++ b/remote.c
@@ -1880,8 +1880,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list)
 }
 
 /*
- * Lookup the upstream branch for the given branch and if present, optionally
- * compute the commit ahead/behind values for the pair.
+ * Compute the commit ahead/behind values for the pair branch_name, base.
  *
  * If abf is AHEAD_BEHIND_FULL, compute the full ahead/behind and return the
  * counts in *num_ours and *num_theirs.  If abf is AHEAD_BEHIND_QUICK, skip
@@ -1891,34 +1890,28 @@ int resolve_remote_symref(struct ref *ref, struct ref *list)
  * The name of the upstream branch (or NULL if no upstream is defined) is
  * returned via *upstream_name, if it is not itself NULL.
  *
- * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no
- * upstream defined, or ref does not exist).  Returns 0 if the commits are
- * identical.  Returns 1 if commits are different.
+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., ref
+ * does not exist).  Returns 0 if the commits are identical.  Returns 1 if
+ * commits are different.
  */
-int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
-		       const char **upstream_name, enum ahead_behind_flags abf)
+
+int stat_compare_info(const char **branch_name, const char **base,
+		      int *num_ours, int *num_theirs,
+		      enum ahead_behind_flags abf)
 {
 	struct object_id oid;
 	struct commit *ours, *theirs;
 	struct rev_info revs;
-	const char *base;
 	struct argv_array argv = ARGV_ARRAY_INIT;
 
-	/* Cannot stat unless we are marked to build on top of somebody else. */
-	base = branch_get_upstream(branch, NULL);
-	if (upstream_name)
-		*upstream_name = base;
-	if (!base)
-		return -1;
-
 	/* Cannot stat if what we used to build on no longer exists */
-	if (read_ref(base, &oid))
+	if (read_ref(*base, &oid))
 		return -1;
 	theirs = lookup_commit_reference(the_repository, &oid);
 	if (!theirs)
 		return -1;
 
-	if (read_ref(branch->refname, &oid))
+	if (read_ref(*branch_name, &oid))
 		return -1;
 	ours = lookup_commit_reference(the_repository, &oid);
 	if (!ours)
@@ -1932,7 +1925,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
 	if (abf == AHEAD_BEHIND_QUICK)
 		return 1;
 	if (abf != AHEAD_BEHIND_FULL)
-		BUG("stat_tracking_info: invalid abf '%d'", abf);
+		BUG("stat_compare_info: invalid abf '%d'", abf);
 
 	/* Run "rev-list --left-right ours...theirs" internally... */
 	argv_array_push(&argv, ""); /* ignored */
@@ -1966,6 +1959,56 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
 	return 1;
 }
 
+/*
+ * Lookup the upstream branch for the given branch and if present, optionally
+ * compute the commit ahead/behind values for the pair.
+ *
+ * If abf is AHEAD_BEHIND_FULL, compute the full ahead/behind and return the
+ * counts in *num_ours and *num_theirs.  If abf is AHEAD_BEHIND_QUICK, skip
+ * the (potentially expensive) a/b computation (*num_ours and *num_theirs are
+ * set to zero).
+ *
+ * The name of the upstream branch (or NULL if no upstream is defined) is
+ * returned via *upstream_name, if it is not itself NULL.
+ *
+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no
+ * upstream defined, or ref does not exist).  Returns 0 if the commits are
+ * identical.  Returns 1 if commits are different.
+ */
+int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
+		       const char **upstream_name, enum ahead_behind_flags abf)
+{
+	const char *base;
+
+	/* Cannot stat unless we are marked to build on top of somebody else. */
+	base = branch_get_upstream(branch, NULL);
+	if (upstream_name)
+		*upstream_name = base;
+	if (!base)
+		return -1;
+
+	return stat_compare_info(&(branch->refname), &base, num_ours, num_theirs, abf);
+}
+
+/*
+ * Same as above for the push branch for the given branch and if present,
+ * optionally compute the commit ahead/behind values for the pair.
+ */
+
+int stat_push_info(struct branch *branch, int *num_ours, int *num_theirs,
+		   const char **push_name, enum ahead_behind_flags abf)
+{
+	const char *base;
+
+	base = branch_get_push(branch, NULL);
+	if (push_name)
+		*push_name = base;
+	if (!base)
+		return -1;
+
+	return stat_compare_info(&(branch->refname), &base, num_ours, num_theirs, abf);
+}
+
 /*
  * Return true when there is anything to report, otherwise false.
  */
diff --git a/remote.h b/remote.h
index da53ad570b..0a179f8ded 100644
--- a/remote.h
+++ b/remote.h
@@ -254,6 +254,8 @@ enum ahead_behind_flags {
 /* Reporting of tracking info */
 int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
 		       const char **upstream_name, enum ahead_behind_flags abf);
+int stat_push_info(struct branch *branch, int *num_ours, int *num_theirs,
+		   const char **push_name, enum ahead_behind_flags abf);
 int format_tracking_info(struct branch *branch, struct strbuf *sb,
 			 enum ahead_behind_flags abf);
 
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 0ffd630713..1ec747bd32 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -392,6 +392,14 @@ test_atom head upstream:track '[ahead 1]'
 test_atom head upstream:trackshort '>'
 test_atom head upstream:track,nobracket 'ahead 1'
 test_atom head upstream:nobracket,track 'ahead 1'
+
+test_expect_success 'setup for push:track[short]' '
+	git update-ref refs/remotes/myfork/master master &&
+	test_commit third
+'
+
+test_atom head upstream:track '[ahead 2]'
+test_atom head upstream:trackshort '>'
 test_atom head push:track '[ahead 1]'
 test_atom head push:trackshort '>'
 
@@ -420,8 +428,10 @@ test_expect_success 'Check for invalid refname format' '
 test_expect_success 'set up color tests' '
 	cat >expected.color <<-EOF &&
 	$(git rev-parse --short refs/heads/master) <GREEN>master<RESET>
+	$(git rev-parse --short refs/remotes/myfork/master) <GREEN>myfork/master<RESET>
 	$(git rev-parse --short refs/remotes/origin/master) <GREEN>origin/master<RESET>
 	$(git rev-parse --short refs/tags/testtag) <GREEN>testtag<RESET>
+	$(git rev-parse --short refs/tags/third) <GREEN>third<RESET>
 	$(git rev-parse --short refs/tags/two) <GREEN>two<RESET>
 	EOF
 	sed "s/<[^>]*>//g" <expected.color >expected.bare &&
@@ -590,10 +600,11 @@ body contents
 $sig"
 
 cat >expected <<EOF
-$(git rev-parse refs/tags/bogo) <committer@xxxxxxxxxxx> refs/tags/bogo
 $(git rev-parse refs/tags/master) <committer@xxxxxxxxxxx> refs/tags/master
+$(git rev-parse refs/tags/bogo) <committer@xxxxxxxxxxx> refs/tags/bogo
 EOF
 
+
 test_expect_success 'Verify sort with multiple keys' '
 	git for-each-ref --format="%(objectname) %(taggeremail) %(refname)" --sort=objectname --sort=taggeremail \
 		refs/tags/bogo refs/tags/master > actual &&
-- 
Patched on top of v2.21.0-196-g041f5ea1cf (git version 2.21.0)