diff mbox series

[v4] merge: new autosetupmerge option 'simple' for matching branches

Message ID pull.1161.v4.git.1647843442911.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v4] merge: new autosetupmerge option 'simple' for matching branches | expand

Commit Message

Tao Klerks March 21, 2022, 6:17 a.m. UTC
From: Tao Klerks <tao@klerks.biz>

With the default push.default option, "simple", beginners are
protected from accidentally pushing to the "wrong" branch in
centralized workflows: if the remote tracking branch they would push
to does not have the same name as the local branch, and they try to do
a "default push", they get an error and explanation with options.

There is a particular centralized workflow where this often happens:
a user branches to a new local feature branch from an existing
upstream branch, eg with "checkout -b feature1 origin/master". With
the default branch.autosetupmerge configuration (value "true"), git
will automatically add origin/master as the remote tracking branch.

When the user pushes with "git push", they get an error, and (amongst
other things) a suggestion to run "git push origin HEAD". Eventually
they figure out to add "-u" to change the tracking branch, or they set
push.default to "current", or some tooling does one or the other of
these things for them.

When one of their coworkers works on the same branch, they don't get
any of that weirdness. They just "git checkout feature1" and
everything works exactly as they expect, with the shared remote branch
set up as remote tracking branch, and push and pull working out of the
box.

The "stable state" for this way of working is that local branches have
the same-name remote tracking branch (origin/feature1 in this
example), and multiple people can work on that remote feature branch
at the same time, trusting "git pull" to merge or rebase as required
for them to be able to push their interim changes to that same feature
branch on that same remote.

(merging from the upstream "master" branch, and merging back to it,
are separate more involved processes in this flow).

