On 2022.03.07 13:12, Ævar Arnfjörð Bjarmason wrote: > > On Mon, Mar 07 2022, Tao Klerks wrote: > > > On Sun, Mar 6, 2022 at 10:54 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > >> > >> Glen Choo <chooglen@xxxxxxxxxx> writes: > >> > >> > Not that I would ever _recommend_ someone to work like this though. > >> > Junio also wondered whether anyone uses this [1]. > >> > > >> > [1] https://lore.kernel.org/git/xmqqbl2hw10h.fsf@gitster.g > >> > >> I actually think the current octopus behaviour is a sensible one > >> (after all, that was what we envisioned how it would be used when we > >> did the mechanism long time ago). If you have mutliple, say source > >> and docs, groups working for you and you are taking their work from > >> time to time, something like this: > >> > >> [branch "main"] > >> remote = central-repo > >> merge = sources > >> merge = docs > >> > >> would let your folks push into the central-repo, and a "git pull" > >> will help you advance your main branch that you'll publish later. > >> > >> You can explain multiple .merge entries for such an integration > >> branch like I just did and I think such an explanation makes perfect > >> sense, but these are quite different from what we view as "upstream" > >> in the traditional sense. In the setting illustrated here, yours is > >> the main integration repository, the center of the universe, and > >> those folks working in the 'sources' and 'docs' groups view yours as > >> their "upstream". > >> > >> So, "what does it mean to have multiple branch.*.merge entries" has > >> a good answer: you are integrating from these contributors and these > >> entries are not your "upstream" in the usual sense---you do not even > >> push back to them. Asking "what does it mean to have multiple > >> upstream" makes little sense, I would think. > >> > >> Now, with the 'main' branch used in such a manner, if you did > >> > >> $ git branch --track=inherit topic main > >> > >> and worked on the "topic" branch, you do not push back to either the > >> sources or docs of the central-repo, of course, but it is unclear if > >> you even want to "pull" to create octopus from these two branches at > >> the central-repo, which essentially duplicates the pull's you would > >> do on your 'main' branch. I suspect that you'd rather want to merge > >> updates 'main' accumulated (or rebase on them). > >> > >> The reason why I asked what Josh's plans are for the multiple .merge > >> entries in that thread [1] when the "--inherit" feature was being > >> invented was exactly about this point. I wondered if last-one-wins > >> may make sense (and as the above octopus set-up tells us, it may > >> not), but if we want to keep "multiple .merge entries means an > >> integrator making octpus merges", then it may make sense not to copy > >> them when there are multiple with "--track=inherit", to avoid > >> spreading the "curious" nature of the branch like 'main' depicted > >> above. > > > > Given that the notion of "inherit"ing the tracking configuration is a > > (relatively) new one anyway, and given the slightly esoteric nature of > > the "multiple branch merge entries lead to octopus merges" > > functionality, I would argue that it makes more sense to die when > > branching under this specific configuration, saying something like > > "inheriting the tracking configuration of a branch with multiple > > branch merge entries is not supported - we think you're making a > > mistake". > > I don't know if this is plausible in this case, but we need to be very > careful with that in general. I.e. some people might set the "sensible" > default remote config for "origin" in their ~/.gitconfig or whatever, > including "merge" for a "master" and all. > > Then expect that if they have a local repo that we'll take whatever > custom values there to override them, if any. > > So for most config variables that take a "last set wins" it's a feature > to ignore any previous entries. > > But in this case it might be different due to the odditity of the remote > config, how we almost always manage it with "git remote" or "git clone" > etc. > > > Skipping the creation of tracking entries in this case, even with a > > warning/explanation output to stdout, would be a "slightly hidden > > surprise", in that git command output is often not read by, or even > > visible to users when a command is successful, eg in a GUI. > > > > If we think this will basically never happen and really makes no sense > > anyway, as Junio seems to suggest, then I would argue the extra > > complexity in the codebase to support the "inheriting multiple branch > > merge entries" is unwarranted. > > > > Either way, I will happily drop this topic as it does not appear to > > require follow-up in direct relation to my "--track=simple" > > work/proposal. On the other hand, I'd be happy to work on a patch to > > eliminate this multi-tracking-branch-inheritance path/support (undoing > > some of Josh's work here) if the team believes this makes sense. > > Just on my side: Don't take any of my comments in this thread as a "we > shouldn't do this", it was genuine confusion, thanks for clearing it up > :) > > Perhaps a gently step into adding validation for this (if needed) is to > do the die()/advise() or whatever or the write (i.e. when we copy > branches, or use --track=inherit) v.s. when we use the config (in "git > fetch" et al) ? Sorry for the late response to this thread. I don't have strong feelings regarding either keeping the current --track=inherit behavior or disallowing inheritance of multiple merge options. However, here is my original thinking that led me to the current implementation: If someone is using multiple merge branches, they are already treating the "upstream" config in a somewhat non-standard manner, as Junio already mentioned. Presumably, they know what they're doing. While the --track=inherit mode is intended for automatically setting up *push* configuration, there may be some unforeseen-by-me benefit to also inheriting this non-standard setup. Since there's nothing technical preventing inheriting multiple merge branches, it seems better to trust the user to know what they're doing, rather than put a die() in place to stop them. If folks feel that I'm wrong in my reasoning here, I'm happy to help review patches to fix the issue, and wouldn't feel like anyone is stepping on my toes if they do so :).