This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9065

merge: make low-level update() private (API)
ClosedPublic

Authored by martinvonz on Sep 21 2020, 4:36 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG2c86b9587740: merge: make low-level update() private (API)
rHG2748f837e192: merge: make low-level update() private (API)
Summary

We have very few callers left that call the low-level merge.update()
function. I think it's time to make it private. I'll remove the
remaining callers in coming patches, except for one call from the
rebase module. I hope to eventually fix that too, but it's more
complex because it requires teaching merge.graft() to work with a
dirty working copy.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Sep 21 2020, 4:36 PM
martinvonz updated this revision to Diff 22767.Sep 23 2020, 3:12 AM
martinvonz updated this revision to Diff 22806.Sep 23 2020, 1:28 PM
pulkit accepted this revision.Sep 25 2020, 3:21 AM
This revision is now accepted and ready to land.Sep 25 2020, 3:21 AM
martinvonz added a commit: rHG2748f837e192: merge: make low-level update() private (API).Sep 25 2020, 5:19 AM
Closed by commit rHG2748f837e192: merge: make low-level update() private (API) (authored by martinvonz). · Explain Why
This revision was automatically updated to reflect the committed changes.
marmoute added a subscriber: marmoute.Sep 25 2020, 8:01 AM

tests/test-install.t and tests/test-duplicateoptions.py are unhappy about this change.

--- /tmp/mercurial-ci/tests/test-duplicateoptions.py.out
+++ /tmp/mercurial-ci/tests/test-duplicateoptions.py.err
@@ -0,0 +1,9 @@
+Traceback (most recent call last):
+  File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+    extsetup(ui)
+  File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+    extensions.wrapfunction(merge, b'update', wrapupdate)
+  File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+    origfn = getattr(container, funcname)
+AttributeError: 'module' object has no attribute 'update'
+*** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
ERROR: test-duplicateoptions.py output changed

--- /tmp/mercurial-ci/tests/test-install.t
+++ /tmp/mercurial-ci/tests/test-install.t.err
@@ -164,12 +164,28 @@
 
 debuginstall extension support
   $ hg debuginstall --config extensions.fsmonitor= --config fsmonitor.watchman_exe=false | grep atchman
-  fsmonitor checking for watchman binary... (false)
-   watchman binary missing or broken: warning: Watchman unavailable: watchman exited with code 1
+  Traceback (most recent call last):
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+      extsetup(ui)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+      extensions.wrapfunction(merge, b'update', wrapupdate)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+      origfn = getattr(container, funcname)
+  AttributeError: 'module' object has no attribute 'update'
+  *** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
+  [1]
 Verify the json works too:
   $ hg debuginstall --config extensions.fsmonitor= --config fsmonitor.watchman_exe=false -Tjson | grep atchman
-    "fsmonitor-watchman": "false",
-    "fsmonitor-watchman-error": "warning: Watchman unavailable: watchman exited with code 1",
+  Traceback (most recent call last):
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+      extsetup(ui)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+      extensions.wrapfunction(merge, b'update', wrapupdate)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+      origfn = getattr(container, funcname)
+  AttributeError: 'module' object has no attribute 'update'
+  *** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
+  [1]
 
 Verify that Mercurial is installable with pip. Note that this MUST be
 the last test in this file, because we do some nasty things to the
ERROR: test-install.t output changed
pulkit added a comment.Sep 25 2020, 8:35 AM
In D9065#136322, @marmoute wrote:

tests/test-install.t and tests/test-duplicateoptions.py are unhappy about this change.

