[PATCH] status: reword message for a missing upstream branch

[PATCH] status: reword message for a missing upstream branch

[Elia Pinto]

Clone an empty repository, make a commit and run "git status":

    $ git init --bare empty.git
    $ git clone empty.git work && cd work
    $ git commit --allow-empty -m initial
    $ git status
    On branch master
    Your branch is based on 'origin/master', but the upstream is gone.
      (use "git branch --unset-upstream" to fixup)

The upstream is not gone, it was never there: the remote is empty,
so origin/master has not been created yet. The advice is also not
what the user needs; "git push" creates origin/master and the
message goes away.

The message was added in f2e087395b (branch: report invalid tracking
branch as gone, 2013-08-26), assuming that a configured upstream that
cannot be resolved must have been deleted. But "git clone" of an empty
repository configures the upstream anyway, and Git has no way to tell
"deleted" from "not created yet".

Since we cannot tell the two cases apart, change the message for both,
including the "deleted" case that has been reported as "gone" since
2013, so that it is correct either way:

    Your branch is set up to track 'origin/master', which does not exist.
      (use "git push" to create it)
      (use "git branch --unset-upstream" to stop tracking it)

Only suggest "git push" when it would actually create the upstream,
that is when the push destination of the branch (what "@{push}"
resolves to) is the upstream itself. This is not the case, for
example, with push.default set to "nothing", or when pushing to a
different remote (branch.<name>.pushRemote or remote.pushDefault).
It is also not the case with push.default set to "matching": it only
pushes branches that already exist on the remote, so it would not
create the missing upstream. In all these cases only suggest
"git branch --unset-upstream".

"git checkout" prints the same message, so it changes too. The
"[gone]" shown by "git status -sb", "git branch -vv" and
"%(upstream:track)" is not changed, as scripts may depend on it.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
This is an RFC mainly about the wording. The case of a freshly cloned
empty repository has been confusing users for a while, e.g.
https://stackoverflow.com/questions/24870145

I could not find a previous discussion of this case on the list.
Opinions on whether "status:" or "remote:" is the better area prefix
are also welcome, as "git checkout" shows the same message.

This is a resubmission of this patch, which I haven't seen in 
'seen' or 'next' https://lore.kernel.org/git/20261009134004.188952-1-gitter.spiros@gmail.com/
Thanks 

 remote.c                 |  34 +++++++++--
 t/t6040-tracking-info.sh | 120 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 146 insertions(+), 8 deletions(-)

diff --git a/remote.c b/remote.c
index 71170f36a9..4ed4d1ee2f 100644
--- a/remote.c
+++ b/remote.c
@@ -2442,6 +2442,31 @@ static void format_branch_comparison(struct strbuf *sb,
 	}
 }
 
+/*
+ * Would a plain "git push" create the remote-tracking ref "upstream"?
+ * Not with "matching", which only updates branches that already exist
+ * on the remote.
+ */
+static bool push_creates(const char *push_ref, const char *upstream)
+{
+	return push_ref && !strcmp(push_ref, upstream) &&
+	       repo_config_values(the_repository)->push_default !=
+	       PUSH_DEFAULT_MATCHING;
+}
+
+static void format_missing_upstream(struct strbuf *sb, const char *name,
+				    bool suggest_push)
+{
+	strbuf_addf(sb, _("Your branch is set up to track '%s', "
+			  "which does not exist.\n"), name);
+	if (!advice_enabled(ADVICE_STATUS_HINTS))
+		return;
+	if (suggest_push)
+		strbuf_addstr(sb, _("  (use \"git push\" to create it)\n"));
+	strbuf_addstr(sb, _("  (use \"git branch --unset-upstream\" "
+			    "to stop tracking it)\n"));
+}
+
 /*
  * Return true when there is anything to report, otherwise false.
  */
@@ -2503,12 +2528,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 
 		if (cmp < 0) {
 			if (is_upstream) {
-				strbuf_addf(sb,
-					_("Your branch is based on '%s', but the upstream is gone.\n"),
-					short_ref);
-				if (advice_enabled(ADVICE_STATUS_HINTS))
-					strbuf_addstr(sb,
-						_("  (use \"git branch --unset-upstream\" to fixup)\n"));
+				format_missing_upstream(sb, short_ref,
+							push_creates(push_ref,
+								     full_ref));
 				reported = 1;
 			}
 			free(full_ref);
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index e95d420972..39cb755ccc 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -96,7 +96,7 @@ test_expect_success 'checkout (upstream is gone)' '
 		cd test &&
 		git checkout b5
 	) >actual &&
-	test_grep "is based on .*, but the upstream is gone." actual
+	test_grep "set up to track .*, which does not exist." actual
 '
 
 test_expect_success 'checkout (up-to-date with upstream)' '
@@ -123,7 +123,123 @@ test_expect_success 'status (upstream is gone)' '
 		# reports nothing to commit
 		test_must_fail git commit --dry-run
 	) >actual &&
