[PATCH 5 of 6] update: learn --merge to allow merging across topological branches (issue5125)

Martin von Zweigbergk martinvonz at google.com
Wed Feb 15 13:25:49 EST 2017


On Wed, Feb 15, 2017 at 6:22 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> On Wed, 15 Feb 2017 13:29:30 +0000, Gábor STEFANIK wrote:
>> > >> -def updatetotally(ui, repo, checkout, brev, clean=False, check=False):
>> > >> +def updatetotally(ui, repo, checkout, brev, clean=False,
>> > >> +                  updatecheck='linear'):
>> > >
>> > > Please make this updatecheck=None, and set it to 'linear' inside
>> > > merge.update() explicitly if None was passed to that function.
>> >
>> > Will do.
>> >
>> > >
>> > > This way, in patch 6, you can red the config option inside
>> > > merge.update(), and let other callers (primarily extensions) also
>> > automatically follow the config option.
>> >
>> > I don't think it's a good idea for the low-level merge.update() to read the
>> > config. That function is called e.g. by rebase and graft and they should not
>> > have their behavior changed by this config.
>
> I tend to agree with Martin, low-level functions should have less dependency
> on config. Instead, maybe we could add a function that builds options from
> ui (like patch.diffopts), but I don't know if that's appropriate here.
>
>> I forgot to add, I'm particularly concerned about the "guestrepo" extension,
>> which we use extensively at NNG. IIRC in the "grupdate" command, it doesn't
>> use commands.update, and so changing the config option causes "update"
>> and "grupdate" to behave differently.
>>
>> Also, care must be taken about subrepositories. Not sure if our subrepo update
>> code calls command.update - if not, we may even end up with a situation
>> where "hg update" in a repository with subrepos with ui.updatecheck="merge"
>> will do nonlinear updates with a dirty main repo, but not with a dirty subrepo.
>
> IIRC, updating including subrepos isn't a simple cascading operation. Perhaps
> users would be asked how to process changes in subrepos?
>
>> In fact, if we introduce a config option, but only use it in commands.update(),
>> we should make the lower functions error out or at least warn if called without
>> explicit updatecheck, and without other options (e.g. branchmerge) that override
>> linearity checks. This way, we at least catch extensions that don't follow the
>> config option, but try to rely on merge.update()'s linearity checks.
>
> Good point.

Yeah, I agree with that last point. I tried it and there are several
places that fail. For example, clone, rebase, and mq all call
hg.update[repo](). AFAICT, there should never be any changes where
it's being called, so maybe that should be setting force=True to
overwrite. Or maybe we should teach merge.update() to accept
updatecheck='abort' and make these callers pass that value.
Regardless, since what I'm adding is an experimental config, I'd
prefer to make merge.update() default to 'linear' for now and I'll add
a TODO to add the check you mention. I don't have time right now to
clean up all the other callers, and I'd still like to be able to make
progress on this topic.


More information about the Mercurial-devel mailing list