--- /tmp/mercurial-ci/tests/test-duplicateoptions.py.out
+++ /tmp/mercurial-ci/tests/test-duplicateoptions.py.err
@@ -0,0 +1,9 @@
+Traceback (most recent call last):
+  File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+    extsetup(ui)
+  File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+    extensions.wrapfunction(merge, b'update', wrapupdate)
+  File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+    origfn = getattr(container, funcname)
+AttributeError: 'module' object has no attribute 'update'
+*** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
ERROR: test-duplicateoptions.py output changed
--- /tmp/mercurial-ci/tests/test-install.t
+++ /tmp/mercurial-ci/tests/test-install.t.err
@@ -164,12 +164,28 @@
 debuginstall extension support
   $ hg debuginstall --config extensions.fsmonitor= --config fsmonitor.watchman_exe=false | grep atchman
-  fsmonitor checking for watchman binary... (false)
-   watchman binary missing or broken: warning: Watchman unavailable: watchman exited with code 1
+  Traceback (most recent call last):
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+      extsetup(ui)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+      extensions.wrapfunction(merge, b'update', wrapupdate)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+      origfn = getattr(container, funcname)
+  AttributeError: 'module' object has no attribute 'update'
+  *** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
+  [1]
 Verify the json works too:
   $ hg debuginstall --config extensions.fsmonitor= --config fsmonitor.watchman_exe=false -Tjson | grep atchman
-    "fsmonitor-watchman": "false",
-    "fsmonitor-watchman-error": "warning: Watchman unavailable: watchman exited with code 1",
+  Traceback (most recent call last):
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+      extsetup(ui)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+      extensions.wrapfunction(merge, b'update', wrapupdate)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+      origfn = getattr(container, funcname)
+  AttributeError: 'module' object has no attribute 'update'
+  *** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
+  [1]
 Verify that Mercurial is installable with pip. Note that this MUST be
 the last test in this file, because we do some nasty things to the
ERROR: test-install.t output changed

Since this was the tip, I amended the fix and pushed (with 23k obsmarkers).

martinvonz added a comment.Sep 25 2020, 10:43 AM
In D9065#136324, @pulkit wrote:
In D9065#136322, @marmoute wrote:

tests/test-install.t and tests/test-duplicateoptions.py are unhappy about this change.

Since this was the tip, I amended the fix and pushed (with 23k obsmarkers).

Sorry that I didn't notice that and thanks for fixing!

Revision Contents
Changeset List

PathPackages
Mhgext/largefiles/overrides.py (2 lines)
Mhgext/rebase.py (4 lines)
Mhgext/transplant.py (4 lines)
Mmercurial/hg.py (2 lines)
Mmercurial/merge.py (12 lines)
Mrelnotes/next (4 lines)

Diff 22851

hgext/largefiles/overrides.py

path = lfutil.usercachepath(repo.ui, hash) path = lfutil.usercachepath(repo.ui, hash)
with open(path, b"rb") as fpin: with open(path, b"rb") as fpin:
for chunk in util.filechunkiter(fpin): for chunk in util.filechunkiter(fpin):
fp.write(chunk) fp.write(chunk)
err = 0 err = 0
return err return err
@eh.wrapfunction(merge, b'update') @eh.wrapfunction(merge, b'_update')
def mergeupdate(orig, repo, node, branchmerge, force, *args, **kwargs): def mergeupdate(orig, repo, node, branchmerge, force, *args, **kwargs):
matcher = kwargs.get('matcher', None) matcher = kwargs.get('matcher', None)
# note if this is a partial update # note if this is a partial update
partial = matcher and not matcher.always() partial = matcher and not matcher.always()
with repo.wlock(): with repo.wlock():
# branch | | | # branch | | |
# merge | force | partial | action # merge | force | partial | action
# -------+-------+---------+-------------- # -------+-------+---------+--------------

hgext/rebase.py

_( _(
b"hit merge conflicts; rebasing that " b"hit merge conflicts; rebasing that "
b"commit again in the working copy\n" b"commit again in the working copy\n"
) )
) )
cmdutil.bailifchanged(repo) cmdutil.bailifchanged(repo)
self.inmemory = False self.inmemory = False
self._assignworkingcopy() self._assignworkingcopy()
mergemod.update( mergemod._update(
repo, repo,
p1, p1,
branchmerge=False, branchmerge=False,
force=False, force=False,
wc=self.wctx, wc=self.wctx,
) )
rebasenode( rebasenode(
repo, repo,
repo.dirstate.write(repo.currenttransaction()) repo.dirstate.write(repo.currenttransaction())
ctx = repo[rev] ctx = repo[rev]
repo.ui.debug(b" merge against %d:%s\n" % (rev, ctx)) repo.ui.debug(b" merge against %d:%s\n" % (rev, ctx))
if base is not None: if base is not None:
repo.ui.debug(b" detach base %d:%s\n" % (base, repo[base])) repo.ui.debug(b" detach base %d:%s\n" % (base, repo[base]))
# See explanation in merge.graft() # See explanation in merge.graft()
mergeancestor = repo.changelog.isancestor(p1ctx.node(), ctx.node()) mergeancestor = repo.changelog.isancestor(p1ctx.node(), ctx.node())
stats = mergemod.update( stats = mergemod._update(
repo, repo,
rev, rev,
branchmerge=True, branchmerge=True,
force=True, force=True,
ancestor=base, ancestor=base,
mergeancestor=mergeancestor, mergeancestor=mergeancestor,
labels=[b'dest', b'source'], labels=[b'dest', b'source'],
wc=wctx, wc=wctx,

hgext/transplant.py

# wdir's parent, just pull it. # wdir's parent, just pull it.
if parents[0] == p1: if parents[0] == p1:
pulls.append(node) pulls.append(node)
p1 = node p1 = node
continue continue
if pulls: if pulls:
if source != repo: if source != repo:
exchange.pull(repo, source.peer(), heads=pulls) exchange.pull(repo, source.peer(), heads=pulls)
merge.update( merge._update(
repo, pulls[-1], branchmerge=False, force=False repo, pulls[-1], branchmerge=False, force=False
) )
p1 = repo.dirstate.p1() p1 = repo.dirstate.p1()
pulls = [] pulls = []
domerge = False domerge = False
if node in merges: if node in merges:
# pulling all the merge revs at once would mean we # pulling all the merge revs at once would mean we
% (nodemod.short(node), nodemod.short(n)) % (nodemod.short(node), nodemod.short(n))
) )
finally: finally:
if patchfile: if patchfile:
os.unlink(patchfile) os.unlink(patchfile)
tr.close() tr.close()
if pulls: if pulls:
exchange.pull(repo, source.peer(), heads=pulls) exchange.pull(repo, source.peer(), heads=pulls)
merge.update(repo, pulls[-1], branchmerge=False, force=False) merge._update(repo, pulls[-1], branchmerge=False, force=False)
finally: finally:
self.saveseries(revmap, merges) self.saveseries(revmap, merges)
self.transplants.write() self.transplants.write()
if tr: if tr:
tr.release() tr.release()
if lock: if lock:
lock.release() lock.release()

mercurial/hg.py

def updaterepo(repo, node, overwrite, updatecheck=None): def updaterepo(repo, node, overwrite, updatecheck=None):
"""Update the working directory to node. """Update the working directory to node.
When overwrite is set, changes are clobbered, merged else When overwrite is set, changes are clobbered, merged else
returns stats (see pydoc mercurial.merge.applyupdates)""" returns stats (see pydoc mercurial.merge.applyupdates)"""
return mergemod.update( return mergemod._update(
repo, repo,
node, node,
branchmerge=False, branchmerge=False,
force=overwrite, force=overwrite,
labels=[b'working copy', b'destination'], labels=[b'working copy', b'destination'],
updatecheck=updatecheck, updatecheck=updatecheck,
) )

mercurial/merge.py

UPDATECHECK_ABORT = b'abort' # handled at higher layers UPDATECHECK_ABORT = b'abort' # handled at higher layers
UPDATECHECK_NONE = b'none' UPDATECHECK_NONE = b'none'
UPDATECHECK_LINEAR = b'linear' UPDATECHECK_LINEAR = b'linear'
UPDATECHECK_NO_CONFLICT = b'noconflict' UPDATECHECK_NO_CONFLICT = b'noconflict'
def update( def _update(
repo, repo,
node, node,
branchmerge, branchmerge,
force, force,
ancestor=None, ancestor=None,
mergeancestor=False, mergeancestor=False,
labels=None, labels=None,
matcher=None, matcher=None,
def merge(ctx, labels=None, force=False, wc=None): def merge(ctx, labels=None, force=False, wc=None):
"""Merge another topological branch into the working copy. """Merge another topological branch into the working copy.
force = whether the merge was run with 'merge --force' (deprecated) force = whether the merge was run with 'merge --force' (deprecated)
""" """
return update( return _update(
ctx.repo(), ctx.repo(),
ctx.rev(), ctx.rev(),
labels=labels, labels=labels,
branchmerge=True, branchmerge=True,
force=force, force=force,
mergeforce=force, mergeforce=force,
wc=wc, wc=wc,
) )
def clean_update(ctx, wc=None): def clean_update(ctx, wc=None):
"""Do a clean update to the given commit. """Do a clean update to the given commit.
This involves updating to the commit and discarding any changes in the This involves updating to the commit and discarding any changes in the
working copy. working copy.
""" """
return update(ctx.repo(), ctx.rev(), branchmerge=False, force=True, wc=wc) return _update(ctx.repo(), ctx.rev(), branchmerge=False, force=True, wc=wc)
def revert_to(ctx, matcher=None, wc=None): def revert_to(ctx, matcher=None, wc=None):
"""Revert the working copy to the given commit. """Revert the working copy to the given commit.
The working copy will keep its current parent(s) but its content will The working copy will keep its current parent(s) but its content will
be the same as in the given commit. be the same as in the given commit.
""" """
return update( return _update(
ctx.repo(), ctx.repo(),
ctx.rev(), ctx.rev(),
branchmerge=False, branchmerge=False,
force=True, force=True,
updatedirstate=False, updatedirstate=False,
matcher=matcher, matcher=matcher,
wc=wc, wc=wc,
) )
wctx = wctx or repo[None] wctx = wctx or repo[None]
pctx = wctx.p1() pctx = wctx.p1()
base = base or ctx.p1() base = base or ctx.p1()
mergeancestor = ( mergeancestor = (
repo.changelog.isancestor(pctx.node(), ctx.node()) repo.changelog.isancestor(pctx.node(), ctx.node())
or pctx.rev() == base.rev() or pctx.rev() == base.rev()
) )
stats = update( stats = _update(
repo, repo,
ctx.node(), ctx.node(),
True, True,
True, True,
base.node(), base.node(),
mergeancestor=mergeancestor, mergeancestor=mergeancestor,
labels=labels, labels=labels,
wc=wctx, wc=wctx,
def back_out(ctx, parent=None, wc=None): def back_out(ctx, parent=None, wc=None):
if parent is None: if parent is None:
if ctx.p2() is not None: if ctx.p2() is not None:
raise error.ProgrammingError( raise error.ProgrammingError(
b"must specify parent of merge commit to back out" b"must specify parent of merge commit to back out"
) )
parent = ctx.p1() parent = ctx.p1()
return update( return _update(
ctx.repo(), ctx.repo(),
parent, parent,
branchmerge=True, branchmerge=True,
force=True, force=True,
ancestor=ctx.node(), ancestor=ctx.node(),
mergeancestor=False, mergeancestor=False,
) )

relnotes/next

== Backwards Compatibility Changes == == Backwards Compatibility Changes ==
== Internal API Changes == == Internal API Changes ==
* `merge.update()` is now private (renamed to `_update()`). Hopefully
the higher-level functions available in the same module cover your
use cases.