-	test_grep "is based on .*, but the upstream is gone." actual
+	test_grep "set up to track .*, which does not exist." actual
+'
+
+test_expect_success 'setup clone of an empty repository' '
+	git init --bare empty.git &&
+	git clone empty.git empty-clone &&
+	git -C empty-clone commit --allow-empty -m initial
+'
+
+test_expect_success 'status (upstream not yet pushed after empty clone)' '
+	cat >expect <<-EOF &&
+	On branch main
+	Your branch is set up to track ${SQ}origin/main${SQ}, which does not exist.
+	  (use "git push" to create it)
+	  (use "git branch --unset-upstream" to stop tracking it)
+
+	nothing to commit, working tree clean
+	EOF
+	git -C empty-clone status >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'checkout (upstream not yet pushed after empty clone)' '
+	test_when_finished "git -C empty-clone branch -D tmp" &&
+	cat >expect <<-EOF &&
+	Your branch is set up to track ${SQ}origin/main${SQ}, which does not exist.
+	  (use "git push" to create it)
+	  (use "git branch --unset-upstream" to stop tracking it)
+	EOF
+	git -C empty-clone checkout -b tmp &&
+	git -C empty-clone checkout main >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'status -s -b (upstream not yet pushed after empty clone)' '
+	echo "## main...origin/main [gone]" >expect &&
+	git -C empty-clone status -s -b >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'setup expected output without "git push" advice' '
+	cat >expect-no-push <<-EOF
+	On branch main
+	Your branch is set up to track ${SQ}origin/main${SQ}, which does not exist.
+	  (use "git branch --unset-upstream" to stop tracking it)
+
+	nothing to commit, working tree clean
+	EOF
+'
+
+test_expect_success 'status (missing upstream, push.default=nothing)' '
+	git -C empty-clone -c push.default=nothing status >actual &&
+	test_cmp expect-no-push actual
+'
+
+# "matching" only updates branches that already exist on the remote
+test_expect_success 'status (missing upstream, push.default=matching)' '
+	git -C empty-clone -c push.default=matching status >actual &&
+	test_cmp expect-no-push actual
+'
+
+test_expect_success 'status (missing upstream, pushRemote is another remote)' '
+	git init --bare fork.git &&
+	git -C empty-clone remote add fork ../fork.git &&
+	git -C empty-clone -c branch.main.pushRemote=fork status >actual &&
+	test_cmp expect-no-push actual
+'
+
+test_expect_success 'status (missing upstream, advice.statusHints=false)' '
+	cat >expect <<-EOF &&
+	On branch main
+	Your branch is set up to track ${SQ}origin/main${SQ}, which does not exist.
+
+	nothing to commit, working tree clean
+	EOF
+	git -C empty-clone -c advice.statusHints=false status >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'setup upstream deleted on the remote and pruned' '
+	git init --bare pruned.git &&
+	git clone pruned.git pruned-clone &&
+	(
+		cd pruned-clone &&
+		git commit --allow-empty -m initial &&
+		git push origin main &&
+		git checkout -b topic &&
+		git push -u origin topic
+	) &&
+	git -C pruned.git branch -D topic &&
+	git -C pruned-clone fetch --prune
+'
+
+test_expect_success 'status (upstream deleted on the remote and pruned)' '
+	cat >expect <<-EOF &&
+	On branch topic
+	Your branch is set up to track ${SQ}origin/topic${SQ}, which does not exist.
+	  (use "git push" to create it)
+	  (use "git branch --unset-upstream" to stop tracking it)
+
+	nothing to commit, working tree clean
+	EOF
+	git -C pruned-clone status >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'short formats still show "gone" for a deleted upstream' '
+	echo "## topic...origin/topic [gone]" >expect &&
+	git -C pruned-clone status -s -b >actual &&
+	test_cmp expect actual &&
+	git -C pruned-clone branch -vv >actual &&
+	test_grep "^\* topic .* \[origin/topic: gone\] initial$" actual &&
+	echo "topic [gone]" >expect &&
+	git -C pruned-clone for-each-ref \
+		--format="%(refname:short) %(upstream:track)" \
+		refs/heads/topic >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'status (up-to-date with upstream)' '
-- 
2.53.0

Re: [PATCH] status: reword message for a missing upstream branch

[Harald Nordgren]

Hi Elia!

Is this a duplicate, seems you sent out the same thing here already?
https://lore.kernel.org/git/20261009134004.188952-1-gitter.spiros@gmail.com/


Harald

Re: [PATCH] status: reword message for a missing upstream branch

[Junio C Hamano]

Elia Pinto <gitter.spiros@gmail.com> writes:

> The message was added in f2e087395b (branch: report invalid tracking
> branch as gone, 2013-08-26), assuming that a configured upstream that
> cannot be resolved must have been deleted. But "git clone" of an empty
> repository configures the upstream anyway, and Git has no way to tell
> "deleted" from "not created yet".

