Re: [PATCH v4] branch: add flags and config to inherit tracking

From: Josh Steadmon <steadmon@google.com>
Date: Nov 30, 2021, 22:05 UTC
Message-ID: <YaagLkQOUia7RsYq@google.com>
In-Reply-To: <xmqqbl2hw10h.fsf@gitster.g>

On 2021.11.18 14:29, Junio C Hamano wrote:

Show 21 quoted lines

> Josh Steadmon <steadmon@google.com> writes:
> 
> > I've addressed Glen's feedback from V3. However, this brings up a new
> > issue that was not obvious before: "branch.<name>.merge" can be
> > specified more than once. On the other hand, the existing tracking setup
> > code supports only a single merge entry. For now I'm defaulting to use
> > the first merge entry listed in the branch struct, but I'm curious what
> > people think the best solution would be. This may be another point in
> > favor of Ævar's suggestion to reuse the copy-branch-config machinery.
> 
> Or we can extend "existing tracking setup code" to support multiple
> merge sources.
> 
> How does the "git pull" machinery react to them, by the way?  I
> think the original intention is to support pulling multiple branches
> from the (single) remote configured for the branch with a single
> invocation of "git pull", creating an octopus merge, but does it
> still work, or nobody uses such a crazy curiosity anymore and it was
> once broken and left in non-working state ever since?  What I am
> dreaming here is if we can safely ignore all but one of them, taking
> the usual "last-one-wins" rule, after some transition period.

It does still work and creates an octopus merge. Tested as follows:

$ git clone https://github.com/gitster/git
$ cd git
$ git config pull.rebase false
$ git checkout -b test-branch v2.30.0
$ cat >>.git/config <<EOF
[branch "test-branch"]
	remote = origin
	merge = refs/heads/master
	merge = refs/heads/ab/ambiguous-object-name
	merge = refs/heads/js/scalar
EOF
$ git pull
Fast-forwarding to: 4ef9e1ce4ac4ad79f99f5c5712146254b4cca530
Trying simple merge with 352f8e8fcba4726340c946200149e6285c514fc0
Trying simple merge with abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Simple merge did not work, trying automatic merge.
Auto-merging Makefile
Auto-merging contrib/buildsystems/CMakeLists.txt
Merge made by the 'octopus' strategy.
[...]

$ git cat-file commit HEAD tree 78bd62c62edfc6e51beb956e548b92b97210ddd4 parent 4ef9e1ce4ac4ad79f99f5c5712146254b4cca530 parent 352f8e8fcba4726340c946200149e6285c514fc0 parent abe6bb3905392d5eb6b01fa6e54d7e784e0522aa author Josh Steadmon <josh@steadmon.net> 1638309707 -0800 committer Josh Steadmon <josh@steadmon.net> 1638309707 -0800

Merge branches 'ab/ambiguous-object-name', 'js/scalar' and 'master' of https://github.com/gitster/git into test-branch

Show 18 quoted lines

> > +int parse_opt_tracking_mode(const struct option *opt, const char *arg, int unset) {
> > +	if (unset)
> > +		*(enum branch_track *)opt->value = BRANCH_TRACK_NEVER;
> > +	else if (!arg || !strcmp(arg, "direct"))
> > +		*(enum branch_track *)opt->value = BRANCH_TRACK_EXPLICIT;
> > +	else if (!strcmp(arg, "inherit"))
> > +		*(enum branch_track *)opt->value = BRANCH_TRACK_INHERIT;
> > +	else
> > +		return error(_("option `--track' expects \"direct\" or \"inherit\""));
> 
> According to recent discussion in another thread,
> 
> 	error(_("option '--%s` expects '%s' or '%s'"),
> 		"track", "direct", "inherit");
> 
> would be more translater friendly, as these three words are not
> subject to translation?  I am not sure if it is really worth it,
> though.

I don't really feel strongly either way, so I switched to the more translatable version. I was originally following some other examples in the file, where it seemed that most strings that would require more than one field expansion were just hardcoded instead.