This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9067

merge: replace calls to hg.updaterepo() by merge.update()
ClosedPublic

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

Details

Reviewers
durin42
pulkit
Group Reviewers
hg-reviewers
Commits
rHG5c8230ca37f2: merge: replace calls to hg.updaterepo() by merge.update()
Summary

The former no longer buys us anything.

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 22808.Sep 23 2020, 1:28 PM
pulkit added a subscriber: pulkit.Sep 24 2020, 3:08 AM

While I agree that resulting API is better than the previous one, however I think:

  • This is an API breakage we can prevent
  • merge.update() used to do a different thing before and now does something else before

Although, we don't provide any API's compatibility but preventing such breakages when we can sound a good thing to me. I think we can leave hg.updaterepo() as it is.

martinvonz added a comment.Sep 24 2020, 3:13 AM
In D9067#136182, @pulkit wrote:

While I agree that resulting API is better than the previous one, however I think:

  • This is an API breakage we can prevent
  • merge.update() used to do a different thing before and now does something else before

Want me to add a new merge.plain_update() or something like that instead?

Although, we don't provide any API's compatibility but preventing such breakages when we can sound a good thing to me. I think we can leave hg.updaterepo() as it is.

Sure, I can do that. So just leave it without callers you mean? Or drop this patch completely?

pulkit added a comment.Sep 24 2020, 3:36 AM
In D9067#136183, @martinvonz wrote:
In D9067#136182, @pulkit wrote:

While I agree that resulting API is better than the previous one, however I think:

  • This is an API breakage we can prevent
  • merge.update() used to do a different thing before and now does something else before

Want me to add a new merge.plain_update() or something like that instead?

Although, we don't provide any API's compatibility but preventing such breakages when we can sound a good thing to me. I think we can leave hg.updaterepo() as it is.

Sure, I can do that. So just leave it without callers you mean? Or drop this patch completely?

Let's leave hg.updaterepo() for now. If we want to remove, we can add a deprecation warning for few/one releases.

martinvonz added a comment.Sep 24 2020, 10:03 AM
In D9067#136215, @pulkit wrote:
In D9067#136183, @martinvonz wrote:
In D9067#136182, @pulkit wrote:

While I agree that resulting API is better than the previous one, however I think:

  • This is an API breakage we can prevent
  • merge.update() used to do a different thing before and now does something else before

Want me to add a new merge.plain_update() or something like that instead?

How about this question? Do you want me to call the new function merge.plain_update() instead? Another option is to do that while leaving merge.update() as a proxy to merge._update() with a call to ui.deprecwarn(). I'm fine with any of these 3 approaches (I'd prefer what I currently have -- it should be pretty simple for callers to check if _update() exists and prefer that for compatibility until they've finished a migration).

Although, we don't provide any API's compatibility but preventing such breakages when we can sound a good thing to me. I think we can leave hg.updaterepo() as it is.

Sure, I can do that. So just leave it without callers you mean? Or drop this patch completely?

Let's leave hg.updaterepo() for now. If we want to remove, we can add a deprecation warning for few/one releases.

Sure, will do. I'll assume you mean to leave it without callers (i.e. to not drop this entire patch).

martinvonz retitled this revision from merge: delete hg.updaterepo() and use merge.update() instead (API) to merge: replace calls to hg.updaterepo() by merge.update().Sep 24 2020, 11:23 AM
martinvonz edited the summary of this revision. (Show Details)
martinvonz updated this revision to Diff 22834.
pulkit added a comment.Sep 25 2020, 3:24 AM
In D9067#136225, @martinvonz wrote:
In D9067#136215, @pulkit wrote:
In D9067#136183, @martinvonz wrote:
In D9067#136182, @pulkit wrote:

While I agree that resulting API is better than the previous one, however I think:

  • This is an API breakage we can prevent
  • merge.update() used to do a different thing before and now does something else before

Want me to add a new merge.plain_update() or something like that instead?