I am totally with you up to this point.  This is a great issue to
tackle.  I am curious to learn how you would solve it without
throwing the baby with bathwater, though.  Let me read on.

> Since we cannot tell the two cases apart, change the message for both,
> including the "deleted" case that has been reported as "gone" since
> 2013, so that it is correct either way:
>
>     Your branch is set up to track 'origin/master', which does not exist.
>       (use "git push" to create it)
>       (use "git branch --unset-upstream" to stop tracking it)

Ahh, "does not exist".  Clever.  That does cover both cases and
reads reasonably natural.  If we wanted to have something short that
we can reuse elsewhere (like inside [] in place of [gone]), we could
say which is "missing", perhaps.

Do we emit "git push" suggestion unconditionally (meaning, does "git
push" immediately in this state always create the "upstream" that
does not exist yet), or do we have to check if "git push" will
create it before saying so?

> Only suggest "git push" when it would actually create the upstream,
> that is when the push destination of the branch (what "@{push}"
> resolves to) is the upstream itself.

Ah, your flow of explanation exactly matches how I explore the
possibilities in my head, given the thinking of the author revealed
so far.  Excellent log message that is very easy to follow.

> This is not the case, for
> example, with push.default set to "nothing", or when pushing to a
> different remote (branch.<name>.pushRemote or remote.pushDefault).
> It is also not the case with push.default set to "matching": it only
> pushes branches that already exist on the remote, so it would not
> create the missing upstream. In all these cases only suggest
> "git branch --unset-upstream".

Indeed.  That sounds like a good way to go.

> "git checkout" prints the same message, so it changes too. The
> "[gone]" shown by "git status -sb", "git branch -vv" and
> "%(upstream:track)" is not changed, as scripts may depend on it.

Does this refer to what wt_shortstatus_print_tracking() says?  It is
_(translated), so in a sense it is a fair game to be improved.

Re: [PATCH] status: reword message for a missing upstream branch

[Elia Pinto]

Junio C Hamano <gitster@pobox.com> writes:

> Ahh, "does not exist".  Clever.  That does cover both cases and
> reads reasonably natural.  If we wanted to have something short that
> we can reuse elsewhere (like inside [] in place of [gone]), we could
> say which is "missing", perhaps.

"missing" sounds good to me for the short form. I'd rather keep it
out of this patch, though; see below for why the bracketed label
needs a bit more care than the long message.

> Do we emit "git push" suggestion unconditionally (meaning, does "git
> push" immediately in this state always create the "upstream" that
> does not exist yet), or do we have to check if "git push" will
> create it before saying so?

We check, as the next paragraph of the log says: the hint is given
only when @{push} resolves to the upstream itself, and push.default
is not "matching".

The check errs on the side of not giving the hint. For example, with
push.default=matching but an explicit remote.<name>.push refspec,
"git push" uses the refspec and would create the upstream, but we
still do not suggest it, because push_creates() looks at push.default
without checking whether a refspec overrides it. I'll fix that in v2
by checking push.default only when the remote has no push refspec
and is not a mirror, which is the same order branch_get_push_1()
follows.

> Does this refer to what wt_shortstatus_print_tracking() says?  It is
> _(translated), so in a sense it is a fair game to be improved.

Yes, and you are right, the justification in the log message is
wrong for some of the commands it lists. Looking more closely:

 - "git status -sb" uses LABEL(N_("gone")) in
   wt_shortstatus_print_tracking(), so it is translated (e.g.
   "[sparito]" in Italian).

 - "git status --porcelain -b" goes through the same function, but
   wt_porcelain_print() sets s->no_gettext, so it always says
   "[gone]". That one has to stay.

 - "git status --porcelain=v2" never says "gone"; it prints
   "# branch.upstream" and omits the "# branch.ab" line.

 - "%(upstream:track)" in "git for-each-ref" uses the untranslated
   "gone" from ref-filter.c, which is documented in
   git-for-each-ref.adoc, so that one has to stay too. But "git
   branch" (and "git tag") call setup_ref_filter_porcelain_msg(),
   so "git branch -vv" and "git branch --format=%(upstream:track)"
   show the translated label.

So "scripts may depend on it" is true only for --porcelain and
for-each-ref. The other two are human-readable output and could be
changed.

I am still hesitant to change the label in "git branch -vv" as part
of this patch. Translated or not, under LC_ALL=C it says "gone", and

    git branch -vv | grep ': gone]'

is a very common recipe for finding branches whose upstream was
pruned. To keep the porcelain output as "gone" while changing the
human output, wt_shortstatus_print_tracking() also could not keep
using LABEL() with a single msgid.

So for v2 I plan to leave the bracketed label alone and correct the
log message to say what is really going on. If we want "[missing]"
in "git status -sb" and "git branch -vv", I'd do it as a separate
follow-up patch, where we can discuss the compatibility question
separately.

Thanks for the review.