There is a problem in this flow/way of working, however, which is that
the first user, when they first branched from origin/master, ended up
with the "wrong" remote tracking branch (different from the stable
state). For a while, before they pushed (and maybe longer, if they
don't use -u/--set-upstream), their "git pull" wasn't getting other
users' changes to the feature branch - it was getting any changes from
the remote "master" branch instead (a completely different class of
changes!)

Any experienced git user will presumably say "well yeah, that's what
it means to have the remote tracking branch set to origin/master!" -
but that user didn't *ask* to have the remote master branch added as
remote tracking branch - that just happened automatically when they
branched their feature branch. They didn't necessarily even notice or
understand the meaning of the "set up to track 'origin/master'"
message when they created the branch - especially if they are using a
GUI.

Looking at how to fix this, you might think "OK, so disable auto setup
of remote tracking - set branch.autosetupmerge to false" - but that
will inconvenience the *second* user in this story - the one who just
wanted to start working on the feature branch. The first and second
users swap roles at different points in time of course - they should
both have a sane configuration that does the right thing in both
situations.

Make these flows painless by introducing a new branch.autosetupmerge
option called "simple", to match the same-name "push.default" option
that makes similar assumptions.

This new option automatically sets up tracking in a *subset* of the
current default situations: when the original ref is a remote tracking
branch *and* has the same branch name on the remote (as the new local
branch name).

With this new configuration, in the example situation above, the first
user does *not* get origin/master set up as the tracking branch for
the new local branch. If they "git pull" in their new local-only
branch, they get an error explaining there is no upstream branch -
which makes sense and is helpful. If they "git push", they get an
error explaining how to push *and* suggesting they specify
--set-upstream - which is exactly the right thing to do for them.

This new option is likely not appropriate for users intentionally
implementing a "triangular workflow" with a shared upstream tracking
branch, that they "git pull" in and a "private" feature branch that
they push/force-push to just for remote safe-keeping until they are
ready to push up to the shared branch explicitly/separately. Such
users are likely to prefer keeping the current default
merge.autosetupmerge=true behavior, and change their push.default to
"current".

Also extend the existing branch tests with three new cases testing
this option - the obvious matching-name and non-matching-name cases,
and also a non-matching-ref-type case. The matching-name case needs to
temporarily create an independent repo to fetch from, as the general
strategy of using the local repo as the remote in these tests
precludes locally branching with the same name as in the "remote".

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
    adding new branch.autosetupmerge option "simple"
    
    This patchset introduces a new option to the branch.autosetupmerge
    setting, "simple", which is intended to be consistent with and
    complementary to the push.default "simple" option.
    
    The push.defaut option "simple" helps produce predictable/understandable
    behavior for beginners, where they don't accidentally push to the
    "wrong" branch in centralized workflows. If they create a local branch
    with a different name and then try to do a plain push, it will helpfully
    fail and explain why.
    
    However, such users can often find themselves confused by the behavior
    of git after they first branch, and before they push. At that stage,
    their upstream tracking branch is the original remote branch, and pull
    will be bringing in "upstream changes" - eg all changes to "main", in a
    typical project where that's where they branched from. On the other
    hand, once they push their new branch (dealing with the initial error,
    following instructions to push to the right name), subsequent "pull"
    calls will behave as expected, only bring in any changes to that new
    branch they pushed.
    
    The new option introduced here, with push.default set to simple, ensures
    that push/pull behavior is generally consistent - tracking will be
    automatically set up for branches that push will work for (and pull will
    be consistent for) only.
    
    Changes since v3:
    
     * squashed new-tests commit into main changes, as per Ævar's advice
     * added some hopefully-helpful comments in some prior existing code
     * improved tests to use better idioms following Ævar and Eric's advice

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1161%2FTaoK%2Ffeature-branch-autosetupmerge-simple-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1161/TaoK/feature-branch-autosetupmerge-simple-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1161

Range-diff vs v3:

 1:  0b5d4789512 ! 1:  eca8ab2eb7b merge: new autosetupmerge option 'simple' for matching branches
     @@ Commit message
          merge.autosetupmerge=true behavior, and change their push.default to
          "current".
      
     +    Also extend the existing branch tests with three new cases testing
     +    this option - the obvious matching-name and non-matching-name cases,
     +    and also a non-matching-ref-type case. The matching-name case needs to
     +    temporarily create an independent repo to fetch from, as the general
     +    strategy of using the local repo as the remote in these tests
     +    precludes locally branching with the same name as in the "remote".
     +
          Signed-off-by: Tao Klerks <tao@klerks.biz>
      
       ## Documentation/config/branch.txt ##
     @@ Documentation/git-branch.txt: The exact upstream branch is chosen depending on t
       how the `branch.<name>.remote` and `branch.<name>.merge` options are used.
      
       ## branch.c ##
     +@@ branch.c: static int find_tracked_branch(struct remote *remote, void *priv)
     + 			free(tracking->spec.src);
     + 			string_list_clear(tracking->srcs, 0);
     + 		}
     ++		/* remote_find_tracking() searches by src if present */
     + 		tracking->spec.src = NULL;
     + 	}
     +-
     + 	return 0;
     + }
     + 
      @@ branch.c: static void setup_tracking(const char *new_ref, const char *orig_ref,
     + 
     + 	if (!tracking.matches)
     + 		switch (track) {
     ++		/* If ref is not remote, still use local */
     + 		case BRANCH_TRACK_ALWAYS:
     + 		case BRANCH_TRACK_EXPLICIT:
     + 		case BRANCH_TRACK_OVERRIDE:
     ++		/* Remote matches not evaluated */
     + 		case BRANCH_TRACK_INHERIT:
     + 			break;
     ++		/* Otherwise, if no remote don't track */
     + 		default:
       			goto cleanup;
       		}
       
      +	/*
     -+	 * This check does not apply to the BRANCH_TRACK_INHERIT
     -+	 * option; you can inherit one or more tracking entries
     -+	 * and the tracking.matches counter is not incremented.
     ++	 * This check does not apply to BRANCH_TRACK_INHERIT;
     ++	 * that supports multiple entries in tracking_srcs but
     ++	 * leaves tracking.matches at 0.
      +	 */
       	if (tracking.matches > 1)
       		die(_("not tracking: ambiguous information for ref %s"),
     @@ branch.c: static void setup_tracking(const char *new_ref, const char *orig_ref,
      +		 * Only track if remote branch name matches.
      +		 * Reaching into items[0].string is safe because
      +		 * we know there is at least one and not more than
     -+		 * one entry (because not BRANCH_TRACK_INHERIT).
     ++		 * one entry (because only BRANCH_TRACK_INHERIT can
     ++		 * produce more than one entry).
      +		 */
      +		const char *tracked_branch;
      +		if (!skip_prefix(tracking.srcs->items[0].string,
     @@ config.c: static int git_default_branch_config(const char *var, const char *valu
       		}
       		git_branch_track = git_config_bool(var, value);
       		return 0;
     +
     + ## t/t3200-branch.sh ##
     +@@ t/t3200-branch.sh: test_expect_success 'branch from tag w/--track causes failure' '
     + 	test_must_fail git branch --track my11 foobar
     + '
     + 
     ++test_expect_success 'simple tracking works when remote branch name matches' '
     ++	test_when_finished "rm -rf otherserver" &&
     ++	git init otherserver &&
     ++	test_commit -C otherserver my_commit 1 &&
     ++	git -C otherserver branch feature &&
     ++	test_config branch.autosetupmerge simple &&
     ++	test_config remote.otherserver.url otherserver &&
     ++	test_config remote.otherserver.fetch refs/heads/*:refs/remotes/otherserver/* &&
     ++	git fetch otherserver &&
     ++	git branch feature otherserver/feature &&
     ++	test_cmp_config otherserver branch.feature.remote &&
     ++	test_cmp_config refs/heads/feature branch.feature.merge
     ++'
     ++
     ++test_expect_success 'simple tracking skips when remote branch name does not match' '
     ++	test_config branch.autosetupmerge simple &&
     ++	test_config remote.local.url . &&
     ++	test_config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
     ++	git fetch local &&
     ++	git branch my-other local/main &&
     ++	test_cmp_config "" --default "" branch.my-other.remote &&
     ++	test_cmp_config "" --default "" branch.my-other.merge
     ++'
     ++
     ++test_expect_success 'simple tracking skips when remote ref is not a branch' '
     ++	test_config branch.autosetupmerge simple &&
     ++	test_config remote.localtags.url . &&
     ++	test_config remote.localtags.fetch refs/tags/*:refs/remotes/localtags/* &&
     ++	git tag mytag12 main &&
     ++	git fetch localtags &&
     ++	git branch mytag12 localtags/mytag12 &&
     ++	test_cmp_config "" --default "" branch.mytag12.remote &&
     ++	test_cmp_config "" --default "" branch.mytag12.merge
     ++'
     ++
     + test_expect_success '--set-upstream-to fails on multiple branches' '
     + 	echo "fatal: too many arguments to set new upstream" >expect &&
     + 	test_must_fail git branch --set-upstream-to main a b c 2>err &&
 2:  d5b18c7949f < -:  ----------- t3200: tests for new branch.autosetupmerge option "simple"


 Documentation/config/branch.txt |  4 +++-
 Documentation/git-branch.txt    | 18 ++++++++++-------
 branch.c                        | 25 ++++++++++++++++++++++-
 branch.h                        |  1 +
 config.c                        |  3 +++
 t/t3200-branch.sh               | 35 +++++++++++++++++++++++++++++++++
 6 files changed, 77 insertions(+), 9 deletions(-)


base-commit: 74cc1aa55f30ed76424a0e7226ab519aa6265061

Comments

Josh Steadmon April 18, 2022, 6:15 p.m. UTC | #1
On 2022.03.21 06:17, Tao Klerks via GitGitGadget wrote:
> From: Tao Klerks <tao@klerks.biz>
> 
> With the default push.default option, "simple", beginners are
> protected from accidentally pushing to the "wrong" branch in
> centralized workflows: if the remote tracking branch they would push
> to does not have the same name as the local branch, and they try to do
> a "default push", they get an error and explanation with options.
> 
> There is a particular centralized workflow where this often happens:
> a user branches to a new local feature branch from an existing
> upstream branch, eg with "checkout -b feature1 origin/master". With
> the default branch.autosetupmerge configuration (value "true"), git
> will automatically add origin/master as the remote tracking branch.
> 
> When the user pushes with "git push", they get an error, and (amongst
> other things) a suggestion to run "git push origin HEAD". Eventually
> they figure out to add "-u" to change the tracking branch, or they set
> push.default to "current", or some tooling does one or the other of
> these things for them.
> 
> When one of their coworkers works on the same branch, they don't get
> any of that weirdness. They just "git checkout feature1" and
> everything works exactly as they expect, with the shared remote branch
> set up as remote tracking branch, and push and pull working out of the
> box.
> 
> The "stable state" for this way of working is that local branches have
> the same-name remote tracking branch (origin/feature1 in this
> example), and multiple people can work on that remote feature branch
> at the same time, trusting "git pull" to merge or rebase as required
> for them to be able to push their interim changes to that same feature
> branch on that same remote.
> 
> (merging from the upstream "master" branch, and merging back to it,
> are separate more involved processes in this flow).
> 
> There is a problem in this flow/way of working, however, which is that
> the first user, when they first branched from origin/master, ended up
> with the "wrong" remote tracking branch (different from the stable
> state). For a while, before they pushed (and maybe longer, if they
> don't use -u/--set-upstream), their "git pull" wasn't getting other
> users' changes to the feature branch - it was getting any changes from
> the remote "master" branch instead (a completely different class of
> changes!)
> 
> Any experienced git user will presumably say "well yeah, that's what
> it means to have the remote tracking branch set to origin/master!" -
> but that user didn't *ask* to have the remote master branch added as
> remote tracking branch - that just happened automatically when they
> branched their feature branch. They didn't necessarily even notice or
> understand the meaning of the "set up to track 'origin/master'"
> message when they created the branch - especially if they are using a
> GUI.
> 
> Looking at how to fix this, you might think "OK, so disable auto setup
> of remote tracking - set branch.autosetupmerge to false" - but that
> will inconvenience the *second* user in this story - the one who just
> wanted to start working on the feature branch. The first and second
> users swap roles at different points in time of course - they should
> both have a sane configuration that does the right thing in both
> situations.
> 
> Make these flows painless by introducing a new branch.autosetupmerge
> option called "simple", to match the same-name "push.default" option
> that makes similar assumptions.
> 
> This new option automatically sets up tracking in a *subset* of the
> current default situations: when the original ref is a remote tracking
> branch *and* has the same branch name on the remote (as the new local
> branch name).
> 
> With this new configuration, in the example situation above, the first
> user does *not* get origin/master set up as the tracking branch for
> the new local branch. If they "git pull" in their new local-only
> branch, they get an error explaining there is no upstream branch -
> which makes sense and is helpful. If they "git push", they get an
> error explaining how to push *and* suggesting they specify
> --set-upstream - which is exactly the right thing to do for them.
> 
> This new option is likely not appropriate for users intentionally
> implementing a "triangular workflow" with a shared upstream tracking
> branch, that they "git pull" in and a "private" feature branch that
> they push/force-push to just for remote safe-keeping until they are
> ready to push up to the shared branch explicitly/separately. Such
> users are likely to prefer keeping the current default
> merge.autosetupmerge=true behavior, and change their push.default to
> "current".

I think this is a good solution for relatively inexperienced users, and
I don't see any issues with the implementation or tests. However, I
wonder how users for whom this may be useful are going to discover this
option? I don't expect that such users are going to be watching Git's
release notes looking for new features such as this, or carefully
reading documentation changes.

In the discussion on v3 of this series, you mentioned you were thinking
about adding an advice setting to point users here; is there a reason
why that didn't make it into v4? It seems appropriate to me to add one,
perhaps at the point where a user with "autosetupmerge=true" would run
into a failure when trying to push?
Tao Klerks April 20, 2022, 5:12 a.m. UTC | #2
On Mon, Apr 18, 2022 at 8:15 PM Josh Steadmon <steadmon@google.com> wrote:
>
>
> I think this is a good solution for relatively inexperienced users, and
> I don't see any issues with the implementation or tests.

Yay, thanks for the feedback!

> However, I
> wonder how users for whom this may be useful are going to discover this
> option? I don't expect that such users are going to be watching Git's
> release notes looking for new features such as this, or carefully
> reading documentation changes.

Honestly, I was being a bit selfish here - I effectively control the
gitconfig of "my" users, so I was planning on enabling this by default
and letting it "settle in" in git at large, eventually proposing to
change the default.

I understand/agree that this is a little naive - if no-one has reason
to try the new behavior, very little information as to its
usefulness/appropriateness is likely to emerge, and it will never be
an obviously good idea to change the default.

>
> In the discussion on v3 of this series, you mentioned you were thinking
> about adding an advice setting to point users here; is there a reason
> why that didn't make it into v4?

The advice I mentioned I would work on wasn't actually about this new
setting/behavior, but rather about the previously existing (and
reasonably unrelated) "not tracking: ambiguous information for ref"
error, which I found to be unreasonably cryptic.

I submitted that advice change as
https://lore.kernel.org/git/pull.1183.v7.git.1648793113943.gitgitgadget@gmail.com/,
and it's gone out in a recent release.

> It seems appropriate to me to add one,
> perhaps at the point where a user with "autosetupmerge=true" would run
> into a failure when trying to push?

Having thought about this a bit, I agree. On the one hand I'm a little
nervous about adding this kind of public behavior change as I would
imagine it's more likely to encounter resistance here, on the other
hand I do think it will make the changes themselves much more useful.
Also, this patchset hasn't moved in a while, so "holding it up" with
new changes may not be a significant concern.

the current advice looks something like:
---
fatal: The upstream branch of your current branch does not match
the name of your current branch.  To push to the upstream branch
on the remote, use

    git push origin HEAD:master

To push to the branch of the same name on the remote, use

    git push origin HEAD

To choose either option permanently, see push.default in 'git help config'.
---

I would propose to add one sentence at the end along the lines of:
---
To instead avoid automatically configuring upstream branches when
their name doesn't match the local branch, see option 'simple' of
branch.autosetupmerge in 'git help config'.
---

Does that make sense to you?
Josh Steadmon April 20, 2022, 5:19 p.m. UTC | #3
On 2022.04.20 07:12, Tao Klerks wrote:
> On Mon, Apr 18, 2022 at 8:15 PM Josh Steadmon <steadmon@google.com> wrote:
> > It seems appropriate to me to add one,
> > perhaps at the point where a user with "autosetupmerge=true" would run
> > into a failure when trying to push?
> 
> Having thought about this a bit, I agree. On the one hand I'm a little
> nervous about adding this kind of public behavior change as I would
> imagine it's more likely to encounter resistance here, on the other
> hand I do think it will make the changes themselves much more useful.
> Also, this patchset hasn't moved in a while, so "holding it up" with
> new changes may not be a significant concern.
> 
> the current advice looks something like:
> ---
> fatal: The upstream branch of your current branch does not match
> the name of your current branch.  To push to the upstream branch
> on the remote, use
> 
>     git push origin HEAD:master
> 
> To push to the branch of the same name on the remote, use
> 
>     git push origin HEAD
> 
> To choose either option permanently, see push.default in 'git help config'.
> ---
> 
> I would propose to add one sentence at the end along the lines of:
> ---
> To instead avoid automatically configuring upstream branches when
> their name doesn't match the local branch, see option 'simple' of
> branch.autosetupmerge in 'git help config'.
> ---
> 
> Does that make sense to you?

Sounds good to me.
Junio C Hamano April 20, 2022, 5:43 p.m. UTC | #4
Tao Klerks <tao@klerks.biz> writes:

>> However, I
>> wonder how users for whom this may be useful are going to discover this
>> option? I don't expect that such users are going to be watching Git's
>> release notes looking for new features such as this, or carefully
>> reading documentation changes.
>
> Honestly, I was being a bit selfish here - I effectively control the
> gitconfig of "my" users, so I was planning on enabling this by default
> and letting it "settle in" in git at large, eventually proposing to
> change the default.

I am afraid that it is double disservice to your users.  Once they
graduate your organization, they notice that their Git does not work
as they expect and puzzled.

> ...
> To choose either option permanently, see push.default in 'git help config'.
> ---
>
> I would propose to add one sentence at the end along the lines of:
> ---
> To instead avoid automatically configuring upstream branches when
> their name doesn't match the local branch, see option 'simple' of
> branch.autosetupmerge in 'git help config'.
> ---
>
> Does that make sense to you?

Two questions.

 - If a user follows the push.default advice, does it have any
   advantage to set branch.autosetupmerge=simple at all?

 - If a user follows the branch.autosetupmerge=simple advice, what
   happens their "git push" on a branch that the .merge is not set
   due to this configuration?  Shouldn't they have to set up the
   push.default for these branches anyway?

While it might be a good thing to mention branch.autosetupmerge
configuration variable, I am not sure if "To instead avoid" is a
good thing to say here.  It sounds as if the user can ignore
push.default as long as branch.autosetupmerge is taken care of, but
I suspect that is not the case.  Setting the latter to 'simple'
means there are *MORE* branches that do not have .remote/.merge set
up, doesn't it?  Which in turn means that we are relying more on
what push.default is set to, right?
Tao Klerks April 20, 2022, 9:31 p.m. UTC | #5
On Wed, Apr 20, 2022 at 7:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tao Klerks <tao@klerks.biz> writes:
>
> > ...
> > To choose either option permanently, see push.default in 'git help config'.
> > ---
> >
> > I would propose to add one sentence at the end along the lines of:
> > ---
> > To instead avoid automatically configuring upstream branches when
> > their name doesn't match the local branch, see option 'simple' of
> > branch.autosetupmerge in 'git help config'.
> > ---
> >
> > Does that make sense to you?
>
> Two questions.
>
>  - If a user follows the push.default advice, does it have any
>    advantage to set branch.autosetupmerge=simple at all?

Probably not?

It really depends what they set push.default to:
* If they set it to upstream/tracking, then
branch.autosetupmerge=simple doesn't make much sense. You can set
both, but the outcome is effectively the same as setting push.default
to simple - not very useful.
* If they set it to "current", then it probably doesn't make sense
because what they're angling for is probably a triangular workflow,
which branch.autosetupmerge=simple very explicitly doesn't support /
doesn't make sense for. "matching" seems to be an extreme version of
the same setup.
* If they set it to "nothing" I'm not sure - I haven't understood in
what workflows that makes sense.

Generally, I expect that branch.autosetupmerge=simple makes the most
sense with push.default left at the default of "simple", for...
"simple" workflows :)

>
>  - If a user follows the branch.autosetupmerge=simple advice, what
>    happens their "git push" on a branch that the .merge is not set
>    due to this configuration?  Shouldn't they have to set up the
>    push.default for these branches anyway?

If the user follows the branch.autosetupmerge=simple advice (and
leaves push.default at the "simple" default), what they get at push
time will depend on whether they branched from a same-name remote
branch or anything else:

If they branched from a same-name remote branch, their "git push" will
be perfectly uneventful / unsurprising: they will simply push to the
remote branch. This is the same as without
branch.autosetupmerge=simple.

If they branched from a different-name remote branch (they created an
new / independent local branch), then no remote tracking relationship
will have been set up, and instead of the "fatal: The upstream branch
of your current branch does not match
the name of your current branch" error and advice, they will get a
much simpler error and advice:

---
fatal: The current branch whatevs has no upstream branch.
To push the current branch and set the remote as upstream, use

    git push --set-upstream origin whatevs
---

When they follow those instructions, they will be in the "simple"
setup same as if they had just branched from same-name.

Importantly, as soon as they enable branch.autosetupmerge=simple, they
never see the original mismatching-name error and advice anymore -
they never again end up with mismatching names at all. (except in edge
cases like branch renames)

>
> While it might be a good thing to mention branch.autosetupmerge
> configuration variable, I am not sure if "To instead avoid" is a
> good thing to say here.  It sounds as if the user can ignore
> push.default as long as branch.autosetupmerge is taken care of, but
> I suspect that is not the case.

I disagree. If they get that error and advice, then their push.default
is set to "simple". If they then set their branch.autosetupmerge to
"simple" also, this is the simple coherent setup that I, at least,
would recommend to non-experts.

> Setting the latter to 'simple'
> means there are *MORE* branches that do not have .remote/.merge set
> up, doesn't it?  Which in turn means that we are relying more on
> what push.default is set to, right?

No - the idea here is that instead of telling push.default to do
something *independent* of the tracking branch (like, for example,
"current"), the setup the user ends up with is one where the tracking
branch, if there is one, is always the same-name where you will push
to.

When you create a new branch (by branching with a new name), your new
branch doesn't initially have an upstream tracking branch - and that's
right and correct, there's literally nothing on the server for you to
track yet - but the first time you push, the (existing) advice
encourages you to set up that tracking relationship. In this flow you
very explicitly *don't* rely on push.default, because you never want
to end up in a confusing (un-simple) situation where what you're
pulling from and what you're pushing to aren't the same thing - a
triangular workflow.

The "push the current branch and set the remote as upstream" advice is
consistent with how many/most GUIs will handle first push for a branch
that does not have an upstream tracking relationship yet - GUIs will
typically automatically specify (or set the UI default to) the
"--set-upstream" option on that first push.
Junio C Hamano April 21, 2022, 1:53 a.m. UTC | #6
Tao Klerks <tao@klerks.biz> writes:

> If they branched from a different-name remote branch (they created an
> new / independent local branch), then no remote tracking relationship
> will have been set up, and instead of the "fatal: The upstream branch
> of your current branch does not match
> the name of your current branch" error and advice, they will get a
> much simpler error and advice:
>
> ---
> fatal: The current branch whatevs has no upstream branch.
> To push the current branch and set the remote as upstream, use
>
>     git push --set-upstream origin whatevs
> ---
>
> When they follow those instructions, they will be in the "simple"
> setup same as if they had just branched from same-name.

Which means that they need to see an error once, offered to either
set push.default or branch.autosetupmerge (it is not "and/or", but
"or", because you want to tell them to set "instead of push.default,
set branch.autosetupmerge"), and if they follow the latter, they have
to then hit a different error and be told to do the "set-upstream"
individually.  I am wondering if that is more irritating than it is
worth.  Instead, if you tell them to use branch.autosetupmerge=simple
and use push.default to something better than simple, wouldn't that
cover more cases and give fewer roadblocks to the end-user with
unnecessary errors?

>> Setting the latter to 'simple'
>> means there are *MORE* branches that do not have .remote/.merge set
>> up, doesn't it?  Which in turn means that we are relying more on
>> what push.default is set to, right?
>
> No

Why no?  if setupauto is yes, then any new branch forked from a
remote-tracking branch will get .remote/.merge set up, and with these
specific configuration they can "push" back to the configured place.
If it is set to simple, only new branches forked from a remote-tracking
branch that happens to have the same name will get it, and others do
not get .remote/.merge set up.  Which means user's "git push" will then
consult push.default settings, and setting it right becomes more 
important, no?
Tao Klerks April 21, 2022, 10:04 a.m. UTC | #7
On Thu, Apr 21, 2022 at 3:53 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tao Klerks <tao@klerks.biz> writes:
>
> > If they branched from a different-name remote branch (they created an
> > new / independent local branch), then no remote tracking relationship
> > will have been set up, and instead of the "fatal: The upstream branch
> > of your current branch does not match
> > the name of your current branch" error and advice, they will get a
> > much simpler error and advice:
> >
> > ---
> > fatal: The current branch whatevs has no upstream branch.
> > To push the current branch and set the remote as upstream, use
> >
> >     git push --set-upstream origin whatevs
> > ---
> >
> > When they follow those instructions, they will be in the "simple"
> > setup same as if they had just branched from same-name.
>
> Which means that they need to see an error once, offered to either
> set push.default or branch.autosetupmerge (it is not "and/or", but
> "or", because you want to tell them to set "instead of push.default,
> set branch.autosetupmerge"), and if they follow the latter, they have
> to then hit a different error and be told to do the "set-upstream"
> individually.

They don't *have* to hit that error, they can set --set-upstream
pre-emptively, but if they're "just following prompts" then that's
what happens, yes.

> I am wondering if that is more irritating than it is
> worth.  Instead, if you tell them to use branch.autosetupmerge=simple
> and use push.default to something better than simple, wouldn't that
> cover more cases and give fewer roadblocks to the end-user with
> unnecessary errors?

I think you're on to something I missed here.

Unfortunately, I'm not sure what "something better than simple" for
push.default actually is, in the current system.

The most obvious option is to set it to "current", so:
- you only get branch-time tracking for same-name branches because of
branch.autosetupmerge=simple, and
- you always get same-name pushes regardless of whether there is an
upstream or not thanks to push.default, so you never see a "do this
other thing to push" message...

But then you have a new problem: While new branches push consistently,
they never have an upstream tracking ref! This in turn means these
no-tracking-ref branches, although they push smoothly, do not show
ahead/behind state in "git status", and simply don't support a regular
"git pull". That's not "simple".

Where I think you're onto something, is that I believe there *should*
be a way to say "if I request a default push and there is *no* remote
tracking branch, then just push to the first remote, using the same
branch name, *and set up tracking*". Now that would be simple.

I don't know whether that behavior would require yet another
push.default value, or if there's a better way of integrating it into
the existing options/behaviors. I'm also not sure what should happen,
in this scheme, if I happened to clash/overlap with an existing remote
tracking branch. But this does seem like where I would like to end up.

>
> >> Setting the latter to 'simple'
> >> means there are *MORE* branches that do not have .remote/.merge set
> >> up, doesn't it?  Which in turn means that we are relying more on
> >> what push.default is set to, right?
> >
> > No
>
> Why no?  if setupauto is yes, then any new branch forked from a
> remote-tracking branch will get .remote/.merge set up, and with these
> specific configuration they can "push" back to the configured place.
> If it is set to simple, only new branches forked from a remote-tracking
> branch that happens to have the same name will get it, and others do
> not get .remote/.merge set up.

But as long as push.default is set to "simple", *which is the only way
you get the above message ever*, those cases where the new setupauto
option avoids a tracking branch altogether simply change the error
message from "your remote branch name does not match - you have lots
of options" to "you do not have a remote branch yet - push like this
(and you'll be all set for this branch henceforth)".

Insofar as you can only ever get the "you might want to set setupauto
to simple" message when push.default is set to simple, the set of
cases where you get an error on push ends up being the exact same set
of cases - you just get a clearer more sensible error.

> Which means user's "git push" will then
> consult push.default settings, and setting it right becomes more
> important, no?

If there were another push.default option that led to more automatic
*and* correct outcomes, I would agree - and I believe that pursuing
the existence of such an option makes sense.

Do you agree that none of the push.default options available today are
"right" for this flow? Do you have a preference or opinion as to
whether:
* push.default=current should be changed to set up tracking when absent, or
* push.default=simple should be changed to "simply" push and set up
tracking when there is no tracking, or
* a new push.default option should be introduced for this behavior, or
* some other configuration should be introduced to specify "and set up
tracking on default push if missing" (and if so, under what
circumstances should it kick in?)
Junio C Hamano April 22, 2022, 2:27 a.m. UTC | #8
Tao Klerks <tao@klerks.biz> writes:

>> I am wondering if that is more irritating than it is
>> worth.  Instead, if you tell them to use branch.autosetupmerge=simple
>> and use push.default to something better than simple, wouldn't that
>> cover more cases and give fewer roadblocks to the end-user with
>> unnecessary errors?
>
> I think you're on to something I missed here.
>
> Unfortunately, I'm not sure what "something better than simple" for
> push.default actually is, in the current system.

"none", probably.  Much better than "current" that can create new
branches on the other side, which you would want to do with an
explicit end-user instruction (i.e. not with "git push", but with
"git push origin topic").

This depends on what you are really trying to achieve.  If we think
it through, perhaps it may turn out to be a combination of a bit
flawed workflow with a bit inadequate toolset.

With "simple" (both in branch.autosetupmerge and push.default), I
can see that if you create "main" from their "main" and "maint" from
their "maint", you want to see that

 (1) your "git pull" to integrate what happend on their "main" or
     "maint" respectively, and

 (2) your "git push" to push what you did on your "main" to their
     "main", and "maint" to "maint".

But it is totally unclear what you really want to do on "topic" you
created this way:

    $ git checkout -b topic origin/main

Currently, with both set to "simple", you do not even get .remote
and .merge for the "topic" branch, so your "git pull" simply does
not work.  And "git push" will also refuse to work.

But then why are you [*] forking from origin/main in the first
place?  What is the purpose you created 'topic' and what do you
plan to do with the result you develop on 'topic'?

	Side note: "you" do not refer to"Tao, the advocate of the
	'simple' configuration", but figuratively the user who
	followed the "simple" route and created topic out of
	origin/main that is not connected to origin/main.

Whatever you commit on topic eventually becomes part of what you'd
push to origin or elsewhere.  I'd assume it would be origin, because
as the user who choose 'simple', you have some branches that you
push back to the same name over there.  Presumably, those are the
primary integration branches the project has, like 'trunk', 'main',
'master', etc.

So perhaps the user would have been better off to fork off of the
LOCAL branch that would eventually be pushed back?  In other words,
the above user who created 'topic' would have done 

    $ git checkout -b main origin/main

to use as a local integration branch that collects the work you will
do locally that is targetted for their 'main' track, so to create a
topic that aims to be part of what is pushed back to their 'main'
track, you would want to do

    $ git checkout -b topic main

instead?  That way, "git push" would either not get .merge/.remote
(when branch.autosetupmerge is set to 'true') or point at your local
'main' branch.

 - The symptom you get from the former is no better than what you
   get from branch.autosetupmerge=simple but it is not worse.
   "push" and "pull" refuses to work and suggest you to do something
   additional.

 - The latter would make your "git push" and "git pull" on 'topic'
   to work with your local 'main', treating your 'main' in a way
   very similar to how you treat your remote 'main' when you are on
   your own 'main', which is quite reasonable if your change flow is
   designed to be "work on topic, when the changes on topic proves
   OK, send that to main, and when the changes on main proves OK,
   send that to their main".

I guess I am esseentially saying that the usefulness of "simple" for
branch.autosetupmerge is dubious.

> Do you agree that none of the push.default options available today are
> "right" for this flow? Do you have a preference or opinion as to
> whether:
> * push.default=current should be changed to set up tracking when absent, or
> * push.default=simple should be changed to "simply" push and set up
> tracking when there is no tracking, or
> * a new push.default option should be introduced for this behavior, or
> * some other configuration should be introduced to specify "and set up
> tracking on default push if missing" (and if so, under what
> circumstances should it kick in?)

None of the above, I guess.
Tao Klerks April 22, 2022, 9:24 a.m. UTC | #9
On Fri, Apr 22, 2022 at 4:27 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tao Klerks <tao@klerks.biz> writes:
>
> >> I am wondering if that is more irritating than it is
> >> worth.  Instead, if you tell them to use branch.autosetupmerge=simple
> >> and use push.default to something better than simple, wouldn't that
> >> cover more cases and give fewer roadblocks to the end-user with
> >> unnecessary errors?
> >
> > I think you're on to something I missed here.
> >
> > Unfortunately, I'm not sure what "something better than simple" for
> > push.default actually is, in the current system.
>
> "none", probably.  Much better than "current" that can create new
> branches on the other side, which you would want to do with an
> explicit end-user instruction (i.e. not with "git push", but with
> "git push origin topic").
>

Hmm, I don't understand you here. You either mean "simple is the best
option you could choose for push.default, when
branch.autosetupmerge=simple, none of the other options are better",
or there's a small typo and you're saying "push.default=nothing would
be better". I'll assume the latter, but I'm not sure because I don't
see how it can be a good-faith statement.

"nothing" is a good setting for someone who needs and/or wants to make
a conscious choice about where they push to, *every time* they push,
regardless of any remote tracking information. For whom the
operational and mental overhead of choosing a target ref to push to
every time is less than the cost of defaulting to *any* given workflow
or "local to remote branch mapping strategy".

How could this possibly be something we recommend or think is
generally "best" for arbitrary, especially novice, users??

> This depends on what you are really trying to achieve.  If we think
> it through, perhaps it may turn out to be a combination of a bit
> flawed workflow with a bit inadequate toolset.
>
> With "simple" (both in branch.autosetupmerge and push.default), I
> can see that if you create "main" from their "main" and "maint" from
> their "maint", you want to see that
>
>  (1) your "git pull" to integrate what happend on their "main" or
>      "maint" respectively, and
>
>  (2) your "git push" to push what you did on your "main" to their
>      "main", and "maint" to "maint".
>
> But it is totally unclear what you really want to do on "topic" you
> created this way:
>
>     $ git checkout -b topic origin/main
>

The idea of the "simple" workflow which I propose to better support,
is that in creating this branch in this way, you are very clearly
saying:
---
I want to create a new topic branch 'topic' that starts at the current
state of 'origin/main':
* I will want to be able to "back up" my topic branch by pushing it to
the server - and "git push" should do the right thing, just that -
push my current branch to the server.
* I will want to be able to collaborate with others on this topic
branch - after I have pushed, and they have checked out this same
branch, we will all be able to push and pull on this branch seamlessly
* I will choose whether and when to merge in newer changes from
"origin/main", and do so explicitly (not using a simple "git pull")
* I will choose whether and when to rebase on top of "origin/main",
assuming I work alone on this branch or my collaborators are
sufficiently comfortable with rebasing workflows that it will be ok,
and do so explicitly (not using a simple "git pull")
* I will choose whether and when to push my changes on this branch
back to master, and do so by explicitly pushing master after having
merged in this topic branch
---

I would argue that git generally has a "problem", in that
"branch.XXX.merge" entries have two classes of
meanings/interpretation:
* That is the "parent branch"
** The one I want changes from, when I say "update me with changes"
** The one I eventually want to get my changes to
* That is the "remote instance/address" of *this* branch
** If I pull, it's to get changes to the same branch that others might have made
** When I push, it's to get this branch onto the server (not to get my
changes into the "upstream")

For local-only branches, git currently encourages the former
interpretation; when you create the new branch, by default you get the
tracking branch set up.

As soon as you wish to keep your changes on the remote, however, and
especially if you're going to share this topic branch with others,
*you have to give up that interpretation*, for that branch! Git does
not provide you with any facility to indicate a "parent branch",
besides using "branch.XXX.merge" on *local* branches. There's also no
way to share a concept of "parent branch" with others, via a remote,
inherent in git. You can of course look at the commits on your branch
and compare to the commit histories of other branches, you can use
naming strategies to indicate an intended parent, etc - but there's no
inherent storage and signalling mechanism for this idea of a "parent
branch" that you create a topic branch from, with the intent of
merging back eventually.

I call this a "problem" because, in my experience, it confuses people.
The system defaults to setting a "branch.XXX.merge" relationship,
presumably in the hopes of being helpful, but as soon as you want to
share your topic branch (or even just back it up on the server without
jumping through strange hoops), you need to give up that "helpfulness"
and switch to the other model where that just stores the remote
instance of the same branch, rather than the parent.

My proposal here is to support a workflow that accepts, and assumes,
that git does not really have the concept of a "parent branch", and
that "branch.XXX.merge" relationships exist primarily to support the
relationship between local branches and their remote instances.

That of course introduces a tradeoff/compromise: What the user loses,
in this workflow, is the ability to have many very short-lived
local-only branches, all with the same "branch.XXX.merge" upstream,
treating that upstream as the "parent" implicitly. This workflow does
not of course prevent you or discourage you from creating lots of
short-lived local branches - but it does take away the *assumption*
that they're local-only, and the corresponding facility to treat the
upstream as the thing push and pull should work with.

Based on your feedback here, maybe "simple" is not the right name to
associate with workflow, its assumptions and tradeoffs - I believe is
accurately represents the intent and closely relates to the apparent
design intent behind the push.default=simple option, but I'd love
proposals as to how to name (and do) it better!

> Currently, with both set to "simple", you do not even get .remote
> and .merge for the "topic" branch, so your "git pull" simply does
> not work.  And "git push" will also refuse to work.
>

That's right - because the assumption is that you've just created a
new independent branch - independent by name, and therefore
independent by default. You can of course add "--track" if you know
what you're doing and know this is a local-only branch and you want it
to track what you branched from and have "pull" bring in changes from
there (without explicitly specifying so)!

> But then why are you [*] forking from origin/main in the first
> place?  What is the purpose you created 'topic' and what do you
> plan to do with the result you develop on 'topic'?

The assumption, in this workflow, is that you plan to work on that
branch, potentially push to origin to back up or share your work, and
will decide explicitly when to merge in changes from the origin you
branched (forked) from, or merge changes up there.

>
>         Side note: "you" do not refer to"Tao, the advocate of the
>         'simple' configuration", but figuratively the user who
>         followed the "simple" route and created topic out of
>         origin/main that is not connected to origin/main.
>
> Whatever you commit on topic eventually becomes part of what you'd
> push to origin or elsewhere.  I'd assume it would be origin, because
> as the user who choose 'simple', you have some branches that you
> push back to the same name over there.  Presumably, those are the
> primary integration branches the project has, like 'trunk', 'main',
> 'master', etc.
>
> So perhaps the user would have been better off to fork off of the
> LOCAL branch that would eventually be pushed back?  In other words,
> the above user who created 'topic' would have done
>
>     $ git checkout -b main origin/main
>

(completely beside the point, but they would be more likely to have
just done "git checkout main", for the same outcome)

> to use as a local integration branch that collects the work you will
> do locally that is targetted for their 'main' track, so to create a
> topic that aims to be part of what is pushed back to their 'main'
> track, you would want to do
>
>     $ git checkout -b topic main
>
> instead?  That way, "git push" would either not get .merge/.remote
> (when branch.autosetupmerge is set to 'true') or point at your local
> 'main' branch.

I'm not sure I understand or agree with what you're saying here with
"would [otherwise] point at your local 'main' branch". I have to
assume you mean that would be the outcome with "always", while the
former would be the outcome with "true" and "false" (and the proposed
"simple"), and there would be a third possible outcome with "inherit",
where "topic" would end up tracking "origin/main" directly.

>
>  - The symptom you get from the former is no better than what you
>    get from branch.autosetupmerge=simple but it is not worse.
>    "push" and "pull" refuses to work and suggest you to do something
>    additional.

In suggesting the user could/should have done that (in order to get a
sane workflow, presumably), you are also suggesting that they should
keep the state of that "local version of the upstream they eventually
want to get their changes into" up-to-date: They should first check
out master (for example), pull on master to get the state they expect,
and *then* create their new differently-named local branch.

If they take a shortcut (specify the origin branch), they would get
the wrong behavior, and stand a good chance of not understanding what
is happening. I think this is a "bad" process - a bad thing to force
users to learn/understand in order for them to be productive.

>
>  - The latter would make your "git push" and "git pull" on 'topic'
>    to work with your local 'main', treating your 'main' in a way
>    very similar to how you treat your remote 'main' when you are on
>    your own 'main', which is quite reasonable if your change flow is
>    designed to be "work on topic, when the changes on topic proves
>    OK, send that to main, and when the changes on main proves OK,
>    send that to their main".

(assuming you were referring to a "branch.autosetupmerge=always" outcome)

It can be considered "reasonable" if this branch is local-only, yes.
As a user, you then need to understand this duality / distinction
between local-only branches that pull directly against some "semantic
upstream", and local-and-remote branches that

>
> I guess I am esseentially saying that the usefulness of "simple" for
> branch.autosetupmerge is dubious.
>

I understand that, and respectfully disagree :)

> > Do you agree that none of the push.default options available today are
> > "right" for this flow? Do you have a preference or opinion as to
> > whether:
> > * push.default=current should be changed to set up tracking when absent, or
> > * push.default=simple should be changed to "simply" push and set up
> > tracking when there is no tracking, or
> > * a new push.default option should be introduced for this behavior, or
> > * some other configuration should be introduced to specify "and set up
> > tracking on default push if missing" (and if so, under what
> > circumstances should it kick in?)
>
> None of the above, I guess.

I made a mistake here, in under-emphasising "for this flow"; your
answer seems to be more of an "in general, git is powerful enough that
if the user knows to and chooses to do the right thing, they get the
right outcome, and this proposed flow is flawed because it
under-supports local short-lived never-individually-pushed branches".
I completely agree with the former, and while I agree I would love to
have an even better flow that could easily and transparently support
"parent branch" and "server representation of the branch" as separate
concepts - git simply isn't there at this time (and I don't know how
to get it there, and I suspect you and others would not want to bake
such concepts into git).

You stated earlier that I would do my users a disservice in setting
things up to support this flow (by default), without making it
significantly discoverable for the wider git user community, because
they would find that git later behaved differently in other settings.
This is true, but doing them a disservice in terms of git expertise
across contexts is far less important, to me in my context, than
making them comfortable and productive in this specific context - and
coming from , the blinkered workflow I propose is a veritable utopia
of power & flexibility compared to the very-central VCS they come
from. It is also well in-line with how our governance processes and
DevOps processes work, in terms of the meaning of "branch" on the
shared server.

Anyway, I've gone way off-topic I think. I hope I can convince you
that this workflow makes sense for some segment of the git current and
future population, that (with adjustments yet to be made), pushing to
same-name on the remote with tracking implicitly/by default makes
sense, and that making this workflow discoverable to users beyond my
org would also have value.
Tao Klerks April 22, 2022, 1:27 p.m. UTC | #10
On Fri, Apr 22, 2022 at 11:24 AM Tao Klerks <tao@klerks.biz> wrote:
>
> On Fri, Apr 22, 2022 at 4:27 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Tao Klerks <tao@klerks.biz> writes:
> >
> > >
> > > Unfortunately, I'm not sure what "something better than simple" for
> > > push.default actually is, in the current system.
> >
> > "none", probably.  Much better than "current" that can create new
> > branches on the other side, which you would want to do with an
> > explicit end-user instruction (i.e. not with "git push", but with
> > "git push origin topic").
> >
>
> Hmm, I don't understand you here. You either mean "simple is the best
> option you could choose for push.default, when
> branch.autosetupmerge=simple, none of the other options are better",
> or there's a small typo and you're saying "push.default=nothing would
> be better". I'll assume the latter, but I'm not sure because I don't
> see how it can be a good-faith statement.
>

My apologies for the tone here, I clearly sent before re-reading
properly. I cannot presume to state what you did mean; I meant to say
something like "I assume you mean either X or Y",  or "my plausible
interpretations are X or Y", or something similarly reflective of my
own limitations.
Junio C Hamano April 23, 2022, 4:44 a.m. UTC | #11
Tao Klerks <tao@klerks.biz> writes:

>> "none", probably.  Much better than "current" that can create new
>> branches on the other side, which you would want to do with an
>> explicit end-user instruction (i.e. not with "git push", but with
>> "git push origin topic").

Sorry, "nothing" was what I meant.  Those non-simple branches are
left unconfigured with ASU=simple.  We both agree that the user does
not want to see the "with push.default=simple you have, you cannot
push from it" but the reason why they do not want to see can be
multiple.  You are assuming that they want to push to somewhere
else.  I am taking into account that they may not want to push them
at all, but only use locally.  If the intended workflow is

	git checkout -b main [origin/main] ;# assuming DWIM is on 
	git checkout -b topic origin/main
	... work work work ...
	git checkout main
	git merge topic
	... test test test ...
	... ahh, no it does not work, back to fix topic ...
	git reset --hard ORIG_HEAD
	git checkout topic
	... work work work ...
	git rebase -i ...
	git checkout main
	git merge topic
	... test test test ...
        ... yay, this time it is perfect and we can push it out ...
	git push

i.e. matching "simple" branches like main are used to locally bundle
what you locally worked on, and the result is pushed out to the
other side from there, while non-simple branches like topic are used
to locally work on your real changes, it is reasonable to expect
that the user wants "git push" to fail when the 'topic' branch is
checked out.

But unfortunately that does not work at the last step, as "nothing"
unfortunately affects the last step that tries to check out 'main',
too.  push.default='simple' would make it work.

> I would argue that git generally has a "problem", in that
> "branch.XXX.merge" entries have two classes of
> meanings/interpretation:
> * That is the "parent branch"
> ** The one I want changes from, when I say "update me with changes"
> ** The one I eventually want to get my changes to
> * That is the "remote instance/address" of *this* branch
> ** If I pull, it's to get changes to the same branch that others might have made
> ** When I push, it's to get this branch onto the server (not to get my
> changes into the "upstream")

Yes, that is very well known, and there arey mechanisms to support
some workflows that separates "where I get changes from" and "where
I publish my work" (look for "triangular workflows" in the list
archive).  

The thing is, "simple" is *NOT* meant for triangular workflow.  It
was to cater to novice users who are used to cvs/svn style
centralized "there is one place everybody pulls from and pushes to,
which is where they meet" model.
> Based on your feedback here, maybe "simple" is not the right name to
> associate with workflow, its assumptions and tradeoffs - I believe is
> accurately represents the intent and closely relates to the apparent
> design intent behind the push.default=simple option, but I'd love
> proposals as to how to name (and do) it better!
>
>> Currently, with both set to "simple", you do not even get .remote
>> and .merge for the "topic" branch, so your "git pull" simply does
>> not work.  And "git push" will also refuse to work.
>>
>
> That's right - because the assumption is that you've just created a
> new independent branch - independent by name, and therefore
> independent by default. You can of course add "--track" if you know
> what you're doing and know this is a local-only branch and you want it
> to track what you branched from and have "pull" bring in changes from
> there (without explicitly specifying so)!
>
>> But then why are you [*] forking from origin/main in the first
>> place?  What is the purpose you created 'topic' and what do you
>> plan to do with the result you develop on 'topic'?
>
> The assumption, in this workflow, is that you plan to work on that
> branch, potentially push to origin to back up or share your work, and
> will decide explicitly when to merge in changes from the origin you
> branched (forked) from, or merge changes up there.
>
>>
>>         Side note: "you" do not refer to"Tao, the advocate of the
>>         'simple' configuration", but figuratively the user who
>>         followed the "simple" route and created topic out of
>>         origin/main that is not connected to origin/main.
>>
>> Whatever you commit on topic eventually becomes part of what you'd
>> push to origin or elsewhere.  I'd assume it would be origin, because
>> as the user who choose 'simple', you have some branches that you
>> push back to the same name over there.  Presumably, those are the
>> primary integration branches the project has, like 'trunk', 'main',
>> 'master', etc.
>>
>> So perhaps the user would have been better off to fork off of the
>> LOCAL branch that would eventually be pushed back?  In other words,
>> the above user who created 'topic' would have done
>>
>>     $ git checkout -b main origin/main
>>
>
> (completely beside the point, but they would be more likely to have
> just done "git checkout main", for the same outcome)
>
>> to use as a local integration branch that collects the work you will
>> do locally that is targetted for their 'main' track, so to create a
>> topic that aims to be part of what is pushed back to their 'main'
>> track, you would want to do
>>
>>     $ git checkout -b topic main
>>
>> instead?  That way, "git push" would either not get .merge/.remote
>> (when branch.autosetupmerge is set to 'true') or point at your local
>> 'main' branch.
>
> I'm not sure I understand or agree with what you're saying here with
> "would [otherwise] point at your local 'main' branch". I have to
> assume you mean that would be the outcome with "always",

Yeah, I meant to add the matching (when ... is set to ...) after the
sentence and forgot.  You inferred what I meant to say correctly.

> In suggesting the user could/should have done that (in order to get a
> sane workflow, presumably), you are also suggesting that they should
> keep the state of that "local version of the upstream they eventually
> want to get their changes into" up-to-date: They should first check
> out master (for example), pull on master to get the state they expect,
> and *then* create their new differently-named local branch.

FWIW, I am not.

I do not think it is healthy nor necessary to make your local work
"catch up" too often with the outside world unnecessarily, be it
done with rebase or with merge.  They _can_ update 'master' when
outside world has something worth adding to your topic extra
dependency on and then update 'topic' to include what you took to
'master' from the outside.  Dissociating the 'topic' from outside
world is one way to encourage a better workflow.
Tao Klerks April 24, 2022, 11:57 a.m. UTC | #12
On Sat, Apr 23, 2022 at 6:44 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tao Klerks <tao@klerks.biz> writes:
>
> >> "none", probably.  Much better than "current" that can create new
> >> branches on the other side, which you would want to do with an
> >> explicit end-user instruction (i.e. not with "git push", but with
> >> "git push origin topic").
>
> Sorry, "nothing" was what I meant.  Those non-simple branches are
> left unconfigured with ASU=simple.  We both agree that the user does
> not want to see the "with push.default=simple you have, you cannot
> push from it" but the reason why they do not want to see can be
> multiple.  You are assuming that they want to push to somewhere
> else.  I am taking into account that they may not want to push them
> at all, but only use locally.  If the intended workflow is
>
>         git checkout -b main [origin/main] ;# assuming DWIM is on
>         git checkout -b topic origin/main
>         ... work work work ...
>         git checkout main
>         git merge topic
>         ... test test test ...
>         ... ahh, no it does not work, back to fix topic ...
>         git reset --hard ORIG_HEAD
>         git checkout topic
>         ... work work work ...
>         git rebase -i ...
>         git checkout main
>         git merge topic
>         ... test test test ...
>         ... yay, this time it is perfect and we can push it out ...
>         git push

(two interesting/surprising things here:
1) The user chooses to merge to master *before* testing (does not test on topic)
2) The user does not use CI pipelines of any kind
)

>
> i.e. matching "simple" branches like main are used to locally bundle
> what you locally worked on, and the result is pushed out to the
> other side from there, while non-simple branches like topic are used
> to locally work on your real changes, it is reasonable to expect
> that the user wants "git push" to fail when the 'topic' branch is
> checked out.

I would argue that the user who wants push to fail here is a very rare
user. Presumably they thought they were somewhere else, and the "git
push" was a complete mistake? (why else would you run the command??)

For users of a central repo (github, gitlab, bitbucket, teamhub, etc
etc) when they say "git push" it is normally/typically to, well...
push their changes to the remote! To back them up because they're
worried about losing them, or to share them with
teammates/collaborators, to run CI, or for any other reason.

I'm not arguing that "nothing" is useless to everyone, but I am
arguing that for the workflow you have highlighted above, "nothing" is
not a valuable setting. It provides close to no practical safety
benefits, and makes pushing shared branches much more awkward.

>
> But unfortunately that does not work at the last step, as "nothing"
> unfortunately affects the last step that tries to check out 'main',
> too.  push.default='simple' would make it work.
>

OK

> > I would argue that git generally has a "problem", in that
> > "branch.XXX.merge" entries have two classes of
> > meanings/interpretation:
> > * That is the "parent branch"
> > ** The one I want changes from, when I say "update me with changes"
> > ** The one I eventually want to get my changes to
> > * That is the "remote instance/address" of *this* branch
> > ** If I pull, it's to get changes to the same branch that others might have made
> > ** When I push, it's to get this branch onto the server (not to get my
> > changes into the "upstream")
>
> Yes, that is very well known, and there arey mechanisms to support
> some workflows that separates "where I get changes from" and "where
> I publish my work" (look for "triangular workflows" in the list
> archive).

Yes, the best/simplest summary I've seen so far is the github blog
post https://github.blog/2015-07-29-git-2-5-including-multiple-worktrees-and-triangular-workflows/

In the particular model highlighted there, you use "branch.XXX.merge"
entries to indicate "the parent branch" while keeping the "remote
branch instance" separate, by leveraging the "push.default=current"
setting, but of course this "parent branch" information is still
local-only, and you cannot collaborate on your feature/topic branch
with others. If someone else checks out your topic branch from your
server and pushes some changes to it, then your flow breaks, because
your "git pull" means "bring in changes from the parent", not "bring
in any changes that might have occurred on the topic branch".

I understand there are techniques/flows that users can choose to use,
but I don't think this changes the fundamental and, for beginners,
problematic, ambiguity of meaning of "branch.XXX.merge". The
"branch.autosetupmerge=simple" proposal is to simplify it down to
"branch.XXX.merge entries indicate what the remote instance of this
branch is, and will normally be aligned with the name of the local
branch". This "simplification" is incompatible with the particular
triangular workflow highlighted above.

>
> > In suggesting the user could/should have done that (in order to get a
> > sane workflow, presumably), you are also suggesting that they should
> > keep the state of that "local version of the upstream they eventually
> > want to get their changes into" up-to-date: They should first check
> > out master (for example), pull on master to get the state they expect,
> > and *then* create their new differently-named local branch.
>
> FWIW, I am not.

Fair enough, sorry I misunderstood. What I meant is that you need to
"maintain" your local master when you do eventually want to push up
any topic branch, *and* any other time you do want to "catch up" with
upstream changes; assuming you work on multiple topic branches in
parallel (which is one of the "superpowers" of git), the local master
has lots of different reasons to change.

>
> I do not think it is healthy nor necessary to make your local work
> "catch up" too often with the outside world unnecessarily, be it
> done with rebase or with merge.  They _can_ update 'master' when
> outside world has something worth adding to your topic extra
> dependency on and then update 'topic' to include what you took to
> 'master' from the outside.  Dissociating the 'topic' from outside
> world is one way to encourage a better workflow.

On this we agree, I guess :)


I will have another go at proposing a complete, easy-to-understand,
easy-to-enter, "simple" workflow that emphasises local and remote
branch "correspondence" by encouraging "branch.XXX.merge" to always
and automatically be set to the same-name branch on the remote (and
not any other "parent" you might have branched from when creating a
topic branch), and a reasonable non-intrusive, non-misleading way to
on-ramp into it.
Tao Klerks April 29, 2022, 7:31 a.m. UTC | #13
On Sun, Apr 24, 2022 at 1:57 PM Tao Klerks <tao@klerks.biz> wrote:
>
>
> I will have another go at proposing a complete, easy-to-understand,
> easy-to-enter, "simple" workflow that emphasises local and remote
> branch "correspondence" by encouraging "branch.XXX.merge" to always
> and automatically be set to the same-name branch on the remote (and
> not any other "parent" you might have branched from when creating a
> topic branch), and a reasonable non-intrusive, non-misleading way to
> on-ramp into it.

I now have a complete proposal that I think is coherent, clear,
improves the user experience as desired, and does not interfere with
any of the existing functionality/workflows. There are a couple
niggles around naming that I'd like feedback on. I expect to submit
this new proposal today.

(nb: it's become a patch series again)
diff mbox series

Patch

diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt
index 1e0c7af014b..8df10d07129 100644
--- a/Documentation/config/branch.txt
+++ b/Documentation/config/branch.txt
@@ -9,7 +9,9 @@  branch.autoSetupMerge::
 	automatic setup is done when the starting point is either a
 	local branch or remote-tracking branch; `inherit` -- if the starting point
 	has a tracking configuration, it is copied to the new
-	branch. This option defaults to true.
+	branch; `simple` -- automatic setup is done only when the starting point
+	is a remote-tracking branch and the new branch has the same name as the
+	remote branch. This option defaults to true.
 
 branch.autoSetupRebase::
 	When a new branch is created with 'git branch', 'git switch' or 'git checkout'
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index c8b4f9ce3c7..ae82378349d 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -221,13 +221,17 @@  The exact upstream branch is chosen depending on the optional argument:
 itself as the upstream; `--track=inherit` means to copy the upstream
 configuration of the start-point branch.
 +
-`--track=direct` is the default when the start point is a remote-tracking branch.
-Set the branch.autoSetupMerge configuration variable to `false` if you
-want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track`
-were given. Set it to `always` if you want this behavior when the
-start-point is either a local or remote-tracking branch. Set it to
-`inherit` if you want to copy the tracking configuration from the
-branch point.
+The branch.autoSetupMerge configuration variable specifies how `git switch`,
+`git checkout` and `git branch` should behave when neither `--track` nor
+`--no-track` are specified:
++
+The default option, `true`, behaves as though `--track=direct`
+were given whenever the start-point is a remote-tracking branch.
+`false` behaves as if `--no-track` were given. `always` behaves as though
+`--track=direct` were given. `inherit` behaves as though `--track=inherit`
+were given. `simple` behaves as though `--track=direct` were given only when
+the start-point is a remote-tracking branch and the new branch has the same
+name as the remote branch.
 +
 See linkgit:git-pull[1] and linkgit:git-config[1] for additional discussion on
 how the `branch.<name>.remote` and `branch.<name>.merge` options are used.
diff --git a/branch.c b/branch.c
index 6b31df539a5..86ea91e76f8 100644
--- a/branch.c
+++ b/branch.c
@@ -30,9 +30,9 @@  static int find_tracked_branch(struct remote *remote, void *priv)
 			free(tracking->spec.src);
 			string_list_clear(tracking->srcs, 0);
 		}
+		/* remote_find_tracking() searches by src if present */
 		tracking->spec.src = NULL;
 	}
-
 	return 0;
 }
 
@@ -243,19 +243,42 @@  static void setup_tracking(const char *new_ref, const char *orig_ref,
 
 	if (!tracking.matches)
 		switch (track) {
+		/* If ref is not remote, still use local */
 		case BRANCH_TRACK_ALWAYS:
 		case BRANCH_TRACK_EXPLICIT:
 		case BRANCH_TRACK_OVERRIDE:
+		/* Remote matches not evaluated */
 		case BRANCH_TRACK_INHERIT:
 			break;
+		/* Otherwise, if no remote don't track */
 		default:
 			goto cleanup;
 		}
 
+	/*
+	 * This check does not apply to BRANCH_TRACK_INHERIT;
+	 * that supports multiple entries in tracking_srcs but
+	 * leaves tracking.matches at 0.
+	 */
 	if (tracking.matches > 1)
 		die(_("not tracking: ambiguous information for ref %s"),
 		    orig_ref);
 
+	if (track == BRANCH_TRACK_SIMPLE) {
+		/*
+		 * Only track if remote branch name matches.
+		 * Reaching into items[0].string is safe because
+		 * we know there is at least one and not more than
+		 * one entry (because only BRANCH_TRACK_INHERIT can
+		 * produce more than one entry).
+		 */
+		const char *tracked_branch;
+		if (!skip_prefix(tracking.srcs->items[0].string,
+				 "refs/heads/", &tracked_branch) ||
+		    strcmp(tracked_branch, new_ref))
+			return;
+	}
+
 	if (tracking.srcs->nr < 1)
 		string_list_append(tracking.srcs, orig_ref);
 	if (install_branch_config_multiple_remotes(config_flags, new_ref,
diff --git a/branch.h b/branch.h
index 04df2aa5b51..560b6b96a8f 100644
--- a/branch.h
+++ b/branch.h
@@ -12,6 +12,7 @@  enum branch_track {
 	BRANCH_TRACK_EXPLICIT,
 	BRANCH_TRACK_OVERRIDE,
 	BRANCH_TRACK_INHERIT,
+	BRANCH_TRACK_SIMPLE,
 };
 
 extern enum branch_track git_branch_track;
diff --git a/config.c b/config.c
index e78397725c9..8de87400085 100644
--- a/config.c
+++ b/config.c
@@ -1686,6 +1686,9 @@  static int git_default_branch_config(const char *var, const char *value)
 		} else if (value && !strcmp(value, "inherit")) {
 			git_branch_track = BRANCH_TRACK_INHERIT;
 			return 0;
+		} else if (value && !strcmp(value, "simple")) {
+			git_branch_track = BRANCH_TRACK_SIMPLE;
+			return 0;
 		}
 		git_branch_track = git_config_bool(var, value);
 		return 0;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 7a0ff75ba86..7a5a44a1ebf 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -886,6 +886,41 @@  test_expect_success 'branch from tag w/--track causes failure' '
 	test_must_fail git branch --track my11 foobar
 '
 
+test_expect_success 'simple tracking works when remote branch name matches' '
+	test_when_finished "rm -rf otherserver" &&
+	git init otherserver &&
+	test_commit -C otherserver my_commit 1 &&
+	git -C otherserver branch feature &&
+	test_config branch.autosetupmerge simple &&
+	test_config remote.otherserver.url otherserver &&
+	test_config remote.otherserver.fetch refs/heads/*:refs/remotes/otherserver/* &&
+	git fetch otherserver &&
+	git branch feature otherserver/feature &&
+	test_cmp_config otherserver branch.feature.remote &&
+	test_cmp_config refs/heads/feature branch.feature.merge
+'
+
+test_expect_success 'simple tracking skips when remote branch name does not match' '
+	test_config branch.autosetupmerge simple &&
+	test_config remote.local.url . &&
+	test_config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
+	git fetch local &&
+	git branch my-other local/main &&
+	test_cmp_config "" --default "" branch.my-other.remote &&
+	test_cmp_config "" --default "" branch.my-other.merge
+'
+
+test_expect_success 'simple tracking skips when remote ref is not a branch' '
+	test_config branch.autosetupmerge simple &&
+	test_config remote.localtags.url . &&
+	test_config remote.localtags.fetch refs/tags/*:refs/remotes/localtags/* &&
+	git tag mytag12 main &&
+	git fetch localtags &&
+	git branch mytag12 localtags/mytag12 &&
+	test_cmp_config "" --default "" branch.mytag12.remote &&
+	test_cmp_config "" --default "" branch.mytag12.merge
+'
+
 test_expect_success '--set-upstream-to fails on multiple branches' '
 	echo "fatal: too many arguments to set new upstream" >expect &&
 	test_must_fail git branch --set-upstream-to main a b c 2>err &&