Re: [PATCH v27 2/2] status: add status.compareBranches config for multiple branch comparisons

From: Jeff King <peff@peff.net>
Date: Jan 22, 2026, 22:01 UTC
Message-ID: <20260122220154.GA2107958@coredump.intra.peff.net>
In-Reply-To: <xmqq5x8tmlj7.fsf@gitster.g>

On Thu, Jan 22, 2026 at 12:37:16PM -0800, Junio C Hamano wrote:

Show 18 quoted lines

> > +static char *resolve_compare_branch(struct branch *branch, const char *name)
> > +{
> > +	struct strbuf buf = STRBUF_INIT;
> > +	const char *resolved = NULL;
> > +	char *ret;
> > +
> > +	if (!branch || !name)
> > +		return NULL;
> > +
> > +	if (!strcasecmp(name, "@{upstream}") || !strcasecmp(name, "@{u}"))
> > +		resolved = branch_get_upstream(branch, NULL);
> > +	else if (!strcasecmp(name, "@{push}"))
> > +		resolved = branch_get_push(branch, NULL);
> 
> OK.  Usually @{upstream} without anything before the at-sign means
> the upstream of the current branch, but we need to force pretend
> that branch were the current branch, so we'd need to special case
> like this, which looks reasonable.

Yeah, it is unfortunate to have to special-case these names, even though the resolving functions already know about them. If we are looking at branch "foo" we could rewrite them as "foo@{upstream}", etc, but that also requires special-casing (though maybe slightly less, if we just accept the @ sign?).

I can think of two alternatives, though.

One is that repo_interpret_branch_name() could accept a field in its options struct to set the default branch (rather than "HEAD"). Something like this (totally untested):

diff --git a/object-name.c b/object-name.c
index 8b862c124e..925a487d84 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1732,7 +1732,7 @@ static int interpret_branch_mark(struct repository *r,
 		branch = branch_get(name_str);
 		free(name_str);
 	} else
-		branch = branch_get(NULL);
+		branch = branch_get(options->default_branch);
 
 	value = get_data(branch, &err);
 	if (!value) {
diff --git a/object-name.h b/object-name.h
index cda4934cd5..962f99b0f6 100644
--- a/object-name.h
+++ b/object-name.h
@@ -119,6 +119,12 @@ struct interpret_branch_name_options {
 	 * of die()-ing.
 	 */
 	unsigned nonfatal_dangling_mark : 1;
+
+	/*
+	 * Pass this to branch_get() when interpreting @-marks without a
+	 * branch, rather than using HEAD.
+	 */
+	const char *default_branch;
 };
 int repo_interpret_branch_name(struct repository *r,
 			       const char *str, int len,

Most callers would continue to pass NULL in the usual way, but the
resolution here would pass in the current branch name.


The second is a bit more complicated, but is even more flexible. Part of
the point of this status.compareBranches approach is that you can add
regular refnames to the list. But would a user want to use a name that
is derived from the comparison branch, rather than just a static name?
That is, to compare branch "foo" against "origin/foo"? Usually that is
exactly the kind of refspec-application that @{upstream} and @{push} are
computing (after taking into account various config). But if you have a
third source of refs, would you want to be able to compare to
"origin/%s", where %s is the shortened branch name?

In which case these values could become "%s@{upstream}" and "%s@{push}",
and they could just be fed straight to the branch-interpret machinery.

> > +	strbuf_addf(&buf, "refs/remotes/%s", name);
> > +	resolved = refs_resolve_ref_unsafe(
> > +		get_main_ref_store(the_repository),
> > +		buf.buf,
> > +		RESOLVE_REF_READING,
> > +		NULL, NULL);
> > +	if (resolved) {
> > +		ret = xstrdup(resolved);
> > +		strbuf_release(&buf);
> > +		return ret;
> > +	}
> 
> It would be handy to be able to say "origin/master" (or even just
> "origin", which is interpreted as "origin/HEAD" via the DWIM
> machinery) and prepending of "refs/remotes/" above does help such
> DWIMmery, but I wonder if it is too limiting?  Would there be
> situations where you would want to compare with something outside
> refs/remotes/ hierarchy?
> 
> For example, writing "v2.52.0" there to see how far we came since
> the last release would become impossible if we always force prepend
> "refs/remotes/".  I wonder if we can reuse already existing DWIMmery
> that uses refs.c::ref_rev_parse_rules[], which should allow such use
> case, while still allowing you to write "origin/master"?

Yeah, I think tags or even local branches would be plausible candidates.
But at any rate, I'd expect these to be resolved in the "usual" way that
we do elsewhere. If we switch to using repo_dwim_ref() or something that
interprets @-marks, then that matches nicely with the suggestions I gave
above.

-Peff