[PATCH 5 of 6] update: learn --merge to allow merging across topological branches (issue5125)
Gábor STEFANIK
Gabor.STEFANIK at nng.com
Wed Feb 15 08:29:30 EST 2017
>
--------------------------------------------------------------------------
This message, including its attachments, is confidential. For more information please read NNG's email policy here:
http://www.nng.com/emailpolicy/
By responding to this email you accept the email policy.
-----Original Message-----
> From: Martin von Zweigbergk [mailto:martinvonz at google.com]
> Sent: Tuesday, February 14, 2017 11:36 PM
> To: Gábor STEFANIK <Gabor.STEFANIK at nng.com>
> Cc: Mercurial-devel <mercurial-devel at mercurial-scm.org>
> Subject: Re: [PATCH 5 of 6] update: learn --merge to allow merging across
> topological branches (issue5125)
>
> (+mercurial-devel)
>
> On Tue, Feb 14, 2017 at 10:48 AM, Gábor STEFANIK
> <Gabor.STEFANIK at nng.com> wrote:
> >>
> >
> >
> > ----------------------------------------------------------------------
> > ---- This message, including its attachments, is confidential. For
> > more information please read NNG's email policy here:
> > http://www.nng.com/emailpolicy/
> > By responding to this email you accept the email policy.
> >
> >
> > -----Original Message-----
> >> From: Mercurial-devel
> >> [mailto:mercurial-devel-bounces at mercurial-scm.org]
> >> On Behalf Of Martin von Zweigbergk via Mercurial-devel
> >> Sent: Tuesday, February 14, 2017 2:07 AM
> >> To: mercurial-devel at mercurial-scm.org
> >> Subject: [PATCH 5 of 6] update: learn --merge to allow merging across
> >> topological branches (issue5125)
> >>
> >> # HG changeset patch
> >> # User Martin von Zweigbergk <martinvonz at google.com> # Date
> >> 1487019517 28800
> >> # Mon Feb 13 12:58:37 2017 -0800
> >> # Node ID 75e5a492d7f69420a554fe498ae24060c755b09f
> >> # Parent 59e2d3da607cc09ebe133ab199fc4f343d74e1e6
> >> update: learn --merge to allow merging across topological branches
> >> (issue5125)
> >>
> >> diff -r 59e2d3da607c -r 75e5a492d7f6 mercurial/commands.py
> >> --- a/mercurial/commands.py Mon Feb 13 15:04:46 2017 -0800
> >> +++ b/mercurial/commands.py Mon Feb 13 12:58:37 2017 -0800
> >> @@ -6471,12 +6471,13 @@
> >> @command('^update|up|checkout|co',
> >> [('C', 'clean', None, _('discard uncommitted changes (no backup)')),
> >> ('c', 'check', None, _('require clean working directory')),
> >> + ('m', 'merge', None, _('merge local changes')),
> >> ('d', 'date', '', _('tipmost revision matching date'), _('DATE')),
> >> ('r', 'rev', '', _('revision'), _('REV'))
> >> ] + mergetoolopts,
> >> - _('[-C|-c] [-d DATE] [[-r] REV]'))
> >> + _('[-C|-c|-m] [-d DATE] [[-r] REV]'))
> >> def update(ui, repo, node=None, rev=None, clean=False, date=None,
> >> check=False,
> >> - tool=None):
> >> + merge=None, tool=None):
> >> """update working directory (or switch revisions)
> >>
> >> Update the repository's working directory to the specified @@
> >> -6498,7 +6499,7 @@
> >> The following rules apply when the working directory contains
> >> uncommitted changes:
> >>
> >> - 1. If neither -c/--check nor -C/--clean is specified, and if
> >> + 1. If none of -c/--check, -C/--clean, or -m/--merge is
> >> + specified, and if
> >> the requested changeset is an ancestor or descendant of
> >> the working directory's parent, the uncommitted changes
> >> are merged into the requested changeset and the merged @@
> >> -6507,10 +6508,14 @@
> >> branch), the update is aborted and the uncommitted changes
> >> are preserved.
> >>
> >> - 2. With the -c/--check option, the update is aborted and the
> >> + 2. With the -m/--merge option, the update is allowed even if the
> >> + requested changeset is not an ancestor or descendant of
> >> + the working directory's parent.
> >> +
> >> + 3. With the -c/--check option, the update is aborted and the
> >> uncommitted changes are preserved.
> >>
> >> - 3. With the -C/--clean option, uncommitted changes are discarded and
> >> + 4. With the -C/--clean option, uncommitted changes are
> >> + discarded and
> >> the working directory is updated to the requested changeset.
> >>
> >> To cancel an uncommitted merge (and lose your changes), use @@
> >> -6535,8 +6540,15 @@
> >> if date and rev is not None:
> >> raise error.Abort(_("you can't specify a revision and a
> >> date"))
> >>
> >> - if check and clean:
> >> - raise error.Abort(_("cannot specify both -c/--check and -C/--clean"))
> >> + if len([x for x in (check, clean , merge) if x]) > 1:
> >> + raise error.Abort(_("can only specify one of -c/--check, -C/--clean, "
> >> + "and -m/merge"))
> >> +
> >> + updatecheck = 'linear'
> >> + if check:
> >> + updatecheck = 'abort'
> >> + elif merge:
> >> + updatecheck = None
> >
> > I don't really like the use of None to indicate "always merge".
> > I would strongly prefer updatecheck = 'merge', with None instead
> > representing a default or "unset" case.
>
> Makes sense. Since the check that's done is not about merging, I'll call it
> 'none' instead, if you don't mind.
>
> >
> > This makes setting the default via a config option (as in your 6th
> > patch) a lot safer and easier - see below.
> >
> >>
> >> with repo.wlock():
> >> cmdutil.clearunfinished(repo) @@ -6550,7 +6562,8 @@
> >>
> >> repo.ui.setconfig('ui', 'forcemerge', tool, 'update')
> >>
> >> - return hg.updatetotally(ui, repo, rev, brev, clean=clean,
> check=check)
> >> + return hg.updatetotally(ui, repo, rev, brev, clean=clean,
> >> + updatecheck=updatecheck)
> >>
> >> @command('verify', [])
> >> def verify(ui, repo):
> >> diff -r 59e2d3da607c -r 75e5a492d7f6 mercurial/hg.py
> >> --- a/mercurial/hg.py Mon Feb 13 15:04:46 2017 -0800
> >> +++ b/mercurial/hg.py Mon Feb 13 12:58:37 2017 -0800
> >> @@ -681,18 +681,19 @@
> >> repo.ui.status(_("%d files updated, %d files merged, "
> >> "%d files removed, %d files unresolved\n") %
> >> stats)
> >>
> >> -def updaterepo(repo, node, overwrite):
> >> +def updaterepo(repo, node, overwrite, updatecheck=None):
> >> """Update the working directory to node.
> >>
> >> When overwrite is set, changes are clobbered, merged else
> >>
> >> returns stats (see pydoc mercurial.merge.applyupdates)"""
> >> return mergemod.update(repo, node, False, overwrite,
> >> - labels=['working copy', 'destination'])
> >> + labels=['working copy', 'destination'],
> >> + updatecheck=updatecheck)
> >>
> >> -def update(repo, node, quietempty=False):
> >> - """update the working directory to node, merging linear changes"""
> >> - stats = updaterepo(repo, node, False)
> >> +def update(repo, node, quietempty=False, updatecheck=None):
> >> + """update the working directory to node"""
> >> + stats = updaterepo(repo, node, False, updatecheck=updatecheck)
> >> _showstats(repo, stats, quietempty)
> >> if stats[3]:
> >> repo.ui.status(_("use 'hg resolve' to retry unresolved file
> >> merges\n")) @@ -712,7 +713,8 @@ # naming conflict in updatetotally()
> >> _clean = clean
> >>
> >> -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 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.
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.
>
> >
> >> """Update the working directory with extra care for non-file
> >> components
> >>
> >> This takes care of non-file components below:
> >> @@ -724,7 +726,14 @@
> >> :checkout: to which revision the working directory is updated
> >> :brev: a name, which might be a bookmark to be activated after
> updating
> >> :clean: whether changes in the working directory can be discarded
> >> - :check: whether changes in the working directory should be checked
> >> + :updatecheck: how to deal with a dirty working directory
> >> +
> >> + Valid values for updatecheck are:
> >> +
> >> + * abort: abort if the working directory is dirty
> >> + * None: don't check (merge working directory changes into
> destination)
> >> + * linear: check that update is linear before merging working directory
> >> + changes into destination
> >>
> >> This returns whether conflict is detected at updating or not.
> >> """
> >> @@ -739,9 +748,10 @@
> >> if clean:
> >> ret = _clean(repo, checkout)
> >> else:
> >> - if check:
> >> + if updatecheck == 'abort':
> >> cmdutil.bailifchanged(repo, merge=False)
> >> - ret = _update(repo, checkout)
> >> + updatecheck = None
> >> + ret = _update(repo, checkout, updatecheck=updatecheck)
> >>
> >> if not ret and movemarkfrom:
> >> if movemarkfrom == repo['.'].node():
> >> diff -r 59e2d3da607c -r 75e5a492d7f6 mercurial/merge.py
> >> --- a/mercurial/merge.py Mon Feb 13 15:04:46 2017 -0800
> >> +++ b/mercurial/merge.py Mon Feb 13 12:58:37 2017 -0800
> >> @@ -1444,7 +1444,8 @@
> >> repo.dirstate.normal(f)
> >>
> >> def update(repo, node, branchmerge, force, ancestor=None,
> >> - mergeancestor=False, labels=None, matcher=None,
> >> mergeforce=False):
> >> + mergeancestor=False, labels=None, matcher=None,
> >> mergeforce=False,
> >> + updatecheck='linear'):
> >
> > Again, default to None here, and explicitly set to 'linear'
> > (and later set it based on config) if it's None.
>
> Will do.
>
> >
> >> """
> >> Perform a merge between the working directory and the given node
> >>
> >> @@ -1494,6 +1495,7 @@
> >> # This functon used to find the default destination if node was None,
> but
> >> # that's now in destutil.py.
> >> assert node is not None
> >> + assert updatecheck in (None, 'linear')
> >> # If we're doing a partial update, we need to skip updating
> >> # the dirstate, so make a note of any partial-ness to the
> >> # update here.
> >> @@ -1550,7 +1552,8 @@
> >> repo.hook('update', parent1=xp2, parent2='', error=0)
> >> return 0, 0, 0, 0
> >>
> >> - if pas not in ([p1], [p2]): # nonlinear
> >> + if (updatecheck == 'linear' and
> >> + pas not in ([p1], [p2])): # nonlinear
> >> dirty = wc.dirty(missing=True)
> >> if dirty:
> >> # Branching is a bit strange to ensure we do the
> >> minimal diff -r 59e2d3da607c -r 75e5a492d7f6 tests/test-completion.t
> >> --- a/tests/test-completion.t Mon Feb 13 15:04:46 2017 -0800
> >> +++ b/tests/test-completion.t Mon Feb 13 12:58:37 2017 -0800
> >> @@ -223,7 +223,7 @@
> >> serve: accesslog, daemon, daemon-postexec, errorlog, port,
> >> address, prefix, name, web-conf, webdir-conf, pid-file, stdio,
> >> cmdserver, templates, style, ipv6, certificate
> >> status: all, modified, added, removed, deleted, clean, unknown,
> >> ignored, no-status, copies, print0, rev, change, include, exclude,
> subrepos, template
> >> summary: remote
> >> - update: clean, check, date, rev, tool
> >> + update: clean, check, merge, date, rev, tool
> >> addremove: similarity, subrepos, include, exclude, dry-run
> >> archive: no-decode, prefix, rev, type, subrepos, include, exclude
> >> backout: merge, commit, no-commit, parent, rev, edit, tool,
> >> include, exclude, message, logfile, date, user diff -r 59e2d3da607c
> >> -r 75e5a492d7f6 tests/test-update-branches.t
> >> --- a/tests/test-update-branches.t Mon Feb 13 15:04:46 2017 -0800
> >> +++ b/tests/test-update-branches.t Mon Feb 13 12:58:37 2017 -0800
> >> @@ -160,6 +160,16 @@
> >> parent=1
> >> M foo
> >>
> >> + $ revtest '-m dirty linear' dirty 1 2 -m
> >> + 1 files updated, 0 files merged, 0 files removed, 0 files
> >> + unresolved
> >> + parent=2
> >> + M foo
> >> +
> >> + $ revtest '-m dirty cross' dirty 3 4 -m
> >> + 1 files updated, 0 files merged, 0 files removed, 0 files
> >> + unresolved
> >> + parent=4
> >> + M foo
> >> +
> >> $ revtest '-c dirtysub linear' dirtysub 1 2 -c
> >> abort: uncommitted changes in subrepository 'sub'
> >> parent=1
> >> @@ -171,7 +181,17 @@
> >> parent=2
> >>
> >> $ revtest '-cC dirty linear' dirty 1 2 -cC
> >> - abort: cannot specify both -c/--check and -C/--clean
> >> + abort: can only specify one of -c/--check, -C/--clean, and
> >> + -m/merge
> >> + parent=1
> >> + M foo
> >> +
> >> + $ revtest '-mc dirty linear' dirty 1 2 -mc
> >> + abort: can only specify one of -c/--check, -C/--clean, and
> >> + -m/merge
> >> + parent=1
> >> + M foo
> >> +
> >> + $ revtest '-mC dirty linear' dirty 1 2 -mC
> >> + abort: can only specify one of -c/--check, -C/--clean, and
> >> + -m/merge
> >> parent=1
> >> M foo
> >>
> >> _______________________________________________
> >> Mercurial-devel mailing list
> >> Mercurial-devel at mercurial-scm.org
> >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list