The code handling %(upstream:track) and %(upstream:trackshort) assumed it always had a valid branch that had been sanitized earlier in populate_value(), and thus did not check the return value of the call to stat_tracking_info().
While there is indeed some sanitization code that basically corresponds to stat_tracking_info() returning 0 (no base branch set), the function can also return -1 when the base branch did exist but has since then been deleted.
In this case, num_ours and num_theirs had undefined values and a call to `git for-each-ref --format="%(upstream:track)"` could print spurious values such as
[behind -111794512] [ahead 38881640, behind 5103867]
even for repositories with one single commit.
We now properly verify stat_tracking_info()'s return value and do not print anything if it returns -1. This behavior also matches the documentation ("has no effect if the ref does not have tracking information associated with it").
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
---
v4: Use Jeff's suggestion and simplify the test case and making it
easier to understand what is actually being tested.builtin/for-each-ref.c | 11 +++++++++-- t/t6300-for-each-ref.sh | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-)
Show changes to 2 files +23 −2
builtin/for-each-ref.c, t/t6300-for-each-ref.sh
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 603a90e..52e6323 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -717,7 +717,10 @@ static void populate_value(struct refinfo *ref) starts_with(name, "upstream")) { char buf[40]; - stat_tracking_info(branch, &num_ours, &num_theirs); + if (stat_tracking_info(branch, &num_ours, + &num_theirs) != 1) + continue; + if (!num_ours && !num_theirs) v->s = ""; else if (!num_ours) { @@ -735,7 +738,11 @@ static void populate_value(struct refinfo *ref) } else if (!strcmp(formatp, "trackshort") && starts_with(name, "upstream")) { assert(branch); - stat_tracking_info(branch, &num_ours, &num_theirs); + + if (stat_tracking_info(branch, &num_ours, + &num_theirs) != 1) + continue; + if (!num_ours && !num_theirs) v->s = "="; else if (!num_ours) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index bda354c..dcee7a0 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -335,6 +335,20 @@ test_expect_success 'Check that :track[short] cannot be used with other atoms' ' ' cat >expected <<EOF + + +EOF + +test_expect_success 'Check that :track[short] works when upstream is invalid' ' + test_when_finished "git config branch.master.merge refs/heads/master" && + git config branch.master.merge refs/heads/does-not-exist && + git for-each-ref \ + --format="%(upstream:track)$LF%(upstream:trackshort)" \ + refs/heads > actual && + test_cmp expected actual +' + +cat >expected <<EOF $(git rev-parse --short HEAD) EOF
-- 2.1.4
