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

Yuya Nishihara yuya at tcha.org
Wed Feb 15 09:22:56 EST 2017


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.


More information about the Mercurial-devel mailing list