How about this question? Do you want me to call the new function merge.plain_update() instead? Another option is to do that while leaving merge.update() as a proxy to merge._update() with a call to ui.deprecwarn(). I'm fine with any of these 3 approaches (I'd prefer what I currently have -- it should be pretty simple for callers to check if _update() exists and prefer that for compatibility until they've finished a migration).

I think a deprecation warning will be nice. However, I checked that (in all cases?) the old callers will end up with an error when trying to call a new function the old way, so we can document the replacement in the new merge.update() and that should be okay too I guess.

Although, we don't provide any API's compatibility but preventing such breakages when we can sound a good thing to me. I think we can leave hg.updaterepo() as it is.

Sure, I can do that. So just leave it without callers you mean? Or drop this patch completely?

Let's leave hg.updaterepo() for now. If we want to remove, we can add a deprecation warning for few/one releases.

Sure, will do. I'll assume you mean to leave it without callers (i.e. to not drop this entire patch).

Yes, function with no callers but some deprecation warning.

pulkit accepted this revision.Sep 26 2020, 6:27 AM

Do you plan to add a deprecation warning for updaterepo?

This revision is now accepted and ready to land.Sep 26 2020, 6:27 AM
martinvonz added a comment.Sep 28 2020, 10:58 AM
In D9067#136363, @pulkit wrote:

Do you plan to add a deprecation warning for updaterepo?

Sorry, I just forgot about this review. I will send a patch.

Revision Contents
Changeset List

PathPackages
Mhgext/histedit.py (7 lines)
Mhgext/rebase.py (3 lines)
Mhgext/sparse.py (10 lines)
Mmercurial/hg.py (2 lines)
Mmercurial/shelve.py (4 lines)
Mmercurial/subrepo.py (2 lines)

Diff 22859

hgext/histedit.py

change. change.
This exists mainly so that 'rollup' rules can be a subclass of This exists mainly so that 'rollup' rules can be a subclass of
'fold'. 'fold'.
""" """
return False return False
def finishfold(self, ui, repo, ctx, oldctx, newnode, internalchanges): def finishfold(self, ui, repo, ctx, oldctx, newnode, internalchanges):
parent = ctx.p1().node() mergemod.update(ctx.p1())
hg.updaterepo(repo, parent, overwrite=False)
### prepare new commit data ### prepare new commit data
commitopts = {} commitopts = {}
commitopts[b'user'] = ctx.user() commitopts[b'user'] = ctx.user()
# commit message # commit message
if not self.mergedescs(): if not self.mergedescs():
newmessage = ctx.description() newmessage = ctx.description()
else: else:
newmessage = ( newmessage = (
repo, repo,
ctx, ctx,
repo[newnode], repo[newnode],
commitopts, commitopts,
skipprompt=self.skipprompt(), skipprompt=self.skipprompt(),
) )
if n is None: if n is None:
return ctx, [] return ctx, []
hg.updaterepo(repo, n, overwrite=False) mergemod.update(repo[n])
replacements = [ replacements = [
(oldctx.node(), (newnode,)), (oldctx.node(), (newnode,)),
(ctx.node(), (n,)), (ctx.node(), (n,)),
(newnode, (n,)), (newnode, (n,)),
] ]
for ich in internalchanges: for ich in internalchanges:
replacements.append((ich, (n,))) replacements.append((ich, (n,)))
return repo[n], replacements return repo[n], replacements
state.replacements.extend(replacement_) state.replacements.extend(replacement_)
state.actions.pop(0) state.actions.pop(0)
state.write() state.write()
def _finishhistedit(ui, repo, state, fm): def _finishhistedit(ui, repo, state, fm):
"""This action runs when histedit is finishing its session""" """This action runs when histedit is finishing its session"""
hg.updaterepo(repo, state.parentctxnode, overwrite=False) mergemod.update(repo[state.parentctxnode])
mapping, tmpnodes, created, ntm = processreplacement(state) mapping, tmpnodes, created, ntm = processreplacement(state)
if mapping: if mapping:
for prec, succs in pycompat.iteritems(mapping): for prec, succs in pycompat.iteritems(mapping):
if not succs: if not succs:
ui.debug(b'histedit: %s is dropped\n' % node.short(prec)) ui.debug(b'histedit: %s is dropped\n' % node.short(prec))
else: else:
ui.debug( ui.debug(

hgext/rebase.py

bookmarks, bookmarks,
cmdutil, cmdutil,
commands, commands,
copies, copies,
destutil, destutil,
dirstateguard, dirstateguard,
error, error,
extensions, extensions,
hg,
merge as mergemod, merge as mergemod,
mergestate as mergestatemod, mergestate as mergestatemod,
mergeutil, mergeutil,
node as nodemod, node as nodemod,
obsolete, obsolete,
obsutil, obsutil,
patch, patch,
phases, phases,
# restore original working directory # restore original working directory
# (we do this before stripping) # (we do this before stripping)
newwd = self.state.get(self.originalwd, self.originalwd) newwd = self.state.get(self.originalwd, self.originalwd)
if newwd < 0: if newwd < 0:
# original directory is a parent of rebase set root or ignored # original directory is a parent of rebase set root or ignored
newwd = self.originalwd newwd = self.originalwd
if newwd not in [c.rev() for c in repo[None].parents()]: if newwd not in [c.rev() for c in repo[None].parents()]:
ui.note(_(b"update back to initial working directory parent\n")) ui.note(_(b"update back to initial working directory parent\n"))
hg.updaterepo(repo, newwd, overwrite=False) mergemod.update(repo[newwd])
collapsedas = None collapsedas = None
if self.collapsef and not self.keepf: if self.collapsef and not self.keepf:
collapsedas = newnode collapsedas = newnode
clearrebased( clearrebased(
ui, ui,
repo, repo,
self.destmap, self.destmap,

hgext/sparse.py

from mercurial.i18n import _ from mercurial.i18n import _
from mercurial.pycompat import setattr from mercurial.pycompat import setattr
from mercurial import ( from mercurial import (
commands, commands,
dirstate, dirstate,
error, error,
extensions, extensions,
hg,
logcmdutil, logcmdutil,
match as matchmod, match as matchmod,
merge as mergemod,
pycompat, pycompat,
registrar, registrar,
sparse, sparse,
util, util,
) )
# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for
# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should # extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
pat = enableprofile_pat pat = enableprofile_pat
enableprofile = True enableprofile = True
if sum([include, exclude, enableprofile]) > 1: if sum([include, exclude, enableprofile]) > 1:
raise error.Abort(_(b"too many flags specified.")) raise error.Abort(_(b"too many flags specified."))
# if --narrow is passed, it means they are includes and excludes for narrow # if --narrow is passed, it means they are includes and excludes for narrow
# clone # clone
if not narrow_pat and (include or exclude or enableprofile): if not narrow_pat and (include or exclude or enableprofile):
def clonesparse(orig, self, node, overwrite, *args, **kwargs): def clonesparse(orig, ctx, *args, **kwargs):
sparse.updateconfig( sparse.updateconfig(
self.unfiltered(), ctx.repo().unfiltered(),
pat, pat,
{}, {},
include=include, include=include,
exclude=exclude, exclude=exclude,
enableprofile=enableprofile, enableprofile=enableprofile,
usereporootpaths=True, usereporootpaths=True,
) )
return orig(self, node, overwrite, *args, **kwargs) return orig(ctx, *args, **kwargs)
extensions.wrapfunction(hg, b'updaterepo', clonesparse) extensions.wrapfunction(mergemod, b'update', clonesparse)
return orig(ui, repo, *args, **opts) return orig(ui, repo, *args, **opts)
def _setupclone(ui): def _setupclone(ui):
entry = commands.table[b'clone'] entry = commands.table[b'clone']
entry[1].append((b'', b'enable-profile', [], b'enable a sparse profile')) entry[1].append((b'', b'enable-profile', [], b'enable a sparse profile'))
entry[1].append((b'', b'include', [], b'include sparse pattern')) entry[1].append((b'', b'include', [], b'include sparse pattern'))
entry[1].append((b'', b'exclude', [], b'exclude sparse pattern')) entry[1].append((b'', b'exclude', [], b'exclude sparse pattern'))

mercurial/hg.py

force=overwrite, force=overwrite,
labels=[b'working copy', b'destination'], labels=[b'working copy', b'destination'],
updatecheck=updatecheck, updatecheck=updatecheck,
) )
def update(repo, node, quietempty=False, updatecheck=None): def update(repo, node, quietempty=False, updatecheck=None):
"""update the working directory to node""" """update the working directory to node"""
stats = updaterepo(repo, node, False, updatecheck=updatecheck) stats = mergemod.update(repo[node], updatecheck=updatecheck)
_showstats(repo, stats, quietempty) _showstats(repo, stats, quietempty)
if stats.unresolvedcount: if stats.unresolvedcount:
repo.ui.status(_(b"use 'hg resolve' to retry unresolved file merges\n")) repo.ui.status(_(b"use 'hg resolve' to retry unresolved file merges\n"))
return stats.unresolvedcount > 0 return stats.unresolvedcount > 0
# naming conflict in clone() # naming conflict in clone()
_update = update _update = update

mercurial/shelve.py

b'in the working copy\n' b'in the working copy\n'
) )
ui.status(msg) ui.status(msg)
else: else:
# only strip the shelvectx if we produced one # only strip the shelvectx if we produced one
state.nodestoremove.append(newnode) state.nodestoremove.append(newnode)
shelvectx = repo[newnode] shelvectx = repo[newnode]
hg.updaterepo(repo, pendingctx.node(), overwrite=False) merge.update(pendingctx)
mergefiles(ui, repo, state.wctx, shelvectx) mergefiles(ui, repo, state.wctx, shelvectx)
restorebranch(ui, repo, state.branchtorestore) restorebranch(ui, repo, state.branchtorestore)
if not phases.supportinternal(repo): if not phases.supportinternal(repo):
repair.strip( repair.strip(
ui, repo, state.nodestoremove, backup=False, topic=b'shelve' ui, repo, state.nodestoremove, backup=False, topic=b'shelve'
) )
shelvedstate.clear(repo) shelvedstate.clear(repo)
shelvectx = tmpwctx shelvectx = tmpwctx
msg = _( msg = _(
b'note: unshelved changes already existed ' b'note: unshelved changes already existed '
b'in the working copy\n' b'in the working copy\n'
) )
ui.status(msg) ui.status(msg)
else: else:
shelvectx = repo[newnode] shelvectx = repo[newnode]
hg.updaterepo(repo, tmpwctx.node(), False) merge.update(tmpwctx)
return shelvectx, ispartialunshelve return shelvectx, ispartialunshelve
def _forgetunknownfiles(repo, shelvectx, addedbefore): def _forgetunknownfiles(repo, shelvectx, addedbefore):
# Forget any files that were unknown before the shelve, unknown before # Forget any files that were unknown before the shelve, unknown before
# unshelve started, but are now added. # unshelve started, but are now added.
shelveunknown = shelvectx.extra().get(b'shelve_unknown') shelveunknown = shelvectx.extra().get(b'shelve_unknown')

mercurial/subrepo.py

urepo.ui.warn( urepo.ui.warn(
_(b'revision %s in subrepository "%s" is hidden\n') _(b'revision %s in subrepository "%s" is hidden\n')
% (revision[0:12], self._path) % (revision[0:12], self._path)
) )
repo = urepo repo = urepo
if overwrite: if overwrite:
merge.clean_update(repo[revision]) merge.clean_update(repo[revision])
else: else:
hg.updaterepo(repo, revision, False) merge.update(repo[revision])
@annotatesubrepoerror @annotatesubrepoerror
def merge(self, state): def merge(self, state):
self._get(state) self._get(state)
cur = self._repo[b'.'] cur = self._repo[b'.']
dst = self._repo[state[1]] dst = self._repo[state[1]]
anc = dst.ancestor(cur) anc = dst.ancestor(cur)