{"thread":{"id":"50938","subject":"[PATCH 0/1] Fix a bug in ref-filter","startedAt":"2019-04-15T21:05:15Z","lastAt":"2019-04-18T00:23:21Z","messageCount":8,"participants":["Damien Robert","Jeff King","Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"373907","messageId":"20190415210416.7525-1-damien.olivier.robert+git@gmail.com","threadId":"50938","inReplyTo":null,"subject":"[PATCH 0/1] Fix a bug in ref-filter","fromName":"Damien Robert","fromEmail":"damien.olivier.robert@gmail.com","sentAt":"2019-04-15T21:04:15Z","receivedAt":"2019-04-15T21:05:15Z","isPatch":true,"sender":{"key":"damien.olivier.robert@gmail.com","avatar":null},"body":"Dear git developers,\n\nTesting the behaviour of git with respect to pushing and pulling, I noticed\nthat %(push:track) in git for-each-ref reported the wrong value (ie the\nupstream's value rather than the push one).\n\nThis patch fixes that.\n\nPlease tell me if you would prefer I split this patch in two, one\nintroducing `stat_push_info` in remote.c and the following one using it in\nref-filter.c\n\n\nDamien Robert (1):\n  Fix %(push:track) in ref-filter\n\n ref-filter.c            |  7 ++--\n remote.c                | 78 +++++++++++++++++++++++++++++++----------\n remote.h                |  2 ++\n t/t6300-for-each-ref.sh | 12 ++++++-\n 4 files changed, 78 insertions(+), 21 deletions(-)\n\n-- \nPatched on top of v2.21.0-196-g041f5ea1cf (git version 2.21.0)\n\n"},{"id":"373908","messageId":"20190415210416.7525-2-damien.olivier.robert+git@gmail.com","threadId":"50938","inReplyTo":"20190415210416.7525-1-damien.olivier.robert+git@gmail.com","subject":"[PATCH 1/1] Fix %(push:track) in ref-filter","fromName":"Damien Robert","fromEmail":"damien.olivier.robert@gmail.com","sentAt":"2019-04-15T21:04:16Z","receivedAt":"2019-04-15T21:05:38Z","isPatch":true,"sender":{"key":"damien.olivier.robert@gmail.com","avatar":null},"body":"In ref-filter.c, when processing the atom %(push:track), the\nahead/behind values are computed using `stat_tracking_info` which refers\nto the upstream branch.\n\nFix that by introducing a new function `stat_push_info` in remote.c\n(exported in remote.h), which does the same thing but for the push\nbranch. Factorise the ahead/behind computation of `stat_tracking_info` into\n`stat_compare_info` so that it can be reused for `stat_push_info`.\n\nThis bug was not detected in t/t6300-for-each-ref.sh because in the test\nfor push:track, both the upstream and the push branch were ahead by 1.\nChange the test so that the upstream branch is ahead by 2 while the push\nbranch is ahead by 1, this allow us to test that %(push:track) refer to\nthe correct branch.\n\nThis change the expected value of some following tests, so update them\ntoo.\n\nSigned-off-by: Damien Robert <damien.olivier.robert+git@gmail.com>\n---\n ref-filter.c            |  7 ++--\n remote.c                | 79 +++++++++++++++++++++++++++++++----------\n remote.h                |  2 ++\n t/t6300-for-each-ref.sh | 13 ++++++-\n 4 files changed, 80 insertions(+), 21 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3aca105307..82e277222b 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1391,8 +1391,11 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n \tif (atom->u.remote_ref.option == RR_REF)\n \t\t*s = show_ref(&atom->u.remote_ref.refname, refname);\n \telse if (atom->u.remote_ref.option == RR_TRACK) {\n-\t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n-\t\t\t\t       NULL, AHEAD_BEHIND_FULL) < 0) {\n+\t\tif ((atom->u.remote_ref.push ?\n+\t\t     stat_push_info(branch, &num_ours, &num_theirs,\n+\t\t\t\t    NULL, AHEAD_BEHIND_FULL) :\n+\t\t     stat_tracking_info(branch, &num_ours, &num_theirs,\n+\t\t\t\t\tNULL, AHEAD_BEHIND_FULL)) < 0) {\n \t\t\t*s = xstrdup(msgs.gone);\n \t\t} else if (!num_ours && !num_theirs)\n \t\t\t*s = xstrdup(\"\");\ndiff --git a/remote.c b/remote.c\nindex 9cc3b07d21..b2b37d1e8d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1880,8 +1880,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list)\n }\n \n /*\n- * Lookup the upstream branch for the given branch and if present, optionally\n- * compute the commit ahead/behind values for the pair.\n+ * Compute the commit ahead/behind values for the pair branch_name, base.\n  *\n  * If abf is AHEAD_BEHIND_FULL, compute the full ahead/behind and return the\n  * counts in *num_ours and *num_theirs.  If abf is AHEAD_BEHIND_QUICK, skip\n@@ -1891,34 +1890,28 @@ int resolve_remote_symref(struct ref *ref, struct ref *list)\n  * The name of the upstream branch (or NULL if no upstream is defined) is\n  * returned via *upstream_name, if it is not itself NULL.\n  *\n- * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no\n- * upstream defined, or ref does not exist).  Returns 0 if the commits are\n- * identical.  Returns 1 if commits are different.\n+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., ref\n+ * does not exist).  Returns 0 if the commits are identical.  Returns 1 if\n+ * commits are different.\n  */\n-int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n-\t\t       const char **upstream_name, enum ahead_behind_flags abf)\n+\n+int stat_compare_info(const char **branch_name, const char **base,\n+\t\t      int *num_ours, int *num_theirs,\n+\t\t      enum ahead_behind_flags abf)\n {\n \tstruct object_id oid;\n \tstruct commit *ours, *theirs;\n \tstruct rev_info revs;\n-\tconst char *base;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n-\t/* Cannot stat unless we are marked to build on top of somebody else. */\n-\tbase = branch_get_upstream(branch, NULL);\n-\tif (upstream_name)\n-\t\t*upstream_name = base;\n-\tif (!base)\n-\t\treturn -1;\n-\n \t/* Cannot stat if what we used to build on no longer exists */\n-\tif (read_ref(base, &oid))\n+\tif (read_ref(*base, &oid))\n \t\treturn -1;\n \ttheirs = lookup_commit_reference(the_repository, &oid);\n \tif (!theirs)\n \t\treturn -1;\n \n-\tif (read_ref(branch->refname, &oid))\n+\tif (read_ref(*branch_name, &oid))\n \t\treturn -1;\n \tours = lookup_commit_reference(the_repository, &oid);\n \tif (!ours)\n@@ -1932,7 +1925,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \tif (abf == AHEAD_BEHIND_QUICK)\n \t\treturn 1;\n \tif (abf != AHEAD_BEHIND_FULL)\n-\t\tBUG(\"stat_tracking_info: invalid abf '%d'\", abf);\n+\t\tBUG(\"stat_compare_info: invalid abf '%d'\", abf);\n \n \t/* Run \"rev-list --left-right ours...theirs\" internally... */\n \targv_array_push(&argv, \"\"); /* ignored */\n@@ -1966,6 +1959,56 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \treturn 1;\n }\n \n+/*\n+ * Lookup the upstream branch for the given branch and if present, optionally\n+ * compute the commit ahead/behind values for the pair.\n+ *\n+ * If abf is AHEAD_BEHIND_FULL, compute the full ahead/behind and return the\n+ * counts in *num_ours and *num_theirs.  If abf is AHEAD_BEHIND_QUICK, skip\n+ * the (potentially expensive) a/b computation (*num_ours and *num_theirs are\n+ * set to zero).\n+ *\n+ * The name of the upstream branch (or NULL if no upstream is defined) is\n+ * returned via *upstream_name, if it is not itself NULL.\n+ *\n+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no\n+ * upstream defined, or ref does not exist).  Returns 0 if the commits are\n+ * identical.  Returns 1 if commits are different.\n+ */\n+int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n+\t\t       const char **upstream_name, enum ahead_behind_flags abf)\n+{\n+\tconst char *base;\n+\n+\t/* Cannot stat unless we are marked to build on top of somebody else. */\n+\tbase = branch_get_upstream(branch, NULL);\n+\tif (upstream_name)\n+\t\t*upstream_name = base;\n+\tif (!base)\n+\t\treturn -1;\n+\n+\treturn stat_compare_info(&(branch->refname), &base, num_ours, num_theirs, abf);\n+}\n+\n+/*\n+ * Same as above for the push branch for the given branch and if present,\n+ * optionally compute the commit ahead/behind values for the pair.\n+ */\n+\n+int stat_push_info(struct branch *branch, int *num_ours, int *num_theirs,\n+\t\t   const char **push_name, enum ahead_behind_flags abf)\n+{\n+\tconst char *base;\n+\n+\tbase = branch_get_push(branch, NULL);\n+\tif (push_name)\n+\t\t*push_name = base;\n+\tif (!base)\n+\t\treturn -1;\n+\n+\treturn stat_compare_info(&(branch->refname), &base, num_ours, num_theirs, abf);\n+}\n+\n /*\n  * Return true when there is anything to report, otherwise false.\n  */\ndiff --git a/remote.h b/remote.h\nindex da53ad570b..0a179f8ded 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -254,6 +254,8 @@ enum ahead_behind_flags {\n /* Reporting of tracking info */\n int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \t\t       const char **upstream_name, enum ahead_behind_flags abf);\n+int stat_push_info(struct branch *branch, int *num_ours, int *num_theirs,\n+\t\t   const char **push_name, enum ahead_behind_flags abf);\n int format_tracking_info(struct branch *branch, struct strbuf *sb,\n \t\t\t enum ahead_behind_flags abf);\n \ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 0ffd630713..1ec747bd32 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -392,6 +392,14 @@ test_atom head upstream:track '[ahead 1]'\n test_atom head upstream:trackshort '>'\n test_atom head upstream:track,nobracket 'ahead 1'\n test_atom head upstream:nobracket,track 'ahead 1'\n+\n+test_expect_success 'setup for push:track[short]' '\n+\tgit update-ref refs/remotes/myfork/master master &&\n+\ttest_commit third\n+'\n+\n+test_atom head upstream:track '[ahead 2]'\n+test_atom head upstream:trackshort '>'\n test_atom head push:track '[ahead 1]'\n test_atom head push:trackshort '>'\n \n@@ -420,8 +428,10 @@ test_expect_success 'Check for invalid refname format' '\n test_expect_success 'set up color tests' '\n \tcat >expected.color <<-EOF &&\n \t$(git rev-parse --short refs/heads/master) <GREEN>master<RESET>\n+\t$(git rev-parse --short refs/remotes/myfork/master) <GREEN>myfork/master<RESET>\n \t$(git rev-parse --short refs/remotes/origin/master) <GREEN>origin/master<RESET>\n \t$(git rev-parse --short refs/tags/testtag) <GREEN>testtag<RESET>\n+\t$(git rev-parse --short refs/tags/third) <GREEN>third<RESET>\n \t$(git rev-parse --short refs/tags/two) <GREEN>two<RESET>\n \tEOF\n \tsed \"s/<[^>]*>//g\" <expected.color >expected.bare &&\n@@ -590,10 +600,11 @@ body contents\n $sig\"\n \n cat >expected <<EOF\n-$(git rev-parse refs/tags/bogo) <committer@example.com> refs/tags/bogo\n $(git rev-parse refs/tags/master) <committer@example.com> refs/tags/master\n+$(git rev-parse refs/tags/bogo) <committer@example.com> refs/tags/bogo\n EOF\n \n+\n test_expect_success 'Verify sort with multiple keys' '\n \tgit for-each-ref --format=\"%(objectname) %(taggeremail) %(refname)\" --sort=objectname --sort=taggeremail \\\n \t\trefs/tags/bogo refs/tags/master > actual &&\n-- \nPatched on top of v2.21.0-196-g041f5ea1cf (git version 2.21.0)\n\n"},{"id":"373918","messageId":"20190415220108.GD28128@sigill.intra.peff.net","threadId":"50938","inReplyTo":"20190415210416.7525-2-damien.olivier.robert+git@gmail.com","subject":"Re: [PATCH 1/1] Fix %(push:track) in ref-filter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-15T22:01:08Z","receivedAt":"2019-04-15T22:01:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 15, 2019 at 11:04:16PM +0200, Damien Robert wrote:\n\n> In ref-filter.c, when processing the atom %(push:track), the\n> ahead/behind values are computed using `stat_tracking_info` which refers\n> to the upstream branch.\n\nGood catch. I think this has been broken since %(push) was added in\n29bc88505f (for-each-ref: accept \"%(push)\" format, 2015-05-21). I don't\nactually use the track option, so I never noticed.\n\n> Fix that by introducing a new function `stat_push_info` in remote.c\n> (exported in remote.h), which does the same thing but for the push\n> branch. Factorise the ahead/behind computation of `stat_tracking_info` into\n> `stat_compare_info` so that it can be reused for `stat_push_info`.\n\nMakes sense.\n\n> This bug was not detected in t/t6300-for-each-ref.sh because in the test\n> for push:track, both the upstream and the push branch were ahead by 1.\n> Change the test so that the upstream branch is ahead by 2 while the push\n> branch is ahead by 1, this allow us to test that %(push:track) refer to\n> the correct branch.\n\nNice. I wish all patches were this careful about thinking through\ndetails like this.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 3aca105307..82e277222b 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1391,8 +1391,11 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n>  \tif (atom->u.remote_ref.option == RR_REF)\n>  \t\t*s = show_ref(&atom->u.remote_ref.refname, refname);\n>  \telse if (atom->u.remote_ref.option == RR_TRACK) {\n> -\t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n> -\t\t\t\t       NULL, AHEAD_BEHIND_FULL) < 0) {\n> +\t\tif ((atom->u.remote_ref.push ?\n> +\t\t     stat_push_info(branch, &num_ours, &num_theirs,\n> +\t\t\t\t    NULL, AHEAD_BEHIND_FULL) :\n> +\t\t     stat_tracking_info(branch, &num_ours, &num_theirs,\n> +\t\t\t\t\tNULL, AHEAD_BEHIND_FULL)) < 0) {\n>  \t\t\t*s = xstrdup(msgs.gone);\n>  \t\t} else if (!num_ours && !num_theirs)\n\nI'm a big fan of the \"?\" operator, but this ternary-within-an-if might\nbe pushing even my boundaries of taste. :) I wonder if it would be more\nreadable as:\n\n  int ret;\n  if (atom->u.remote_ref.push)\n\tstat_push_info(...);\n  else\n\tstat_tracking_info(...);\n  if (ret < 0)\n\t... gone ...\n  else if (!num_ours && !num_theirs)\n\t... etc ...\n\nI'd even be OK with a ternary for assigning \"ret\". :)\n\nAll that said, we would need to do the exact same conditional for\n\":trackshort\", wouldn't we? The tests don't pick it up because the\nsymbol is still \">\" for both branches (deja vu!). So it might be worth\nnot just having push be 2 ahead, but have it actually be behind instead\n(or in addition to).\n\nSo since we have to do it twice, maybe that makes it worth factoring\nout something like:\n\n  int stat_remote_ref(struct used_atom *atom, struct branch *branch,\n                      int *num_ours, int *num_theirs)\n  {\n        if (atom->u.remote_ref.push)\n                return stat_push_info(branch, &num_ours, &num_theirs,\n                                      NULL, AHEAD_BEHIND_FULL);\n        else\n                return stat_tracking_info(branch, &num_ours, &num_theirs,\n                                          NULL, AHEAD_BEHIND_FULL);\n  }\n\nOr perhaps it argues for just giving access to the more generic stat_*\nfunction, and letting callers pass in a flag for push vs upstream (and\neither leaving stat_tracking_info() as a wrapper, or just updating its\nfew callers).\n\n> -int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n> -\t\t       const char **upstream_name, enum ahead_behind_flags abf)\n> +\n> +int stat_compare_info(const char **branch_name, const char **base,\n> +\t\t      int *num_ours, int *num_theirs,\n> +\t\t      enum ahead_behind_flags abf)\n\nIn the original, we need a pointer-to-pointer for upstream_name, because\nwe return the string as an out-parameter. But here we're just taking two\nstrings as input. We can drop the extra layer of indirection, like the\npatch below.\n\nAlso, since this is an internal helper function for the file, we should\nmark it as static.\n\ndiff --git a/remote.c b/remote.c\nindex b2b37d1e8d..e6ca62dc19 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1895,23 +1895,23 @@ int resolve_remote_symref(struct ref *ref, struct ref *list)\n  * commits are different.\n  */\n \n-int stat_compare_info(const char **branch_name, const char **base,\n-\t\t      int *num_ours, int *num_theirs,\n-\t\t      enum ahead_behind_flags abf)\n+static int stat_compare_info(const char *branch_name, const char *base,\n+\t\t\t     int *num_ours, int *num_theirs,\n+\t\t\t     enum ahead_behind_flags abf)\n {\n \tstruct object_id oid;\n \tstruct commit *ours, *theirs;\n \tstruct rev_info revs;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n \t/* Cannot stat if what we used to build on no longer exists */\n-\tif (read_ref(*base, &oid))\n+\tif (read_ref(base, &oid))\n \t\treturn -1;\n \ttheirs = lookup_commit_reference(the_repository, &oid);\n \tif (!theirs)\n \t\treturn -1;\n \n-\tif (read_ref(*branch_name, &oid))\n+\tif (read_ref(branch_name, &oid))\n \t\treturn -1;\n \tours = lookup_commit_reference(the_repository, &oid);\n \tif (!ours)\n@@ -1987,7 +1987,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \tif (!base)\n \t\treturn -1;\n \n-\treturn stat_compare_info(&(branch->refname), &base, num_ours, num_theirs, abf);\n+\treturn stat_compare_info(branch->refname, base, num_ours, num_theirs, abf);\n }\n \n /*\n@@ -2006,7 +2006,7 @@ int stat_push_info(struct branch *branch, int *num_ours, int *num_theirs,\n \tif (!base)\n \t\treturn -1;\n \n-\treturn stat_compare_info(&(branch->refname), &base, num_ours, num_theirs, abf);\n+\treturn stat_compare_info(branch->refname, base, num_ours, num_theirs, abf);\n }\n \n /*\n\nOf course if you buy my argument above that we should just let\nref-filter call into the generic form of the function, then all of that\nwould change. :)\n\n> [...]\n\nOther than that, the patch looked quite reasonable. I didn't dig too far\ninto the ripple effects of the test changes, since I think we'll end up\nchanging them again to make sure \":trackshort\" is distinct.\n\nThanks for working on this.\n\n-Peff\n"},{"id":"373970","messageId":"20190416123944.vtoremaitywtmkhj@mithrim","threadId":"50938","inReplyTo":"20190415220108.GD28128@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] Fix %(push:track) in ref-filter","fromName":"Damien Robert","fromEmail":"damien.olivier.robert@gmail.com","sentAt":"2019-04-16T12:39:45Z","receivedAt":"2019-04-16T12:39:50Z","isPatch":true,"sender":{"key":"damien.olivier.robert@gmail.com","avatar":null},"body":"From Jeff King, Mon 15 Apr 2019 at 18:01:08 (-0400) :\n> > +\t\tif ((atom->u.remote_ref.push ?\n> > +\t\t     stat_push_info(branch, &num_ours, &num_theirs,\n> > +\t\t\t\t    NULL, AHEAD_BEHIND_FULL) :\n> > +\t\t     stat_tracking_info(branch, &num_ours, &num_theirs,\n> > +\t\t\t\t\tNULL, AHEAD_BEHIND_FULL)) < 0) {\n\n> I'm a big fan of the \"?\" operator, but this ternary-within-an-if might\n> be pushing even my boundaries of taste. :)\n\nI knew I was pushing the limit of readability a bit here :)\n\n> All that said, we would need to do the exact same conditional for\n> \":trackshort\", wouldn't we?\n\nCrap, you are right, I wanted to handle this case too but forgot :-(\n\n> The tests don't pick it up because the\n> symbol is still \">\" for both branches (deja vu!). So it might be worth\n> not just having push be 2 ahead, but have it actually be behind instead\n> (or in addition to).\n\nDone in the new version.\n\n> Or perhaps it argues for just giving access to the more generic stat_*\n> function, and letting callers pass in a flag for push vs upstream (and\n> either leaving stat_tracking_info() as a wrapper, or just updating its\n> few callers).\n\nSo I went ahead with modifying `stat_tracking_info` to accept a 'for_push'\nflag, and updated the few callers. This means that `stat_compare_info` is\nonly used by `stat_tracking_info` so I could reinline it, but I guess it\ncould still be useful latter.\n\n> \n> > -int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n> > -\t\t       const char **upstream_name, enum ahead_behind_flags abf)\n> > +\n> > +int stat_compare_info(const char **branch_name, const char **base,\n> > +\t\t      int *num_ours, int *num_theirs,\n> > +\t\t      enum ahead_behind_flags abf)\n> \n> In the original, we need a pointer-to-pointer for upstream_name, because\n> we return the string as an out-parameter. But here we're just taking two\n> strings as input. We can drop the extra layer of indirection, like the\n> patch below.\n\nGood catch, done.\n\n> Also, since this is an internal helper function for the file, we should\n> mark it as static.\n\nYes. In fact in the first version of the patch I would call\n`stat_compare_info` directly in `ref_filter.c` so I needed to export it in\n`remote.h`, and then when I changed the patch I forgot to make it static.\n\n> Other than that, the patch looked quite reasonable. I didn't dig too far\n> into the ripple effects of the test changes, since I think we'll end up\n> changing them again to make sure \":trackshort\" is distinct.\n\nThere are now less impactful than before because the master branch in the\ntest refers to the same commit as before; there is just a new commit for the\n'myfork' remote branch.\n\n> Thanks for working on this.\n\nYou are welcome. What's the standard way to acknowledge your help in\nthe Foo-By: trailers? I did not put a Reviewed-By: because you reviewed the\nprevious patch, not the current one :)\n\n-- \nDamien Robert\nhttp://www.normalesup.org/~robert/pro\n\n---- >8 -----\nFrom: Damien Robert <damien.olivier.robert+git@gmail.com>\nDate: Tue, 16 Apr 2019 14:16:46 +0200\nSubject: [v2 PATCH 1/1] Fix %(push:track) in ref-filter\n\nIn ref-filter.c, when processing the atom %(push:track), the\nahead/behind values are computed using `stat_tracking_info` which refers\nto the upstream branch.\n\nFix that by introducing a new flag `for_push` in `stat_tracking_info`\nin remote.c, which does the same thing but for the push branch.\nUpdate the few callers of `stat_tracking_info` to handle this flag. This\nensure that whenever we use this function in the future, we are careful\nto specify is this should apply to the upstream or the push branch.\n\nThis bug was not detected in t/t6300-for-each-ref.sh because in the test\nfor push:track, both the upstream and the push branches were behind by 1\nfrom the local branch. Change the test so that the upstream branch is\nbehind by 1 while the push branch is ahead by 1. This allows us to test\nthat %(push:track) refer to the correct branch.\n\nThis change the expected value of some following tests (by introducing\nnew references), so update them too.\n\nSigned-off-by: Damien Robert <damien.olivier.robert+git@gmail.com>\n---\n ref-filter.c            |  6 ++--\n remote.c                | 68 ++++++++++++++++++++++++++++-------------\n remote.h                |  3 +-\n t/t6300-for-each-ref.sh | 14 +++++++--\n wt-status.c             |  4 +--\n 5 files changed, 67 insertions(+), 28 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3aca105307..31af81fb28 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1392,7 +1392,8 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n \t\t*s = show_ref(&atom->u.remote_ref.refname, refname);\n \telse if (atom->u.remote_ref.option == RR_TRACK) {\n \t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n-\t\t\t\t       NULL, AHEAD_BEHIND_FULL) < 0) {\n+\t\t\t\t       NULL, atom->u.remote_ref.push,\n+\t\t\t\t       AHEAD_BEHIND_FULL) < 0) {\n \t\t\t*s = xstrdup(msgs.gone);\n \t\t} else if (!num_ours && !num_theirs)\n \t\t\t*s = xstrdup(\"\");\n@@ -1410,7 +1411,8 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n \t\t}\n \t} else if (atom->u.remote_ref.option == RR_TRACKSHORT) {\n \t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n-\t\t\t\t       NULL, AHEAD_BEHIND_FULL) < 0) {\n+\t\t\t\t       NULL, atom->u.remote_ref.push,\n+\t\t\t\t       AHEAD_BEHIND_FULL) < 0) {\n \t\t\t*s = xstrdup(\"\");\n \t\t\treturn;\n \t\t}\ndiff --git a/remote.c b/remote.c\nindex 9cc3b07d21..e98c6f2a0a 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1880,37 +1880,27 @@ int resolve_remote_symref(struct ref *ref, struct ref *list)\n }\n \n /*\n- * Lookup the upstream branch for the given branch and if present, optionally\n- * compute the commit ahead/behind values for the pair.\n+ * Compute the commit ahead/behind values for the pair branch_name, base.\n  *\n  * If abf is AHEAD_BEHIND_FULL, compute the full ahead/behind and return the\n  * counts in *num_ours and *num_theirs.  If abf is AHEAD_BEHIND_QUICK, skip\n  * the (potentially expensive) a/b computation (*num_ours and *num_theirs are\n  * set to zero).\n  *\n- * The name of the upstream branch (or NULL if no upstream is defined) is\n- * returned via *upstream_name, if it is not itself NULL.\n- *\n- * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no\n- * upstream defined, or ref does not exist).  Returns 0 if the commits are\n- * identical.  Returns 1 if commits are different.\n+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., ref\n+ * does not exist).  Returns 0 if the commits are identical.  Returns 1 if\n+ * commits are different.\n  */\n-int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n-\t\t       const char **upstream_name, enum ahead_behind_flags abf)\n+\n+static int stat_compare_info(const char *branch_name, const char *base,\n+\t\t\t     int *num_ours, int *num_theirs,\n+\t\t\t     enum ahead_behind_flags abf)\n {\n \tstruct object_id oid;\n \tstruct commit *ours, *theirs;\n \tstruct rev_info revs;\n-\tconst char *base;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n-\t/* Cannot stat unless we are marked to build on top of somebody else. */\n-\tbase = branch_get_upstream(branch, NULL);\n-\tif (upstream_name)\n-\t\t*upstream_name = base;\n-\tif (!base)\n-\t\treturn -1;\n-\n \t/* Cannot stat if what we used to build on no longer exists */\n \tif (read_ref(base, &oid))\n \t\treturn -1;\n@@ -1918,7 +1908,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \tif (!theirs)\n \t\treturn -1;\n \n-\tif (read_ref(branch->refname, &oid))\n+\tif (read_ref(branch_name, &oid))\n \t\treturn -1;\n \tours = lookup_commit_reference(the_repository, &oid);\n \tif (!ours)\n@@ -1932,7 +1922,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \tif (abf == AHEAD_BEHIND_QUICK)\n \t\treturn 1;\n \tif (abf != AHEAD_BEHIND_FULL)\n-\t\tBUG(\"stat_tracking_info: invalid abf '%d'\", abf);\n+\t\tBUG(\"stat_compare_info: invalid abf '%d'\", abf);\n \n \t/* Run \"rev-list --left-right ours...theirs\" internally... */\n \targv_array_push(&argv, \"\"); /* ignored */\n@@ -1966,6 +1956,42 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \treturn 1;\n }\n \n+/*\n+ * Lookup the upstream branch for the given branch and if present, optionally\n+ * compute the commit ahead/behind values for the pair.\n+ *\n+ * If abf is AHEAD_BEHIND_FULL, compute the full ahead/behind and return the\n+ * counts in *num_ours and *num_theirs.  If abf is AHEAD_BEHIND_QUICK, skip\n+ * the (potentially expensive) a/b computation (*num_ours and *num_theirs are\n+ * set to zero).\n+ *\n+ * The name of the upstream branch (or NULL if no upstream is defined) is\n+ * returned via *upstream_name, if it is not itself NULL.\n+ *\n+ * If for_push is true, then return the stats and name of the push branch\n+ * rather than the upstream branch.\n+ *\n+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no\n+ * upstream defined, or ref does not exist).  Returns 0 if the commits are\n+ * identical.  Returns 1 if commits are different.\n+ */\n+int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n+\t\t       const char **upstream_name, int for_push,\n+\t\t       enum ahead_behind_flags abf)\n+{\n+\tconst char *base;\n+\n+\t/* Cannot stat unless we are marked to build on top of somebody else. */\n+\tbase = for_push ? branch_get_push(branch, NULL) :\n+\t\tbranch_get_upstream(branch, NULL);\n+\tif (upstream_name)\n+\t\t*upstream_name = base;\n+\tif (!base)\n+\t\treturn -1;\n+\n+\treturn stat_compare_info(branch->refname, base, num_ours, num_theirs, abf);\n+}\n+\n /*\n  * Return true when there is anything to report, otherwise false.\n  */\n@@ -1977,7 +2003,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,\n \tchar *base;\n \tint upstream_is_gone = 0;\n \n-\tsti = stat_tracking_info(branch, &ours, &theirs, &full_base, abf);\n+\tsti = stat_tracking_info(branch, &ours, &theirs, &full_base, 0, abf);\n \tif (sti < 0) {\n \t\tif (!full_base)\n \t\t\treturn 0;\ndiff --git a/remote.h b/remote.h\nindex da53ad570b..0138b3fb98 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -253,7 +253,8 @@ enum ahead_behind_flags {\n \n /* Reporting of tracking info */\n int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n-\t\t       const char **upstream_name, enum ahead_behind_flags abf);\n+\t\t       const char **upstream_name, int for_push,\n+\t\t       enum ahead_behind_flags abf);\n int format_tracking_info(struct branch *branch, struct strbuf *sb,\n \t\t\t enum ahead_behind_flags abf);\n \ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 0ffd630713..836c985744 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -392,8 +392,15 @@ test_atom head upstream:track '[ahead 1]'\n test_atom head upstream:trackshort '>'\n test_atom head upstream:track,nobracket 'ahead 1'\n test_atom head upstream:nobracket,track 'ahead 1'\n-test_atom head push:track '[ahead 1]'\n-test_atom head push:trackshort '>'\n+\n+test_expect_success 'setup for push:track[short]' '\n+\ttest_commit third &&\n+\tgit update-ref refs/remotes/myfork/master master &&\n+\tgit reset master~1\n+'\n+\n+test_atom head push:track '[behind 1]'\n+test_atom head push:trackshort '<'\n \n test_expect_success 'Check that :track[short] cannot be used with other atoms' '\n \ttest_must_fail git for-each-ref --format=\"%(refname:track)\" 2>/dev/null &&\n@@ -420,8 +427,10 @@ test_expect_success 'Check for invalid refname format' '\n test_expect_success 'set up color tests' '\n \tcat >expected.color <<-EOF &&\n \t$(git rev-parse --short refs/heads/master) <GREEN>master<RESET>\n+\t$(git rev-parse --short refs/remotes/myfork/master) <GREEN>myfork/master<RESET>\n \t$(git rev-parse --short refs/remotes/origin/master) <GREEN>origin/master<RESET>\n \t$(git rev-parse --short refs/tags/testtag) <GREEN>testtag<RESET>\n+\t$(git rev-parse --short refs/tags/third) <GREEN>third<RESET>\n \t$(git rev-parse --short refs/tags/two) <GREEN>two<RESET>\n \tEOF\n \tsed \"s/<[^>]*>//g\" <expected.color >expected.bare &&\n@@ -594,6 +603,7 @@ $(git rev-parse refs/tags/bogo) <committer@example.com> refs/tags/bogo\n $(git rev-parse refs/tags/master) <committer@example.com> refs/tags/master\n EOF\n \n+\n test_expect_success 'Verify sort with multiple keys' '\n \tgit for-each-ref --format=\"%(objectname) %(taggeremail) %(refname)\" --sort=objectname --sort=taggeremail \\\n \t\trefs/tags/bogo refs/tags/master > actual &&\ndiff --git a/wt-status.c b/wt-status.c\nindex 445a36204a..5a7ec2cf99 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1851,7 +1851,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \tcolor_fprintf(s->fp, branch_color_local, \"%s\", branch_name);\n \n \tsti = stat_tracking_info(branch, &num_ours, &num_theirs, &base,\n-\t\t\t\t s->ahead_behind_flags);\n+\t\t\t\t 0, s->ahead_behind_flags);\n \tif (sti < 0) {\n \t\tif (!base)\n \t\t\tgoto conclude;\n@@ -1990,7 +1990,7 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n \t\tbranch = branch_get(branch_name);\n \t\tbase = NULL;\n \t\tab_info = stat_tracking_info(branch, &nr_ahead, &nr_behind,\n-\t\t\t\t\t     &base, s->ahead_behind_flags);\n+\t\t\t\t\t     &base, 0, s->ahead_behind_flags);\n \t\tif (base) {\n \t\t\tbase = shorten_unambiguous_ref(base, 0);\n \t\t\tfprintf(s->fp, \"# branch.upstream %s%c\", base, eol);\n-- \nPatched on top of v2.21.0-313-ge35b8cb8e2 (git version 2.21.0)\n\n"},{"id":"373973","messageId":"CAP8UFD1OuhTrCcD43wLQ8m94sZbVAn_8_r3Jjuj0XTsmvx9BhA@mail.gmail.com","threadId":"50938","inReplyTo":"20190416123944.vtoremaitywtmkhj@mithrim","subject":"Re: [PATCH 1/1] Fix %(push:track) in ref-filter","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-04-16T14:13:57Z","receivedAt":"2019-04-16T14:14:12Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Apr 16, 2019 at 2:42 PM Damien Robert\n<damien.olivier.robert@gmail.com> wrote:\n\n> You are welcome. What's the standard way to acknowledge your help in\n> the Foo-By: trailers? I did not put a Reviewed-By: because you reviewed the\n> previous patch, not the current one :)\n\nWe often use:\n\nHelped-by: Jeff King <peff@peff.net>\n"},{"id":"373989","messageId":"20190416214842.GA21429@sigill.intra.peff.net","threadId":"50938","inReplyTo":"20190416123944.vtoremaitywtmkhj@mithrim","subject":"Re: [PATCH 1/1] Fix %(push:track) in ref-filter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-16T21:48:43Z","receivedAt":"2019-04-16T21:48:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 16, 2019 at 02:39:45PM +0200, Damien Robert wrote:\n\n> > Or perhaps it argues for just giving access to the more generic stat_*\n> > function, and letting callers pass in a flag for push vs upstream (and\n> > either leaving stat_tracking_info() as a wrapper, or just updating its\n> > few callers).\n> \n> So I went ahead with modifying `stat_tracking_info` to accept a 'for_push'\n> flag, and updated the few callers. This means that `stat_compare_info` is\n> only used by `stat_tracking_info` so I could reinline it, but I guess it\n> could still be useful latter.\n\nReading this paragraph, my gut reaction was to say that it should stay\nas a single function. But actually looking at the code, I think it is a\nbit nicer to separate out \"compare these two branches\" from \"figure out\nwhich branches to compare\".\n\nThe name \"compare_info\" is a bit vague. Perhaps \"stat_branch_pair\" or\nsomething would be more descriptive.\n\n> > Also, since this is an internal helper function for the file, we should\n> > mark it as static.\n> \n> Yes. In fact in the first version of the patch I would call\n> `stat_compare_info` directly in `ref_filter.c` so I needed to export it in\n> `remote.h`, and then when I changed the patch I forgot to make it static.\n\nHeh. I wondered if that might have been the reason.\n\n> > Thanks for working on this.\n> \n> You are welcome. What's the standard way to acknowledge your help in\n> the Foo-By: trailers? I did not put a Reviewed-By: because you reviewed the\n> previous patch, not the current one :)\n\nRight, Reviewed-by wouldn't be quite right. As Christian noted,\nHelped-by can be used for this (but I am also fine without credit;\nsuggestions are a normal part of review).\n\nOverall the patch looks good to me. I have a few extremely minor nits:\n\n> Subject: [v2 PATCH 1/1] Fix %(push:track) in ref-filter\n\nWe'd usually say \"area: do something\" here, and it's nice to stay\nconsistent so that reading --oneline output is easy. And it's nice if we\ncan avoid vague terms like \"fix\". Maybe:\n\n ref-filter: use correct branch for %(push:track)\n\nor something?\n\n> This bug was not detected in t/t6300-for-each-ref.sh because in the test\n> for push:track, both the upstream and the push branches were behind by 1\n> from the local branch. Change the test so that the upstream branch is\n> behind by 1 while the push branch is ahead by 1. This allows us to test\n> that %(push:track) refer to the correct branch.\n\ns/refer/&s/\n\n> This change the expected value of some following tests (by introducing\n> new references), so update them too.\n\ns/change/&s/\n\n>  \tif (abf != AHEAD_BEHIND_FULL)\n> -\t\tBUG(\"stat_tracking_info: invalid abf '%d'\", abf);\n> +\t\tBUG(\"stat_compare_info: invalid abf '%d'\", abf);\n\nIf we do the name change I mentioned above, don't forger this line. :)\n\n> +int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n> +\t\t       const char **upstream_name, int for_push,\n> +\t\t       enum ahead_behind_flags abf)\n> +{\n\nIs it worth changing \"upstream_name\" since it sometimes is now not\n%(upstream)?\n\n> @@ -1977,7 +2003,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,\n>  \tchar *base;\n>  \tint upstream_is_gone = 0;\n>  \n> -\tsti = stat_tracking_info(branch, &ours, &theirs, &full_base, abf);\n> +\tsti = stat_tracking_info(branch, &ours, &theirs, &full_base, 0, abf);\n\nI was tempted to suggest doing this refactor as a separate patch, so\nthat we'd see less noise in the diff. But in fact half of the callers\nwe'd have to touch are ones that would be modified to use for_push\nanyway. So I think it makes sense to just keep it all together as a\nsingle unit.\n\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> [...]\n> @@ -594,6 +603,7 @@ $(git rev-parse refs/tags/bogo) <committer@example.com> refs/tags/bogo\n>  $(git rev-parse refs/tags/master) <committer@example.com> refs/tags/master\n>  EOF\n>  \n> +\n>  test_expect_success 'Verify sort with multiple keys' '\n>  \tgit for-each-ref --format=\"%(objectname) %(taggeremail) %(refname)\" --sort=objectname --sort=taggeremail \\\n>  \t\trefs/tags/bogo refs/tags/master > actual &&\n\nLeftover stray whitespace?\n\nFor any one of those nits I'd probably say it was not worth a re-roll\n(and the maintainer could adjust them when he picks up the patch).  But\nthere are just enough that it's probably worth making his life easier\nwith a v3.\n\nYou can put my Reviewed-by on it, too. :)\n\n-Peff\n"},{"id":"374010","messageId":"20190417081754.bd27mjxjx7qdxhty@doriath","threadId":"50938","inReplyTo":"20190416214842.GA21429@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] Fix %(push:track) in ref-filter","fromName":"Damien Robert","fromEmail":"damien.olivier.robert@gmail.com","sentAt":"2019-04-17T08:17:54Z","receivedAt":"2019-04-17T08:18:00Z","isPatch":true,"sender":{"key":"damien.olivier.robert@gmail.com","avatar":null},"body":"From Jeff King, Tue 16 Apr 2019 at 17:48:43 (-0400) :\n> The name \"compare_info\" is a bit vague. Perhaps \"stat_branch_pair\" or\n> something would be more descriptive.\n\nDone.\n \n>  ref-filter: use correct branch for %(push:track)\n\nDone.\n\n> s/refer/&s/\n> s/change/&s/\n\nGrmf, thanks.\n\n> Is it worth changing \"upstream_name\" since it sometimes is now not\n> %(upstream)?\n\nChanged to tracking_name.\n\n> Leftover stray whitespace?\n\nOups.\n \n> For any one of those nits I'd probably say it was not worth a re-roll\n> (and the maintainer could adjust them when he picks up the patch).  But\n> there are just enough that it's probably worth making his life easier\n> with a v3.\n> You can put my Reviewed-by on it, too. :)\n\nHere it is:\n\n---- >8 ----\nFrom: Damien Robert <damien.olivier.robert+git@gmail.com>\nDate: Tue, 16 Apr 2019 14:16:46 +0200\nSubject: [PATCHv3 1/1] ref-filter: use correct branch for %(push:track)\n\nIn ref-filter.c, when processing the atom %(push:track), the\nahead/behind values are computed using `stat_tracking_info` which refers\nto the upstream branch.\n\nFix that by introducing a new flag `for_push` in `stat_tracking_info`\nin remote.c, which does the same thing but for the push branch.\nUpdate the few callers of `stat_tracking_info` to handle this flag. This\nensure that whenever we use this function in the future, we are careful\nto specify is this should apply to the upstream or the push branch.\n\nThis bug was not detected in t/t6300-for-each-ref.sh because in the test\nfor push:track, both the upstream and the push branches were behind by 1\nfrom the local branch. Change the test so that the upstream branch is\nbehind by 1 while the push branch is ahead by 1. This allows us to test\nthat %(push:track) refers to the correct branch.\n\nThis changes the expected value of some following tests (by introducing\nnew references), so update them too.\n\nReviewed-by: Jeff King <peff@peff.net>\nSigned-off-by: Damien Robert <damien.olivier.robert+git@gmail.com>\n---\n ref-filter.c            |  6 ++--\n remote.c                | 68 ++++++++++++++++++++++++++++-------------\n remote.h                |  3 +-\n t/t6300-for-each-ref.sh | 13 ++++++--\n wt-status.c             |  4 +--\n 5 files changed, 66 insertions(+), 28 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3aca105307..31af81fb28 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1392,7 +1392,8 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n \t\t*s = show_ref(&atom->u.remote_ref.refname, refname);\n \telse if (atom->u.remote_ref.option == RR_TRACK) {\n \t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n-\t\t\t\t       NULL, AHEAD_BEHIND_FULL) < 0) {\n+\t\t\t\t       NULL, atom->u.remote_ref.push,\n+\t\t\t\t       AHEAD_BEHIND_FULL) < 0) {\n \t\t\t*s = xstrdup(msgs.gone);\n \t\t} else if (!num_ours && !num_theirs)\n \t\t\t*s = xstrdup(\"\");\n@@ -1410,7 +1411,8 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,\n \t\t}\n \t} else if (atom->u.remote_ref.option == RR_TRACKSHORT) {\n \t\tif (stat_tracking_info(branch, &num_ours, &num_theirs,\n-\t\t\t\t       NULL, AHEAD_BEHIND_FULL) < 0) {\n+\t\t\t\t       NULL, atom->u.remote_ref.push,\n+\t\t\t\t       AHEAD_BEHIND_FULL) < 0) {\n \t\t\t*s = xstrdup(\"\");\n \t\t\treturn;\n \t\t}\ndiff --git a/remote.c b/remote.c\nindex 9cc3b07d21..0761d1ab21 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1880,37 +1880,27 @@ int resolve_remote_symref(struct ref *ref, struct ref *list)\n }\n \n /*\n- * Lookup the upstream branch for the given branch and if present, optionally\n- * compute the commit ahead/behind values for the pair.\n+ * Compute the commit ahead/behind values for the pair branch_name, base.\n  *\n  * If abf is AHEAD_BEHIND_FULL, compute the full ahead/behind and return the\n  * counts in *num_ours and *num_theirs.  If abf is AHEAD_BEHIND_QUICK, skip\n  * the (potentially expensive) a/b computation (*num_ours and *num_theirs are\n  * set to zero).\n  *\n- * The name of the upstream branch (or NULL if no upstream is defined) is\n- * returned via *upstream_name, if it is not itself NULL.\n- *\n- * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no\n- * upstream defined, or ref does not exist).  Returns 0 if the commits are\n- * identical.  Returns 1 if commits are different.\n+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., ref\n+ * does not exist).  Returns 0 if the commits are identical.  Returns 1 if\n+ * commits are different.\n  */\n-int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n-\t\t       const char **upstream_name, enum ahead_behind_flags abf)\n+\n+static int stat_branch_pair(const char *branch_name, const char *base,\n+\t\t\t     int *num_ours, int *num_theirs,\n+\t\t\t     enum ahead_behind_flags abf)\n {\n \tstruct object_id oid;\n \tstruct commit *ours, *theirs;\n \tstruct rev_info revs;\n-\tconst char *base;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n-\t/* Cannot stat unless we are marked to build on top of somebody else. */\n-\tbase = branch_get_upstream(branch, NULL);\n-\tif (upstream_name)\n-\t\t*upstream_name = base;\n-\tif (!base)\n-\t\treturn -1;\n-\n \t/* Cannot stat if what we used to build on no longer exists */\n \tif (read_ref(base, &oid))\n \t\treturn -1;\n@@ -1918,7 +1908,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \tif (!theirs)\n \t\treturn -1;\n \n-\tif (read_ref(branch->refname, &oid))\n+\tif (read_ref(branch_name, &oid))\n \t\treturn -1;\n \tours = lookup_commit_reference(the_repository, &oid);\n \tif (!ours)\n@@ -1932,7 +1922,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \tif (abf == AHEAD_BEHIND_QUICK)\n \t\treturn 1;\n \tif (abf != AHEAD_BEHIND_FULL)\n-\t\tBUG(\"stat_tracking_info: invalid abf '%d'\", abf);\n+\t\tBUG(\"stat_branch_pair: invalid abf '%d'\", abf);\n \n \t/* Run \"rev-list --left-right ours...theirs\" internally... */\n \targv_array_push(&argv, \"\"); /* ignored */\n@@ -1966,6 +1956,42 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n \treturn 1;\n }\n \n+/*\n+ * Lookup the tracking branch for the given branch and if present, optionally\n+ * compute the commit ahead/behind values for the pair.\n+ *\n+ * If for_push is true, the tracking branch refers to the push branch,\n+ * otherwise it refers to the upstream branch.\n+ *\n+ * The name of the tracking branch (or NULL if it is not defined) is\n+ * returned via *tracking_name, if it is not itself NULL.\n+ *\n+ * If abf is AHEAD_BEHIND_FULL, compute the full ahead/behind and return the\n+ * counts in *num_ours and *num_theirs.  If abf is AHEAD_BEHIND_QUICK, skip\n+ * the (potentially expensive) a/b computation (*num_ours and *num_theirs are\n+ * set to zero).\n+ *\n+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no\n+ * upstream defined, or ref does not exist).  Returns 0 if the commits are\n+ * identical.  Returns 1 if commits are different.\n+ */\n+int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n+\t\t       const char **tracking_name, int for_push,\n+\t\t       enum ahead_behind_flags abf)\n+{\n+\tconst char *base;\n+\n+\t/* Cannot stat unless we are marked to build on top of somebody else. */\n+\tbase = for_push ? branch_get_push(branch, NULL) :\n+\t\tbranch_get_upstream(branch, NULL);\n+\tif (tracking_name)\n+\t\t*tracking_name = base;\n+\tif (!base)\n+\t\treturn -1;\n+\n+\treturn stat_branch_pair(branch->refname, base, num_ours, num_theirs, abf);\n+}\n+\n /*\n  * Return true when there is anything to report, otherwise false.\n  */\n@@ -1977,7 +2003,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,\n \tchar *base;\n \tint upstream_is_gone = 0;\n \n-\tsti = stat_tracking_info(branch, &ours, &theirs, &full_base, abf);\n+\tsti = stat_tracking_info(branch, &ours, &theirs, &full_base, 0, abf);\n \tif (sti < 0) {\n \t\tif (!full_base)\n \t\t\treturn 0;\ndiff --git a/remote.h b/remote.h\nindex da53ad570b..0138b3fb98 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -253,7 +253,8 @@ enum ahead_behind_flags {\n \n /* Reporting of tracking info */\n int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,\n-\t\t       const char **upstream_name, enum ahead_behind_flags abf);\n+\t\t       const char **upstream_name, int for_push,\n+\t\t       enum ahead_behind_flags abf);\n int format_tracking_info(struct branch *branch, struct strbuf *sb,\n \t\t\t enum ahead_behind_flags abf);\n \ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 0ffd630713..d9235217fc 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -392,8 +392,15 @@ test_atom head upstream:track '[ahead 1]'\n test_atom head upstream:trackshort '>'\n test_atom head upstream:track,nobracket 'ahead 1'\n test_atom head upstream:nobracket,track 'ahead 1'\n-test_atom head push:track '[ahead 1]'\n-test_atom head push:trackshort '>'\n+\n+test_expect_success 'setup for push:track[short]' '\n+\ttest_commit third &&\n+\tgit update-ref refs/remotes/myfork/master master &&\n+\tgit reset master~1\n+'\n+\n+test_atom head push:track '[behind 1]'\n+test_atom head push:trackshort '<'\n \n test_expect_success 'Check that :track[short] cannot be used with other atoms' '\n \ttest_must_fail git for-each-ref --format=\"%(refname:track)\" 2>/dev/null &&\n@@ -420,8 +427,10 @@ test_expect_success 'Check for invalid refname format' '\n test_expect_success 'set up color tests' '\n \tcat >expected.color <<-EOF &&\n \t$(git rev-parse --short refs/heads/master) <GREEN>master<RESET>\n+\t$(git rev-parse --short refs/remotes/myfork/master) <GREEN>myfork/master<RESET>\n \t$(git rev-parse --short refs/remotes/origin/master) <GREEN>origin/master<RESET>\n \t$(git rev-parse --short refs/tags/testtag) <GREEN>testtag<RESET>\n+\t$(git rev-parse --short refs/tags/third) <GREEN>third<RESET>\n \t$(git rev-parse --short refs/tags/two) <GREEN>two<RESET>\n \tEOF\n \tsed \"s/<[^>]*>//g\" <expected.color >expected.bare &&\ndiff --git a/wt-status.c b/wt-status.c\nindex 445a36204a..5a7ec2cf99 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1851,7 +1851,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)\n \tcolor_fprintf(s->fp, branch_color_local, \"%s\", branch_name);\n \n \tsti = stat_tracking_info(branch, &num_ours, &num_theirs, &base,\n-\t\t\t\t s->ahead_behind_flags);\n+\t\t\t\t 0, s->ahead_behind_flags);\n \tif (sti < 0) {\n \t\tif (!base)\n \t\t\tgoto conclude;\n@@ -1990,7 +1990,7 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n \t\tbranch = branch_get(branch_name);\n \t\tbase = NULL;\n \t\tab_info = stat_tracking_info(branch, &nr_ahead, &nr_behind,\n-\t\t\t\t\t     &base, s->ahead_behind_flags);\n+\t\t\t\t\t     &base, 0, s->ahead_behind_flags);\n \t\tif (base) {\n \t\t\tbase = shorten_unambiguous_ref(base, 0);\n \t\t\tfprintf(s->fp, \"# branch.upstream %s%c\", base, eol);\n-- \nPatched on top of v2.21.0-313-ge35b8cb8e2 (git version 2.21.0)\n\n"},{"id":"374077","messageId":"xmqqef60jh56.fsf@gitster-ct.c.googlers.com","threadId":"50938","inReplyTo":"20190417081754.bd27mjxjx7qdxhty@doriath","subject":"Re: [PATCH 1/1] Fix %(push:track) in ref-filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-18T00:23:17Z","receivedAt":"2019-04-18T00:23:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Damien Robert <damien.olivier.robert@gmail.com> writes:\n\n> From: Damien Robert <damien.olivier.robert+git@gmail.com>\n> Date: Tue, 16 Apr 2019 14:16:46 +0200\n> Subject: [PATCHv3 1/1] ref-filter: use correct branch for %(push:track)\n>\n> In ref-filter.c, when processing the atom %(push:track), the\n> ahead/behind values are computed using `stat_tracking_info` which refers\n> to the upstream branch.\n> ...\n\nThanks, both.  Will queue.\n"}]}