* [PATCH 0/4] memory leaks in remote.c
@ 2026-01-19 5:18 Jeff King
2026-01-19 5:19 ` [PATCH 1/4] remote: return non-const pointer from error_buf() Jeff King
` (5 more replies)
0 siblings, 6 replies; 39+ messages in thread
From: Jeff King @ 2026-01-19 5:18 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren
This fixes some memory leaks in remote.c. Not urgent, as they are quite
old, but they are newly triggered in the test suite by Harald's
hn/status-compare-with-push topic. So I think we'd want to build that
topic on top of these.
The first two are just preparatory cleanups. Patch 3 fixes the leak that
Harald's series triggers (and adds its own test, of course). Patch 4 is
a hypothetical leak that I don't think can be triggered in practice (so
it's more of a cleanup).
[1/4]: remote: return non-const pointer from error_buf()
[2/4]: remote: drop const return of tracking_for_push_dest()
[3/4]: remote: fix leak in branch_get_push_1() with invalid "simple" config
[4/4]: remote: always allocate branch.push_tracking_ref
remote.c | 24 ++++++++++++++----------
remote.h | 2 +-
t/for-each-ref-tests.sh | 9 +++++++++
3 files changed, 24 insertions(+), 11 deletions(-)
-Peff
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH 1/4] remote: return non-const pointer from error_buf() 2026-01-19 5:18 [PATCH 0/4] memory leaks in remote.c Jeff King @ 2026-01-19 5:19 ` Jeff King 2026-01-19 6:33 ` Patrick Steinhardt 2026-01-19 5:20 ` [PATCH 2/4] remote: drop const return of tracking_for_push_dest() Jeff King ` (4 subsequent siblings) 5 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2026-01-19 5:19 UTC (permalink / raw) To: git; +Cc: Harald Nordgren We have an error_buf() helper that functions a bit like our error() helper, but returns NULL instead of -1. Its return type is "const char *", but this is overly restrictive. If we use the helper in a function that returns non-const "char *", the compiler will complain about the implicit cast from const to non-const. Meanwhile, the const in the helper is doing nothing useful, as it only ever returns NULL. Let's drop the const, which will let us use it in both types of function. Signed-off-by: Jeff King <peff@peff.net> --- remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote.c b/remote.c index b756ff6f15..3dc100be83 100644 --- a/remote.c +++ b/remote.c @@ -1831,7 +1831,7 @@ int branch_merge_matches(struct branch *branch, } __attribute__((format (printf,2,3))) -static const char *error_buf(struct strbuf *err, const char *fmt, ...) +static char *error_buf(struct strbuf *err, const char *fmt, ...) { if (err) { va_list ap; -- 2.53.0.rc0.338.g08aa8a9473 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] remote: return non-const pointer from error_buf() 2026-01-19 5:19 ` [PATCH 1/4] remote: return non-const pointer from error_buf() Jeff King @ 2026-01-19 6:33 ` Patrick Steinhardt 2026-01-20 0:28 ` Junio C Hamano 0 siblings, 1 reply; 39+ messages in thread From: Patrick Steinhardt @ 2026-01-19 6:33 UTC (permalink / raw) To: Jeff King; +Cc: git, Harald Nordgren On Mon, Jan 19, 2026 at 12:19:45AM -0500, Jeff King wrote: > We have an error_buf() helper that functions a bit like our error() > helper, but returns NULL instead of -1. Its return type is "const char > *", but this is overly restrictive. If we use the helper in a function > that returns non-const "char *", the compiler will complain about > the implicit cast from const to non-const. > > Meanwhile, the const in the helper is doing nothing useful, as it only > ever returns NULL. Let's drop the const, which will let us use it in > both types of function. This function signature is indeed quite misleading, and I'd argue that it continues to be so even after the change. I guess the intent is to make it a bit easier to print an error in functions that return a string. I'm not really a huge fan of this, but it's not a fault of this patch series, so let's read on. Patrick ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] remote: return non-const pointer from error_buf() 2026-01-19 6:33 ` Patrick Steinhardt @ 2026-01-20 0:28 ` Junio C Hamano 2026-01-20 19:38 ` Jeff King 0 siblings, 1 reply; 39+ messages in thread From: Junio C Hamano @ 2026-01-20 0:28 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: Jeff King, git, Harald Nordgren Patrick Steinhardt <ps@pks.im> writes: > This function signature is indeed quite misleading, and I'd argue that > it continues to be so even after the change. I guess the intent is to > make it a bit easier to print an error in functions that return a > string. > > I'm not really a huge fan of this, but it's not a fault of this patch > series, so let's read on. I concur. "If they do not return any useful value, they should be void" was my first reaction, but presumably just like "return error("message");" is a handy way to give message while signalling an error to the caller, these are used to return NULL that signals an error? I do not offhand think of a good longer-term direction to improve this one. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] remote: return non-const pointer from error_buf() 2026-01-20 0:28 ` Junio C Hamano @ 2026-01-20 19:38 ` Jeff King 2026-01-20 20:18 ` Junio C Hamano 0 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2026-01-20 19:38 UTC (permalink / raw) To: Junio C Hamano; +Cc: Patrick Steinhardt, git, Harald Nordgren On Mon, Jan 19, 2026 at 04:28:42PM -0800, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > This function signature is indeed quite misleading, and I'd argue that > > it continues to be so even after the change. I guess the intent is to > > make it a bit easier to print an error in functions that return a > > string. > > > > I'm not really a huge fan of this, but it's not a fault of this patch > > series, so let's read on. > > I concur. "If they do not return any useful value, they should be > void" was my first reaction, but presumably just like "return > error("message");" is a handy way to give message while signalling > an error to the caller, these are used to return NULL that signals > an error? I do not offhand think of a good longer-term direction to > improve this one. Yes, that's exactly the purpose. I don't see many changes that could let it still fulfill that purpose, though perhaps one could argue that it is unnecessarily confusing for the small shortening of the code it provides (and ditto for error() itself). There is one thing it probably could do: return a "void *" instead. That would make it applicable to a wider variety of functions. But it also makes it even more obscure (IMHO), and this is a static-local function that is only used for functions that return strings anyway. If we wanted to make it more generic (and I do not think we want to), we can see that it differs from error() in two dimensions: - error() writes to stderr, but error_buf() writes to a strbuf (or nowhere if the strbuf is NULL) - error() passes along integer "-1" to signal error, but error_buf() passes along NULL So of the four combinations, we have: - stderr / integer: error() - stderr / pointer: not implemented - strbuf / integer: not implemented - strbuf / pointer: error_buf() One could imagine a suite of related functions: error_int(), error_null(), error_buf_int(), error_buf_null() that provide all four. But I do not see us clamoring to extend the pattern further. ;) -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] remote: return non-const pointer from error_buf() 2026-01-20 19:38 ` Jeff King @ 2026-01-20 20:18 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2026-01-20 20:18 UTC (permalink / raw) To: Jeff King; +Cc: Patrick Steinhardt, git, Harald Nordgren Jeff King <peff@peff.net> writes: > On Mon, Jan 19, 2026 at 04:28:42PM -0800, Junio C Hamano wrote: > >> Patrick Steinhardt <ps@pks.im> writes: >> >> > This function signature is indeed quite misleading, and I'd argue that >> > it continues to be so even after the change. I guess the intent is to >> > make it a bit easier to print an error in functions that return a >> > string. >> > >> > I'm not really a huge fan of this, but it's not a fault of this patch >> > series, so let's read on. >> >> I concur. "If they do not return any useful value, they should be >> void" was my first reaction, but presumably just like "return >> error("message");" is a handy way to give message while signalling >> an error to the caller, these are used to return NULL that signals >> an error? I do not offhand think of a good longer-term direction to >> improve this one. > > Yes, that's exactly the purpose. I don't see many changes that could > let it still fulfill that purpose, though perhaps one could argue that > it is unnecessarily confusing for the small shortening of the code it > provides (and ditto for error() itself). > > There is one thing it probably could do: return a "void *" instead. That > would make it applicable to a wider variety of functions. But it also > makes it even more obscure (IMHO), and this is a static-local function > that is only used for functions that return strings anyway. > > If we wanted to make it more generic (and I do not think we want to), we > can see that it differs from error() in two dimensions: > > - error() writes to stderr, but error_buf() writes to a strbuf (or > nowhere if the strbuf is NULL) > > - error() passes along integer "-1" to signal error, but error_buf() > passes along NULL > > So of the four combinations, we have: > > - stderr / integer: error() > - stderr / pointer: not implemented > - strbuf / integer: not implemented > - strbuf / pointer: error_buf() > > One could imagine a suite of related functions: error_int(), > error_null(), error_buf_int(), error_buf_null() that provide all four. > > But I do not see us clamoring to extend the pattern further. ;) ;-). Perhaps stop being cute and doing if (... error ...) { format_error(err, _("error message"), ...); return NULL; } without any magic might be more appropriate for a file-scope static that is only used for a handful of times? I do not think the situation is bad enough to warrant patch noise like this, but it is sufficiently bad that I wish we wrote them in such a more trivial way in the first place X-<. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 2/4] remote: drop const return of tracking_for_push_dest() 2026-01-19 5:18 [PATCH 0/4] memory leaks in remote.c Jeff King 2026-01-19 5:19 ` [PATCH 1/4] remote: return non-const pointer from error_buf() Jeff King @ 2026-01-19 5:20 ` Jeff King 2026-01-19 6:34 ` Patrick Steinhardt 2026-01-19 5:22 ` [PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config Jeff King ` (3 subsequent siblings) 5 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2026-01-19 5:20 UTC (permalink / raw) To: git; +Cc: Harald Nordgren The string returned from tracking_for_push_dest() comes from apply_refspec(), and thus is always an allocated string (or NULL). We should return a non-const pointer so that the caller knows that ownership of the string is being transferred. This goes back to the function's origin in e291c75a95 (remote.c: add branch_get_push, 2015-05-21). It never really mattered because our return is just forwarded through branch_get_push_1(), which returns a const string as part of an intentionally hacky memory management scheme (see that commit for details). As the first step of untangling that hackery, let's drop the extra const from this helper function (and from the variables that store its result). There should be no functional change (yet). Signed-off-by: Jeff King <peff@peff.net> --- remote.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/remote.c b/remote.c index 3dc100be83..5de9619bc7 100644 --- a/remote.c +++ b/remote.c @@ -1869,9 +1869,9 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err) return branch->merge[0]->dst; } -static const char *tracking_for_push_dest(struct remote *remote, - const char *refname, - struct strbuf *err) +static char *tracking_for_push_dest(struct remote *remote, + const char *refname, + struct strbuf *err) { char *ret; @@ -1899,7 +1899,7 @@ static const char *branch_get_push_1(struct repository *repo, if (remote->push.nr) { char *dst; - const char *ret; + char *ret; dst = apply_refspecs(&remote->push, branch->refname); if (!dst) @@ -1929,7 +1929,8 @@ static const char *branch_get_push_1(struct repository *repo, case PUSH_DEFAULT_UNSPECIFIED: case PUSH_DEFAULT_SIMPLE: { - const char *up, *cur; + const char *up; + char *cur; up = branch_get_upstream(branch, err); if (!up) -- 2.53.0.rc0.338.g08aa8a9473 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 2/4] remote: drop const return of tracking_for_push_dest() 2026-01-19 5:20 ` [PATCH 2/4] remote: drop const return of tracking_for_push_dest() Jeff King @ 2026-01-19 6:34 ` Patrick Steinhardt 0 siblings, 0 replies; 39+ messages in thread From: Patrick Steinhardt @ 2026-01-19 6:34 UTC (permalink / raw) To: Jeff King; +Cc: git, Harald Nordgren On Mon, Jan 19, 2026 at 12:20:26AM -0500, Jeff King wrote: > The string returned from tracking_for_push_dest() comes from > apply_refspec(), and thus is always an allocated string (or NULL). We > should return a non-const pointer so that the caller knows that > ownership of the string is being transferred. > > This goes back to the function's origin in e291c75a95 (remote.c: add > branch_get_push, 2015-05-21). It never really mattered because our > return is just forwarded through branch_get_push_1(), which returns a > const string as part of an intentionally hacky memory management scheme > (see that commit for details). Okay, so here we can now also return a `char *` now that `error_buf()` got adapted. > As the first step of untangling that hackery, let's drop the extra const > from this helper function (and from the variables that store its > result). There should be no functional change (yet). Yup. The memory handling still feels weird, but as in the preceding commit that's not a fault of this patch series. Patrick ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config 2026-01-19 5:18 [PATCH 0/4] memory leaks in remote.c Jeff King 2026-01-19 5:19 ` [PATCH 1/4] remote: return non-const pointer from error_buf() Jeff King 2026-01-19 5:20 ` [PATCH 2/4] remote: drop const return of tracking_for_push_dest() Jeff King @ 2026-01-19 5:22 ` Jeff King 2026-01-19 6:34 ` Patrick Steinhardt 2026-01-19 5:23 ` [PATCH 4/4] remote: always allocate branch.push_tracking_ref Jeff King ` (2 subsequent siblings) 5 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2026-01-19 5:22 UTC (permalink / raw) To: git; +Cc: Harald Nordgren Most of the code paths in branch_get_push_1() allocate a string for the @{push} value. We then return the result, which is stored in a "struct branch", so the value is not leaked. But there's one path that does leak: when we are in the "simple" push mode, we have to check that the @{push} value matches what we'd get for @{upstream}. If it doesn't, we return an error, but forget to free the @{push} value we computed. Curiously, the existing tests don't trigger this with LSan, even though they do exercise the code path. As far as I can tell, it should be triggered via: git -c push.default=simple \ -c branch.foo.remote=origin \ -c branch.foo.merge=refs/heads/not-foo \ rev-parse foo@{push} which will complain that the upstream ("not-foo") does not match the push destination ("foo"). We do die() shortly after this, but not until after returning from branch_get_push_1(), which is where the leak happens. So it seems like a false negative in LSan. However, I can trigger it reliably by printing the @{push} value using for-each-ref. This takes a little more setup (because we need "foo" to actually exist to iterate over it with for-each-ref), but we can piggy-back on the existing repo config in t6300. Signed-off-by: Jeff King <peff@peff.net> --- remote.c | 4 +++- t/for-each-ref-tests.sh | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/remote.c b/remote.c index 5de9619bc7..e191b0ff6e 100644 --- a/remote.c +++ b/remote.c @@ -1938,9 +1938,11 @@ static const char *branch_get_push_1(struct repository *repo, cur = tracking_for_push_dest(remote, branch->refname, err); if (!cur) return NULL; - if (strcmp(cur, up)) + if (strcmp(cur, up)) { + free(cur); return error_buf(err, _("cannot resolve 'simple' push to a single destination")); + } return cur; } } diff --git a/t/for-each-ref-tests.sh b/t/for-each-ref-tests.sh index 4593be5fd5..bd2d45c971 100644 --- a/t/for-each-ref-tests.sh +++ b/t/for-each-ref-tests.sh @@ -1744,6 +1744,15 @@ test_expect_success ':remotename and :remoteref' ' ) ' +test_expect_success '%(push) with an invalid push-simple config' ' + echo "refs/heads/main " >expect && + git -c push.default=simple \ + -c remote.pushdefault=myfork \ + for-each-ref \ + --format="%(refname) %(push)" refs/heads/main >actual && + test_cmp expect actual +' + test_expect_success "${git_for_each_ref} --ignore-case ignores case" ' ${git_for_each_ref} --format="%(refname)" refs/heads/MAIN >actual && test_must_be_empty actual && -- 2.53.0.rc0.338.g08aa8a9473 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config 2026-01-19 5:22 ` [PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config Jeff King @ 2026-01-19 6:34 ` Patrick Steinhardt 0 siblings, 0 replies; 39+ messages in thread From: Patrick Steinhardt @ 2026-01-19 6:34 UTC (permalink / raw) To: Jeff King; +Cc: git, Harald Nordgren On Mon, Jan 19, 2026 at 12:22:08AM -0500, Jeff King wrote: > diff --git a/remote.c b/remote.c > index 5de9619bc7..e191b0ff6e 100644 > --- a/remote.c > +++ b/remote.c > @@ -1938,9 +1938,11 @@ static const char *branch_get_push_1(struct repository *repo, > cur = tracking_for_push_dest(remote, branch->refname, err); > if (!cur) > return NULL; > - if (strcmp(cur, up)) > + if (strcmp(cur, up)) { > + free(cur); > return error_buf(err, > _("cannot resolve 'simple' push to a single destination")); > + } Yup, this memory leak was easy to spot in the preceding commit after your refactorings. Patrick ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 4/4] remote: always allocate branch.push_tracking_ref 2026-01-19 5:18 [PATCH 0/4] memory leaks in remote.c Jeff King ` (2 preceding siblings ...) 2026-01-19 5:22 ` [PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config Jeff King @ 2026-01-19 5:23 ` Jeff King 2026-01-19 6:34 ` Patrick Steinhardt 2026-01-19 15:04 ` Triangular workflow Harald Nordgren 2026-01-20 0:31 ` [PATCH 0/4] memory leaks in remote.c Junio C Hamano 5 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2026-01-19 5:23 UTC (permalink / raw) To: git; +Cc: Harald Nordgren In branch_get_push(), we usually allocate a new string for the @{push} ref, but will not do so in push.default=upstream mode, where we just pass back the result of branch_get_upstream() directly. This led to a hacky memory management scheme in e291c75a95 (remote.c: add branch_get_push, 2015-05-21): we store the result in the push_tracking_ref field of a "struct branch", under the assumption that the branch struct will last until the end of the program. So even though the struct doesn't know if it has an allocated string or not, it doesn't matter because we hold on to it either way. But that assumption was violated by f5ccb535cc (remote: fix leaking config strings, 2024-08-22), which added a function to free branch structs. Any struct which is fed to branch_release() is at risk of leaking its push_tracking_ref member. I don't think this can actually be triggered in practice. We rarely actually free the branch structs, and we only fill in the push_tracking_ref string lazily when it is needed. So triggering the leak would require a code path that does both, and I couldn't find one. Still, this is an ugly trap that may eventually spring on us. Since there is only one code path in branch_get_push() that doesn't allocate, let's just have it copy the string. And then we know that push_tracking_ref is always allocated, and we can free it in branch_release(). Signed-off-by: Jeff King <peff@peff.net> --- remote.c | 7 ++++--- remote.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/remote.c b/remote.c index e191b0ff6e..3e9d9b3e1f 100644 --- a/remote.c +++ b/remote.c @@ -272,6 +272,7 @@ static void branch_release(struct branch *branch) free((char *)branch->refname); free(branch->remote_name); free(branch->pushremote_name); + free(branch->push_tracking_ref); merge_clear(branch); } @@ -1883,8 +1884,8 @@ static char *tracking_for_push_dest(struct remote *remote, return ret; } -static const char *branch_get_push_1(struct repository *repo, - struct branch *branch, struct strbuf *err) +static char *branch_get_push_1(struct repository *repo, + struct branch *branch, struct strbuf *err) { struct remote_state *remote_state = repo->remote_state; struct remote *remote; @@ -1924,7 +1925,7 @@ static const char *branch_get_push_1(struct repository *repo, return tracking_for_push_dest(remote, branch->refname, err); case PUSH_DEFAULT_UPSTREAM: - return branch_get_upstream(branch, err); + return xstrdup_or_null(branch_get_upstream(branch, err)); case PUSH_DEFAULT_UNSPECIFIED: case PUSH_DEFAULT_SIMPLE: diff --git a/remote.h b/remote.h index 0ca399e183..fc052945ee 100644 --- a/remote.h +++ b/remote.h @@ -331,7 +331,7 @@ struct branch { int merge_alloc; - const char *push_tracking_ref; + char *push_tracking_ref; }; struct branch *branch_get(const char *name); -- 2.53.0.rc0.338.g08aa8a9473 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 4/4] remote: always allocate branch.push_tracking_ref 2026-01-19 5:23 ` [PATCH 4/4] remote: always allocate branch.push_tracking_ref Jeff King @ 2026-01-19 6:34 ` Patrick Steinhardt 0 siblings, 0 replies; 39+ messages in thread From: Patrick Steinhardt @ 2026-01-19 6:34 UTC (permalink / raw) To: Jeff King; +Cc: git, Harald Nordgren On Mon, Jan 19, 2026 at 12:23:20AM -0500, Jeff King wrote: > diff --git a/remote.c b/remote.c > index e191b0ff6e..3e9d9b3e1f 100644 > --- a/remote.c > +++ b/remote.c > @@ -1924,7 +1925,7 @@ static const char *branch_get_push_1(struct repository *repo, > return tracking_for_push_dest(remote, branch->refname, err); > > case PUSH_DEFAULT_UPSTREAM: > - return branch_get_upstream(branch, err); > + return xstrdup_or_null(branch_get_upstream(branch, err)); > > case PUSH_DEFAULT_UNSPECIFIED: > case PUSH_DEFAULT_SIMPLE: Makes sense. I was wondering whether you'd also change `branch_get_push_1()` in a subsequent patch, so I'm happy to see this. This whole series looks good to me, thanks! Patrick ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-19 5:18 [PATCH 0/4] memory leaks in remote.c Jeff King ` (3 preceding siblings ...) 2026-01-19 5:23 ` [PATCH 4/4] remote: always allocate branch.push_tracking_ref Jeff King @ 2026-01-19 15:04 ` Harald Nordgren 2026-01-20 19:40 ` Jeff King 2026-01-20 0:31 ` [PATCH 0/4] memory leaks in remote.c Junio C Hamano 5 siblings, 1 reply; 39+ messages in thread From: Harald Nordgren @ 2026-01-19 15:04 UTC (permalink / raw) To: peff; +Cc: git, haraldnordgren Thanks a lot Jeff! Would be nice to get this merged ASAP, so I can continue the work on my feature without the memory leak there. Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-19 15:04 ` Triangular workflow Harald Nordgren @ 2026-01-20 19:40 ` Jeff King 0 siblings, 0 replies; 39+ messages in thread From: Jeff King @ 2026-01-20 19:40 UTC (permalink / raw) To: Harald Nordgren; +Cc: git On Mon, Jan 19, 2026 at 04:04:13PM +0100, Harald Nordgren wrote: > Would be nice to get this merged ASAP, so I can continue the work on my > feature without the memory leak there. I imagine it will not get merged until after the upcoming release. The usual thing is to build your branch on top in the meantime, though I don't offhand know how good GGG's support is for then sending only your patches (you might need to build your PR against the branch that Junio created when he picked up the topic, but that is only in gitster/git, not git/git). -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/4] memory leaks in remote.c 2026-01-19 5:18 [PATCH 0/4] memory leaks in remote.c Jeff King ` (4 preceding siblings ...) 2026-01-19 15:04 ` Triangular workflow Harald Nordgren @ 2026-01-20 0:31 ` Junio C Hamano 5 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2026-01-20 0:31 UTC (permalink / raw) To: Jeff King; +Cc: git, Harald Nordgren Jeff King <peff@peff.net> writes: > This fixes some memory leaks in remote.c. Not urgent, as they are quite > old, but they are newly triggered in the test suite by Harald's > hn/status-compare-with-push topic. So I think we'd want to build that > topic on top of these. > > The first two are just preparatory cleanups. Patch 3 fixes the leak that > Harald's series triggers (and adds its own test, of course). Patch 4 is > a hypothetical leak that I don't think can be triggered in practice (so > it's more of a cleanup). > > [1/4]: remote: return non-const pointer from error_buf() > [2/4]: remote: drop const return of tracking_for_push_dest() > [3/4]: remote: fix leak in branch_get_push_1() with invalid "simple" config > [4/4]: remote: always allocate branch.push_tracking_ref > > remote.c | 24 ++++++++++++++---------- > remote.h | 2 +- > t/for-each-ref-tests.sh | 9 +++++++++ > 3 files changed, 24 insertions(+), 11 deletions(-) All look sensible. Will queue. Thanks. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v25 2/2] status: show comparison with push remote tracking branch
@ 2026-01-13 17:03 Jeff King
2026-01-13 18:35 ` Triangular workflow Harald Nordgren
0 siblings, 1 reply; 39+ messages in thread
From: Jeff King @ 2026-01-13 17:03 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
On Tue, Jan 13, 2026 at 12:11:56PM +0000, Harald Nordgren via GitGitGadget wrote:
> Example output when tracking origin/main but push destination is
> origin/feature:
> On branch feature
> Your branch and 'origin/main' have diverged,
> and have 3 and 1 different commits each, respectively.
> (use "git pull" if you want to integrate the remote branch with yours)
>
> Your branch is ahead of 'origin/feature' by 1 commit.
> (use "git push" to publish your local commits)
Can we make this configurable?
I build my daily driver off of the 'jch' branch, which now includes this
series, and I've found that for my triangular workflow the ahead/behind
for the push branch is just useless noise. I treat my push destination
like a mirror, where I always just push up everything at the end of the
day.
I know that the output can be disabled with status.aheadbehind, but:
1. I noticed this first via "git checkout", which does not have such a
flag (AFAIK).
2. That flag would also disable the upstream branch ahead/behind
output, which I do find useful.
-Peff
^ permalink raw reply [flat|nested] 39+ messages in thread* Triangular workflow 2026-01-13 17:03 [PATCH v25 2/2] status: show comparison with push remote tracking branch Jeff King @ 2026-01-13 18:35 ` Harald Nordgren 2026-01-13 21:40 ` Jeff King 0 siblings, 1 reply; 39+ messages in thread From: Harald Nordgren @ 2026-01-13 18:35 UTC (permalink / raw) To: peff; +Cc: git, gitgitgadget, haraldnordgren > For my triangular workflow the ahead/behind for the push branch is just > useless noise. I treat my push destination like a mirror, where I always > just push up everything at the end of the day. This seems like a sub-optimal workflow 🤗 May I ask where you normally push (unless you only ever push once at the end) of the day? If it's another branch than your mirror, why not set that as your push destination then, and push to your push destination with the longer git push origin my-mirror It makes more sense to me to reserve the shorter and more convenient 'git push' for something you do many times a day. Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-13 18:35 ` Triangular workflow Harald Nordgren @ 2026-01-13 21:40 ` Jeff King 2026-01-13 23:01 ` Harald Nordgren 2026-01-14 18:54 ` Junio C Hamano 0 siblings, 2 replies; 39+ messages in thread From: Jeff King @ 2026-01-13 21:40 UTC (permalink / raw) To: Harald Nordgren; +Cc: git, gitgitgadget On Tue, Jan 13, 2026 at 07:35:57PM +0100, Harald Nordgren wrote: > > For my triangular workflow the ahead/behind for the push branch is just > > useless noise. I treat my push destination like a mirror, where I always > > just push up everything at the end of the day. > > This seems like a sub-optimal workflow 🤗 > > May I ask where you normally push (unless you only ever push once at the > end) of the day? If it's another branch than your mirror, why not set that > as your push destination then, and push to your push destination with the > longer > > git push origin my-mirror > > It makes more sense to me to reserve the shorter and more convenient > 'git push' for something you do many times a day. I couldn't quite parse what you meant by "another branch than your mirror", but I'll describe my workflow: 1. My "origin" remote is https://github.com/gitster/git. 2. Most branches have origin/master as their upstream, though occasionally I'll have dependent branches that use "." as their remote and the local base branch for the "branch" field. E.g. git checkout -b jk/some-topic origin/master ... git checkout -t -b jk/another-topic jk/some-topic 3. My mirror is https://github.com/peff/git, which I call "github" in the remote (arguably confusing, but back when I started that convention I was pulling Junio's tree from kernel.org ;) ). I point remote.pushdefault to the "github" remote. 4. My push.default setting is "current", so if I want to push up a single branch, I can just "git push". I hardly ever do that, though. 5. I usually push once per day or so. After integrating my topics into a shared daily-driver branch, I run something like: make test && make install && git push -f github refs/heads/jk/* (it's not exactly that; I have a script which picks out the local refs, but it's the moral equivalent of that). So I don't ever care about the relationship between a specific jk/foo and my mirror's version of it. I don't expect anybody to look at those, and they exist mostly for backup and CI purposes (though I don't run CI on the individual branches, but only the integrated result). And having the extra output from "git checkout" is just extra noise for me, especially because it is easy to see only the second message (which looks just like the upstream ahead/behind message, of course) and get confused. The first time I saw it I thought I had misconfigured something with my branch. -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-13 21:40 ` Jeff King @ 2026-01-13 23:01 ` Harald Nordgren 2026-01-14 2:22 ` D. Ben Knoble 2026-01-14 2:34 ` Jeff King 2026-01-14 18:54 ` Junio C Hamano 1 sibling, 2 replies; 39+ messages in thread From: Harald Nordgren @ 2026-01-13 23:01 UTC (permalink / raw) To: peff; +Cc: git, gitgitgadget, haraldnordgren Hi Jeff! I'm very happy that your responded respectfully despite me basically saying that you were using Git wrong. It's nice to see how some of the pros do it! I'm wondering if since you are scripting this anyway, if you really need a push branch at all? Can't you just as easily switch to doing this in the script: git config push.default upstream git push github jk/some-topic As a note, before I started working on this feature, I don't realize that there was such a thing as a push branch (i.e. something different from the tracking branch). So I had the habit of checking out and pushing like this: git branch --set-upstream-to upstream/master git push origin $(git rev-parse --abbrev-ref HEAD) I worked really well for me. The only issue was missing the status info from my own branch -- which is why I started writing this feature. Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-13 23:01 ` Harald Nordgren @ 2026-01-14 2:22 ` D. Ben Knoble 2026-01-14 7:59 ` Harald Nordgren 2026-01-14 2:34 ` Jeff King 1 sibling, 1 reply; 39+ messages in thread From: D. Ben Knoble @ 2026-01-14 2:22 UTC (permalink / raw) To: Harald Nordgren; +Cc: peff, git, gitgitgadget On Tue, Jan 13, 2026 at 6:01 PM Harald Nordgren <haraldnordgren@gmail.com> wrote: > > Hi Jeff! > > I'm very happy that your responded respectfully despite me basically > saying that you were using Git wrong. It's nice to see how some of the pros > do it! > > I'm wondering if since you are scripting this anyway, if you really need a > push branch at all? Can't you just as easily switch to doing this in the > script: > > git config push.default upstream > git push github jk/some-topic The script is doing the "moral equivalent of" "git push -f github refs/heads/jk/*", so I'm not sure I follow the suggestion here. > As a note, before I started working on this feature, I don't realize > that there was such a thing as a push branch (i.e. something different from > the tracking branch). So I had the habit of checking out and pushing like > this: > > git branch --set-upstream-to upstream/master > git push origin $(git rev-parse --abbrev-ref HEAD) > > I worked really well for me. The only issue was missing the status info > from my own branch -- which is why I started writing this feature. > > > Harald My workflow is different from Peff's, but it is similar along at least one line: it's really convenient to have "git push" with no further arguments (only possibly flags) to push my branch to a remote mirror. -- D. Ben Knoble ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-14 2:22 ` D. Ben Knoble @ 2026-01-14 7:59 ` Harald Nordgren 2026-01-14 21:38 ` Ben Knoble 0 siblings, 1 reply; 39+ messages in thread From: Harald Nordgren @ 2026-01-14 7:59 UTC (permalink / raw) To: ben.knoble; +Cc: git, gitgitgadget, haraldnordgren, peff > My workflow is different from Peff's, but it is similar along at least > one line: it's really convenient to have "git push" with no further > arguments (only possibly flags) to push my branch to a remote mirror. Would you also like the status reporting to be off for your push branch? I asking because that's what Jeff is arguing for. I fully agree on the convenience of bare 'git push', I use it everyday together with these settings and commands for max convenience: git config --global remote.pushDefault origin # As opposed to upstream git config --global push.default current git config --global pull.rebase true git config --global rebase.autoStash true git checkout $(gr | rg '^(upstream|origin)$' | tail -n1)/HEAD -b new_branch git push git status git pull Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 7:59 ` Harald Nordgren @ 2026-01-14 21:38 ` Ben Knoble 0 siblings, 0 replies; 39+ messages in thread From: Ben Knoble @ 2026-01-14 21:38 UTC (permalink / raw) To: Harald Nordgren; +Cc: git, gitgitgadget, peff > Le 14 janv. 2026 à 02:59, Harald Nordgren <haraldnordgren@gmail.com> a écrit : > > >> My workflow is different from Peff's, but it is similar along at least >> one line: it's really convenient to have "git push" with no further >> arguments (only possibly flags) to push my branch to a remote mirror. > > Would you also like the status reporting to be off for your push branch? > I asking because that's what Jeff is arguing for. I’m somewhat indifferent (mildly for having it on by default), but I meant that you don’t want to suggest that in order to use your new feature you don’t want folks to have to break their existing workflows ;) ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-13 23:01 ` Harald Nordgren 2026-01-14 2:22 ` D. Ben Knoble @ 2026-01-14 2:34 ` Jeff King 2026-01-14 7:53 ` Harald Nordgren 2026-01-14 14:15 ` Junio C Hamano 1 sibling, 2 replies; 39+ messages in thread From: Jeff King @ 2026-01-14 2:34 UTC (permalink / raw) To: Harald Nordgren; +Cc: git, gitgitgadget On Wed, Jan 14, 2026 at 12:01:07AM +0100, Harald Nordgren wrote: > I'm wondering if since you are scripting this anyway, if you really need a > push branch at all? Can't you just as easily switch to doing this in the > script: > > git config push.default upstream > git push github jk/some-topic I could (and in fact the script names the remote directly already, because you can't pass refspecs without specifying the remote). But I do occasionally push a single branch with a bare "git push". Usually this is the integration branch, when I am trying to trigger CI manually (e.g., when piling hacks on top in order to debug a CI failure ;) ). So even if I only do it infrequently, it feels weird that a bare "git push" would try to push to the upstream remote (which I don't even have write access to!). > As a note, before I started working on this feature, I don't realize > that there was such a thing as a push branch (i.e. something different from > the tracking branch). So I had the habit of checking out and pushing like > this: > > git branch --set-upstream-to upstream/master > git push origin $(git rev-parse --abbrev-ref HEAD) > > I worked really well for me. The only issue was missing the status info > from my own branch -- which is why I started writing this feature. Yeah, though @{push} is usually not explicitly configured in the same way @{upstream} is, but rather a consequence of how push.default and remote.pushdefault interact. But it was added for exactly this kind of triangular workflow. I sometimes will do stuff like: git range-diff origin @{push} HEAD to compare two iterations of a branch if I know that I haven't pushed. It is a bit of a cheat, because what I really mean is "do a range-diff since the last thing I sent to the list". But if I have just been working on a branch, and I haven't run an integration cycle since then, then I know that the pushed version will match it. There is also branch.*.pushRemote, but I have not found that useful (for my triangular flow there is always a single repo to push to, not one per branch). -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-14 2:34 ` Jeff King @ 2026-01-14 7:53 ` Harald Nordgren 2026-01-14 16:24 ` Jeff King 2026-01-14 14:15 ` Junio C Hamano 1 sibling, 1 reply; 39+ messages in thread From: Harald Nordgren @ 2026-01-14 7:53 UTC (permalink / raw) To: peff; +Cc: git, gitgitgadget, haraldnordgren > I could (and in fact the script names the remote directly already, > because you can't pass refspecs without specifying the remote). But I do > occasionally push a single branch with a bare "git push". Usually this > is the integration branch, when I am trying to trigger CI manually > (e.g., when piling hacks on top in order to debug a CI failure ;) ). > > So even if I only do it infrequently, it feels weird that a bare "git > push" would try to push to the upstream remote (which I don't even have > write access to!). Maybe ’push.default=simple’ or ’push.default=nothing’ are better settings in your scenario. Then you get explicit pushing because no push branch gets set. And thus 'git status' reports not additinal status. > Yeah, though @{push} is usually not explicitly configured in the same > way @{upstream} is, but rather a consequence of how push.default and > remote.pushdefault interact. But it was added for exactly this kind of > triangular workflow. I sometimes will do stuff like: > > git range-diff origin @{push} HEAD I imagine the same thing could be achieved with origin/$(git rev-parse --abbrev-ref HEAD) Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 7:53 ` Harald Nordgren @ 2026-01-14 16:24 ` Jeff King 2026-01-14 17:48 ` Harald Nordgren 2026-01-14 21:38 ` Ben Knoble 0 siblings, 2 replies; 39+ messages in thread From: Jeff King @ 2026-01-14 16:24 UTC (permalink / raw) To: Harald Nordgren; +Cc: git, gitgitgadget On Wed, Jan 14, 2026 at 08:53:09AM +0100, Harald Nordgren wrote: > > I could (and in fact the script names the remote directly already, > > because you can't pass refspecs without specifying the remote). But I do > > occasionally push a single branch with a bare "git push". Usually this > > is the integration branch, when I am trying to trigger CI manually > > (e.g., when piling hacks on top in order to debug a CI failure ;) ). > > > > So even if I only do it infrequently, it feels weird that a bare "git > > push" would try to push to the upstream remote (which I don't even have > > write access to!). > > Maybe ’push.default=simple’ or ’push.default=nothing’ are better settings > in your scenario. Then you get explicit pushing because no push branch gets > set. And thus 'git status' reports not additinal status. If I did that, then the occasional "git push" (without arguments) that I do would fail. Likewise, @{push} would not be usable. > > Yeah, though @{push} is usually not explicitly configured in the same > > way @{upstream} is, but rather a consequence of how push.default and > > remote.pushdefault interact. But it was added for exactly this kind of > > triangular workflow. I sometimes will do stuff like: > > > > git range-diff origin @{push} HEAD > > I imagine the same thing could be achieved with > > origin/$(git rev-parse --abbrev-ref HEAD) Sure, but: 1. It is a lot shorter to type @{push}. ;) 2. Using @{push} works everywhere, even on my non-triangular repos, because it takes into account the push configuration. So it's a much nicer muscle-memory to acquire. -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-14 16:24 ` Jeff King @ 2026-01-14 17:48 ` Harald Nordgren 2026-01-14 21:01 ` Jeff King 2026-01-14 21:38 ` Ben Knoble 1 sibling, 1 reply; 39+ messages in thread From: Harald Nordgren @ 2026-01-14 17:48 UTC (permalink / raw) To: peff; +Cc: git, gitgitgadget, haraldnordgren > Sure, but: > > 1. It is a lot shorter to type @{push}. ;) > > 2. Using @{push} works everywhere, even on my non-triangular repos, > because it takes into account the push configuration. So it's a > much nicer muscle-memory to acquire. Makes sense, I’m all out of arguments here 😅 Please don’t take this the wrong way, but it reminds me of this XKCD: https://xkcd.com/1172/ As long as this is turned on by default then I’m mostly happy. Maybe JCH and others can weigh in on if this needs to be this configurable or always on. Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 17:48 ` Harald Nordgren @ 2026-01-14 21:01 ` Jeff King 0 siblings, 0 replies; 39+ messages in thread From: Jeff King @ 2026-01-14 21:01 UTC (permalink / raw) To: Harald Nordgren; +Cc: git, gitgitgadget On Wed, Jan 14, 2026 at 06:48:45PM +0100, Harald Nordgren wrote: > > Sure, but: > > > > 1. It is a lot shorter to type @{push}. ;) > > > > 2. Using @{push} works everywhere, even on my non-triangular repos, > > because it takes into account the push configuration. So it's a > > much nicer muscle-memory to acquire. > > Makes sense, I’m all out of arguments here 😅 Please don’t take this the > wrong way, but it reminds me of this XKCD: https://xkcd.com/1172/ Perhaps if the person in that comic was the one who implemented @{push} in the first place. ;) That said, I really don't think my use case is exotic. I've set up "git push" in the only way that makes sense for my triangular setup. I just don't happen to use it that often, and I don't want to be reminded me unnecessarily of the relationship between each branch and its push destination. > As long as this is turned on by default then I’m mostly happy. Maybe JCH > and others can weigh in on if this needs to be this configurable or always > on. IMHO not making it configurable is a show-stopper, because without that it leaves people who don't like the new behavior with no escape hatch. As to the default for that config option, I don't have a strong opinion. I'd lean towards "off", as I do not find the feature that compelling myself, and I don't recall ever hearing of it as a pain point for other Git users. But I also recognize that's based on vague vibes, and I think users are generally not very savvy about setting up triangular workflows in the first place. -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 16:24 ` Jeff King 2026-01-14 17:48 ` Harald Nordgren @ 2026-01-14 21:38 ` Ben Knoble 2026-01-14 22:17 ` Jeff King 1 sibling, 1 reply; 39+ messages in thread From: Ben Knoble @ 2026-01-14 21:38 UTC (permalink / raw) To: Jeff King; +Cc: Harald Nordgren, git, gitgitgadget > Le 14 janv. 2026 à 11:31, Jeff King <peff@peff.net> a écrit : > >>> Yeah, though @{push} is usually not explicitly configured in the same >>> way @{upstream} is, but rather a consequence of how push.default and >>> remote.pushdefault interact. But it was added for exactly this kind of >>> triangular workflow. I sometimes will do stuff like: >>> git range-diff origin @{push} HEAD >> I imagine the same thing could be achieved with >> origin/$(git rev-parse --abbrev-ref HEAD) > > Sure, but: > > 1. It is a lot shorter to type @{push}. ;) > > 2. Using @{push} works everywhere, even on my non-triangular repos, Just so I’m clear, this is only with push.default=current, right? I could never make @{push} work otherwise. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 21:38 ` Ben Knoble @ 2026-01-14 22:17 ` Jeff King 2026-01-15 16:17 ` D. Ben Knoble 0 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2026-01-14 22:17 UTC (permalink / raw) To: Ben Knoble; +Cc: Harald Nordgren, git, gitgitgadget On Wed, Jan 14, 2026 at 04:38:33PM -0500, Ben Knoble wrote: > > Le 14 janv. 2026 à 11:31, Jeff King <peff@peff.net> a écrit : > > > >>> Yeah, though @{push} is usually not explicitly configured in the same > >>> way @{upstream} is, but rather a consequence of how push.default and > >>> remote.pushdefault interact. But it was added for exactly this kind of > >>> triangular workflow. I sometimes will do stuff like: > >>> git range-diff origin @{push} HEAD > >> I imagine the same thing could be achieved with > >> origin/$(git rev-parse --abbrev-ref HEAD) > > > > Sure, but: > > > > 1. It is a lot shorter to type @{push}. ;) > > > > 2. Using @{push} works everywhere, even on my non-triangular repos, > > Just so I’m clear, this is only with push.default=current, right? I could never make @{push} work otherwise. I always use push.default=current, though I think @{push} should work with other modes. E.g., with this setup: git checkout -b foo git clone . tmp cd tmp # for the sake of simplicity, our triangle goes to the same place ;) git remote add triangle .. git fetch triangle git config remote.pushdefault triangle then doing: git -c push.default=current rev-parse --symbolic-full-name @{push} and: git -c push.default=matching rev-parse --symbolic-full-name @{push} should both point to refs/remotes/triangle/foo. Using "simple" will not work, because it demands that the upstream and the push destination are the same (so it doesn't really make sense in a triangular flow at all). But in a non-triangular flow, it will happily point @{push} to the same as @{upstream}. I use a triangular flow for git.git, but most of my other repos are just personal projects, and I push/fetch from a single central source. -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 22:17 ` Jeff King @ 2026-01-15 16:17 ` D. Ben Knoble 0 siblings, 0 replies; 39+ messages in thread From: D. Ben Knoble @ 2026-01-15 16:17 UTC (permalink / raw) To: Jeff King; +Cc: Harald Nordgren, git, gitgitgadget On Wed, Jan 14, 2026 at 5:17 PM Jeff King <peff@peff.net> wrote: > > On Wed, Jan 14, 2026 at 04:38:33PM -0500, Ben Knoble wrote: > > > > Le 14 janv. 2026 à 11:31, Jeff King <peff@peff.net> a écrit : > > > > > >>> Yeah, though @{push} is usually not explicitly configured in the same > > >>> way @{upstream} is, but rather a consequence of how push.default and > > >>> remote.pushdefault interact. But it was added for exactly this kind of > > >>> triangular workflow. I sometimes will do stuff like: > > >>> git range-diff origin @{push} HEAD > > >> I imagine the same thing could be achieved with > > >> origin/$(git rev-parse --abbrev-ref HEAD) > > > > > > Sure, but: > > > > > > 1. It is a lot shorter to type @{push}. ;) > > > > > > 2. Using @{push} works everywhere, even on my non-triangular repos, > > > > Just so I’m clear, this is only with push.default=current, right? I could never make @{push} work otherwise. > > I always use push.default=current, though I think @{push} should work > with other modes. E.g., with this setup: > > git checkout -b foo > git clone . tmp > cd tmp > > # for the sake of simplicity, our triangle goes to the same place ;) > git remote add triangle .. > git fetch triangle > git config remote.pushdefault triangle > > then doing: > > git -c push.default=current rev-parse --symbolic-full-name @{push} > > and: > > git -c push.default=matching rev-parse --symbolic-full-name @{push} > > should both point to refs/remotes/triangle/foo. Using "simple" will not > work, because it demands that the upstream and the push destination are > the same (so it doesn't really make sense in a triangular flow at all). Gotcha. Yeah, simple (the default) doesn't work. I suppose upstream might, too. (I also have push.default=current globally, was just wondering about what the minimum was to enabled triangular workflows; see <https://lore.kernel.org/git/CALnO6CAUSU-Pq_r-WYm3o0to6H8MdqiYOuoKaRfL1PTt30VaoQ@mail.gmail.com/>.) > But in a non-triangular flow, it will happily point @{push} to the same > as @{upstream}. I use a triangular flow for git.git, but most of my > other repos are just personal projects, and I push/fetch from a single > central source. > > -Peff Ditto, yeah (_sometimes_ I actually do the triangle origin/main, local branch, origin/branch, so I almost always set upstream as origin/main anyways). Cool and thanks! -- D. Ben Knoble ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 2:34 ` Jeff King 2026-01-14 7:53 ` Harald Nordgren @ 2026-01-14 14:15 ` Junio C Hamano 1 sibling, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2026-01-14 14:15 UTC (permalink / raw) To: Jeff King; +Cc: Harald Nordgren, git, gitgitgadget Jeff King <peff@peff.net> writes: > So even if I only do it infrequently, it feels weird that a bare "git > push" would try to push to the upstream remote (which I don't even have > write access to!). Yes, exactly. I was wondering where Harald's suggestion to swap the two remotes came from. > Yeah, though @{push} is usually not explicitly configured in the same > way @{upstream} is, but rather a consequence of how push.default and > remote.pushdefault interact. But it was added for exactly this kind of > triangular workflow. I sometimes will do stuff like: > > git range-diff origin @{push} HEAD > > to compare two iterations of a branch if I know that I haven't pushed. > It is a bit of a cheat, because what I really mean is "do a range-diff > since the last thing I sent to the list". But if I have just been > working on a branch, and I haven't run an integration cycle since then, > then I know that the pushed version will match it. Great suggestion. It is fun to see that comparing notes among people with different workflows brings out these gems ;-) > There is also branch.*.pushRemote, but I have not found that useful (for > my triangular flow there is always a single repo to push to, not one per > branch). ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-13 21:40 ` Jeff King 2026-01-13 23:01 ` Harald Nordgren @ 2026-01-14 18:54 ` Junio C Hamano 2026-01-14 21:10 ` Jeff King ` (2 more replies) 1 sibling, 3 replies; 39+ messages in thread From: Junio C Hamano @ 2026-01-14 18:54 UTC (permalink / raw) To: Jeff King; +Cc: Harald Nordgren, git, gitgitgadget Jeff King <peff@peff.net> writes: > And having the extra output from "git checkout" is just extra noise for > me, especially because it is easy to see only the second message (which > looks just like the upstream ahead/behind message, of course) and get > confused. The first time I saw it I thought I had misconfigured > something with my branch. It now is clear to me that this should be _optional_, so that those who do really want extra output from the command should explicitly opt into the feature. After all, any optional new feature that you must opt into by definition cannot regress end user experience for those who do not ;-) At the same time, I suspect that extra comparison on top of what we already give against the @{upstream} may not be limited to what Harald implemented (is it essentially the same as specyfing @{push}, or something else?). I wonder if we can come up with a flexible and extensible notation to specify what branch(es) to compare with, so that we can use it as the value of this opt-in configuration variable? Something like [status] compareBranches = @{upstream} @{push} signals that the current branch is compared against these two branches, and not having the configuration (i.e., traditional behaviour, which is left the default) would be equivalent to have [status] compareBranches = @{upstream} or something like that, perhaps? Thanks. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 18:54 ` Junio C Hamano @ 2026-01-14 21:10 ` Jeff King 2026-01-14 21:38 ` Ben Knoble 2026-01-14 23:12 ` Harald Nordgren 2026-01-18 19:58 ` Harald Nordgren 2 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2026-01-14 21:10 UTC (permalink / raw) To: Junio C Hamano; +Cc: Harald Nordgren, git, gitgitgadget On Wed, Jan 14, 2026 at 10:54:53AM -0800, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > And having the extra output from "git checkout" is just extra noise for > > me, especially because it is easy to see only the second message (which > > looks just like the upstream ahead/behind message, of course) and get > > confused. The first time I saw it I thought I had misconfigured > > something with my branch. > > It now is clear to me that this should be _optional_, so that those > who do really want extra output from the command should explicitly > opt into the feature. After all, any optional new feature that you > must opt into by definition cannot regress end user experience for > those who do not ;-) True, but then it also cannot pleasantly surprise people who didn't realize they wanted it. Having your user experience regressed and then tweaking a config option to fix it is not too bad. The deciding factor to me is whether more people will be pleasantly surprised or annoyed. ;) I don't have a strong sense there. As a general principle, though, I think a reasonable path forward for any behavior change is: 1. Implement the new behavior, hidden behind a config option. 2. Wait a while to see how people like the new option, and shake out any bugs. 3. If people like the option and are puzzled why it isn't the default, then flip the default on. In other words, let the utility of the feature be proven in practice by people opting into it. There is a chicken-and-egg problem if they don't know about it, but if it is truly solving a problem people have, then hopefully some of them would look for a solution and find it. End philosophical rambling. ;) > At the same time, I suspect that extra comparison on top of what we > already give against the @{upstream} may not be limited to what > Harald implemented (is it essentially the same as specyfing @{push}, > or something else?). I haven't been following the feature closely, but my understanding is that yes, it's basically "also compare to @{push} if it is not the same as @{upstream}". > I wonder if we can come up with a flexible and extensible notation to > specify what branch(es) to compare with, so that we can use it as the > value of this opt-in configuration variable? Something like > > [status] compareBranches = @{upstream} @{push} > > signals that the current branch is compared against these two > branches, and not having the configuration (i.e., traditional > behaviour, which is left the default) would be equivalent to have > > [status] compareBranches = @{upstream} > > or something like that, perhaps? Interesting. That is more flexible, though I'm not sure how useful that flexibility is. I guess you could imagine putting in a static branch. E.g., if you base your branches off of "master", might you want to show ahead/behind to "maint" or "next"? I have trouble imagining a workflow where I would want to do that often enough for git-status (and checkout) to do it automatically, though. But assuming it suppresses duplicates, then yeah, this feels like a more flexible superset of the functionality. -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 21:10 ` Jeff King @ 2026-01-14 21:38 ` Ben Knoble 2026-01-14 23:08 ` Harald Nordgren 2026-01-19 5:58 ` Chris Torek 0 siblings, 2 replies; 39+ messages in thread From: Ben Knoble @ 2026-01-14 21:38 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Harald Nordgren, git, gitgitgadget > Le 14 janv. 2026 à 16:11, Jeff King <peff@peff.net> a écrit : > > On Wed, Jan 14, 2026 at 10:54:53AM -0800, Junio C Hamano wrote: > >> Jeff King <peff@peff.net> writes: >>> And having the extra output from "git checkout" is just extra noise for >>> me, especially because it is easy to see only the second message (which >>> looks just like the upstream ahead/behind message, of course) and get >>> confused. The first time I saw it I thought I had misconfigured >>> something with my branch. >> It now is clear to me that this should be _optional_, so that those >> who do really want extra output from the command should explicitly >> opt into the feature. After all, any optional new feature that you >> must opt into by definition cannot regress end user experience for >> those who do not ;-) > > True, but then it also cannot pleasantly surprise people who didn't > realize they wanted it. > > Having your user experience regressed and then tweaking a config option > to fix it is not too bad. The deciding factor to me is whether more > people will be pleasantly surprised or annoyed. ;) I don't have a strong > sense there. > > As a general principle, though, I think a reasonable path forward for > any behavior change is: > > 1. Implement the new behavior, hidden behind a config option. > > 2. Wait a while to see how people like the new option, and shake out > any bugs. > > 3. If people like the option and are puzzled why it isn't the default, > then flip the default on. > > In other words, let the utility of the feature be proven in practice by > people opting into it. There is a chicken-and-egg problem if they don't > know about it, but if it is truly solving a problem people have, then > hopefully some of them would look for a solution and find it. > > End philosophical rambling. ;) Agreed generally, but the chicken-egg goes 2 layers deep here due to triangular workflows ;) I favor something similar to what Junio described but also including @{push} by default (and ignoring it if non-existent), so that folks discovering triangular workflows for the first time are easily able to see what is happening. Us ”already triangular” squares are probably well-versed enough in Git to find and tweak the new feature if desired. Idk though. I think more folks at work should try triangular flows, so I’m biased :p ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-14 21:38 ` Ben Knoble @ 2026-01-14 23:08 ` Harald Nordgren 2026-01-19 5:58 ` Chris Torek 1 sibling, 0 replies; 39+ messages in thread From: Harald Nordgren @ 2026-01-14 23:08 UTC (permalink / raw) To: ben.knoble; +Cc: git, gitgitgadget, gitster, haraldnordgren, peff > Agreed generally, but the chicken-egg goes 2 layers deep here due to triangular workflows ;) > > I favor something similar to what Junio described but also including @{push} by default (and ignoring it if non-existent), so that folks discovering > triangular workflows for the first time are easily able to see what is happening. > > Us ”already triangular” squares are probably well-versed enough in Git to find and tweak the new feature if desired. > > Idk though. I think more folks at work should try triangular flows, so I’m biased :p Ben has an excellent point about triangular workflows, and Jeff of course, your philosophical instincts seem correct to me, interesting to read! I would think that most people don't have separate tracking and push branches to begin with. So it's not a lot of people would be bothered by having this be on by default. The people who know about triangular workflows will also be able to find how to turn this off. Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 21:38 ` Ben Knoble 2026-01-14 23:08 ` Harald Nordgren @ 2026-01-19 5:58 ` Chris Torek 2026-01-20 8:35 ` Harald Nordgren 1 sibling, 1 reply; 39+ messages in thread From: Chris Torek @ 2026-01-19 5:58 UTC (permalink / raw) To: Ben Knoble; +Cc: Jeff King, Junio C Hamano, Harald Nordgren, git, gitgitgadget (I'm somewhat behind all week so this is from Wednesday...) Re: status.compareBranch (or similar): I like the idea; I'm not sure what sort of details might be needed in the end though. On Wed, Jan 14, 2026 at 1:42 PM Ben Knoble <ben.knoble@gmail.com> wrote: > I favor something similar to what Junio described but also including @{push} by default (and ignoring it if non-existent), so that folks discovering triangular workflows for the first time are easily able to see what is happening. This also seems to me likely to be the right default. It's useful for a lot of GitHub and similar forges, where you send fixes upstream by first forking some official repository and then cloning your fork (e.g., to a laptop), setting up your local clone (on laptop) to have two remotes: the official repository, and your fork, both on the same forge. It's a little annoying to have to deal with *three* potential "upstream" repos, if you need to back up your local work to a corporate server or forge *plus* a push-for-pull-request at the forge as well, of course. But then at least the "compare with all three" option becomes available. How you wish to spell the default push location is then up to you :-) Chris ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-19 5:58 ` Chris Torek @ 2026-01-20 8:35 ` Harald Nordgren 0 siblings, 0 replies; 39+ messages in thread From: Harald Nordgren @ 2026-01-20 8:35 UTC (permalink / raw) To: chris.torek; +Cc: ben.knoble, git, gitgitgadget, gitster, haraldnordgren, peff >> I favor something similar to what Junio described but also including >> @{push} by default (and ignoring it if non-existent), so that folks >> discovering triangular workflows for the first time are easily able to >> see what is happening. > This also seems to me likely to be the right default. It's useful for a lot of > GitHub and similar forges, where you send fixes upstream by first forking > some official repository and then cloning your fork (e.g., to a laptop), setting > up your local clone (on laptop) to have two remotes: the official repository, > and your fork, both on the same forge. I agree with this too, would like to make '@{upstream} @{push}' the default. Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-14 18:54 ` Junio C Hamano 2026-01-14 21:10 ` Jeff King @ 2026-01-14 23:12 ` Harald Nordgren 2026-01-14 23:31 ` Junio C Hamano 2026-01-18 19:58 ` Harald Nordgren 2 siblings, 1 reply; 39+ messages in thread From: Harald Nordgren @ 2026-01-14 23:12 UTC (permalink / raw) To: gitster; +Cc: git, gitgitgadget, haraldnordgren, peff > I wonder if we can come up with a flexible and > extensible notation to specify what branch(es) to compare with, so > that we can use it as the value of this opt-in configuration > variable? Something like > > [status] compareBranches = @{upstream} @{push} If we go with that, then it becomes trickier code-wise to show push/pull advice for the correct branches. But not impossible since we can check if branch is the same as @{upstream} or @{push}. Philosophically, two main git commands are 'git push' and 'git pull'. So it makes perfect sense to me to signal that those two are special, and not allow other compareBranches. Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Triangular workflow 2026-01-14 23:12 ` Harald Nordgren @ 2026-01-14 23:31 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2026-01-14 23:31 UTC (permalink / raw) To: Harald Nordgren; +Cc: git, gitgitgadget, peff Harald Nordgren <haraldnordgren@gmail.com> writes: >> I wonder if we can come up with a flexible and >> extensible notation to specify what branch(es) to compare with, so >> that we can use it as the value of this opt-in configuration >> variable? Something like >> >> [status] compareBranches = @{upstream} @{push} > > If we go with that, then it becomes trickier code-wise to show push/pull > advice for the correct branches. But not impossible since we can check if > branch is the same as @{upstream} or @{push}. > > Philosophically, two main git commands are 'git push' and 'git pull'. So it > makes perfect sense to me to signal that those two are special, and not > allow other compareBranches. You are falling into the same trap as what the folks who designed the original ahead/behind comparison did by limiting yourself to push and pull. They said object transfer always interacts with the @{upstream}, hence only comparison with that is sufficient and it makes sense not to allow comparing with anything else. Until you started wishing that you want to also compare with @{push}, that is ;-). A static point that has nothing to do with pushing and pulling (e.g., "the latest official release tag") may be something useful to compare with in certain workflows (presumably workflow of the maintainer type), so limiting our sights to object transfer like push and pull is likely to be a mistake. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Triangular workflow 2026-01-14 18:54 ` Junio C Hamano 2026-01-14 21:10 ` Jeff King 2026-01-14 23:12 ` Harald Nordgren @ 2026-01-18 19:58 ` Harald Nordgren 2 siblings, 0 replies; 39+ messages in thread From: Harald Nordgren @ 2026-01-18 19:58 UTC (permalink / raw) To: gitster; +Cc: git, gitgitgadget, haraldnordgren, peff > > And having the extra output from "git checkout" is just extra noise for > > me, especially because it is easy to see only the second message (which > > looks just like the upstream ahead/behind message, of course) and get > > confused. The first time I saw it I thought I had misconfigured > > something with my branch. > > It now is clear to me that this should be _optional_, so that those > who do really want extra output from the command should explicitly > opt into the feature. After all, any optional new feature that you > must opt into by definition cannot regress end user experience for > those who do not ;-) > > At the same time, I suspect that extra comparison on top of what we > already give against the @{upstream} may not be limited to what > Harald implemented (is it essentially the same as specyfing @{push}, > or something else?). I wonder if we can come up with a flexible and > extensible notation to specify what branch(es) to compare with, so > that we can use it as the value of this opt-in configuration > variable? Something like > > [status] compareBranches = @{upstream} @{push} > > signals that the current branch is compared against these two > branches, and not having the configuration (i.e., traditional > behaviour, which is left the default) would be equivalent to have > > [status] compareBranches = @{upstream} > > or something like that, perhaps? I'm implemented this now. Please take a look at the latest patch! But there seems to be a memory leak that I can't figure out after spending some hours running the CI over and over. Harald ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2026-01-20 20:18 UTC | newest] Thread overview: 39+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-19 5:18 [PATCH 0/4] memory leaks in remote.c Jeff King 2026-01-19 5:19 ` [PATCH 1/4] remote: return non-const pointer from error_buf() Jeff King 2026-01-19 6:33 ` Patrick Steinhardt 2026-01-20 0:28 ` Junio C Hamano 2026-01-20 19:38 ` Jeff King 2026-01-20 20:18 ` Junio C Hamano 2026-01-19 5:20 ` [PATCH 2/4] remote: drop const return of tracking_for_push_dest() Jeff King 2026-01-19 6:34 ` Patrick Steinhardt 2026-01-19 5:22 ` [PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config Jeff King 2026-01-19 6:34 ` Patrick Steinhardt 2026-01-19 5:23 ` [PATCH 4/4] remote: always allocate branch.push_tracking_ref Jeff King 2026-01-19 6:34 ` Patrick Steinhardt 2026-01-19 15:04 ` Triangular workflow Harald Nordgren 2026-01-20 19:40 ` Jeff King 2026-01-20 0:31 ` [PATCH 0/4] memory leaks in remote.c Junio C Hamano -- strict thread matches above, loose matches on Subject: below -- 2026-01-13 17:03 [PATCH v25 2/2] status: show comparison with push remote tracking branch Jeff King 2026-01-13 18:35 ` Triangular workflow Harald Nordgren 2026-01-13 21:40 ` Jeff King 2026-01-13 23:01 ` Harald Nordgren 2026-01-14 2:22 ` D. Ben Knoble 2026-01-14 7:59 ` Harald Nordgren 2026-01-14 21:38 ` Ben Knoble 2026-01-14 2:34 ` Jeff King 2026-01-14 7:53 ` Harald Nordgren 2026-01-14 16:24 ` Jeff King 2026-01-14 17:48 ` Harald Nordgren 2026-01-14 21:01 ` Jeff King 2026-01-14 21:38 ` Ben Knoble 2026-01-14 22:17 ` Jeff King 2026-01-15 16:17 ` D. Ben Knoble 2026-01-14 14:15 ` Junio C Hamano 2026-01-14 18:54 ` Junio C Hamano 2026-01-14 21:10 ` Jeff King 2026-01-14 21:38 ` Ben Knoble 2026-01-14 23:08 ` Harald Nordgren 2026-01-19 5:58 ` Chris Torek 2026-01-20 8:35 ` Harald Nordgren 2026-01-14 23:12 ` Harald Nordgren 2026-01-14 23:31 ` Junio C Hamano 2026-01-18 19:58 ` Harald Nordgren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox