This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9066

merge: add a higher-level update() for the common `hg update` use case
ClosedPublic

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

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGc1b603cdc95a: merge: add a higher-level update() for the common `hg update` use case
Summary

This is different from the update() function that I just made
private. The new function is specifically for the normal hg update
use case. It doesn't do a merge and it doesn't do a clean (forced)
update.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Sep 21 2020, 4:36 PM
martinvonz updated this revision to Diff 22768.Sep 23 2020, 3:12 AM
martinvonz updated this revision to Diff 22807.Sep 23 2020, 1:28 PM
pulkit added a subscriber: pulkit.Sep 25 2020, 3:18 AM
pulkit added inline comments.
mercurial/merge.py
2137

Let's add couple of lines of documentation here that if you are looking for old merge.update(), merge._update() is the thing you are looking for.

martinvonz marked an inline comment as done.Sep 25 2020, 10:51 AM
martinvonz updated this revision to Diff 22853.
martinvonz added inline comments.Sep 25 2020, 10:51 AM
mercurial/merge.py
2137

Makes sense. Done.

pulkit accepted this revision.Sep 26 2020, 6:25 AM
This revision is now accepted and ready to land.Sep 26 2020, 6:25 AM
marmoute added subscribers: mercurial-devel, marmoute.Oct 2 2020, 2:34 PM

[Moving a discussion we are having on #hg-evolve here to reach more people]

The clarification of interface seems great overall. However I am a bit
worried about the drop of the initial repo argument to these function.

Passing the repository explicitly seems safer to me. It make sure we are
updating the right repository, and more importantly with the right
repoview (filtering level). It would be easy for some fault code to
reuse output from a function using relaxed filtering to use in a more
filtered context. explicitly passing the repository catch that.

What do you think ?

martinvonz added a comment.Oct 2 2020, 3:41 PM
In D9066#137315, @marmoute wrote:

[Moving a discussion we are having on #hg-evolve here to reach more people]
The clarification of interface seems great overall. However I am a bit
worried about the drop of the initial repo argument to these function.
Passing the repository explicitly seems safer to me. It make sure we are
updating the right repository, and more importantly with the right
repoview (filtering level). It would be easy for some fault code to
reuse output from a function using relaxed filtering to use in a more
filtered context. explicitly passing the repository catch that.
What do you think ?

I can see the concern at some abstract level, but I don't know what you mean more concretely. What would such bad callers look like? Also, does your concern apply to most places we pass around context objects?

Revision Contents
Changeset List

PathPackages
Mhgext/transplant.py (6 lines)
Mmercurial/merge.py (17 lines)

Diff 22755

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]])
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
# couldn't transplant after the latest even if # couldn't transplant after the latest even if
# transplants before them fail. # transplants before them fail.
% (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]])
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/merge.py

labels=labels, labels=labels,
branchmerge=True, branchmerge=True,
force=force, force=force,
mergeforce=force, mergeforce=force,
wc=wc, wc=wc,
) )
def update(ctx, updatecheck=None, wc=None):
"""Do a regular update to the given commit, aborting if there are conflicts.
pulkitUnsubmitted
Done

Let's add couple of lines of documentation here that if you are looking for old merge.update(), merge._update() is the thing you are looking for.

pulkit: Let's add couple of lines of documentation here that if you are looking for old `merge.update…
martinvonzAuthorUnsubmitted
Done

Makes sense. Done.

martinvonz: Makes sense. Done.
The 'updatecheck' argument can be used to control what to do in case of
conflicts.
"""
return _update(
ctx.repo(),
ctx.rev(),
branchmerge=False,
force=False,
labels=[b'working copy', b'destination'],
updatecheck=updatecheck,
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)