[PATCH 2 of 2] scmutil: add a cleanupnodes method for developers
Martin von Zweigbergk
martinvonz at google.com
Wed Jun 28 13:18:22 EDT 2017
On Sun, Jun 25, 2017 at 1:32 PM, Jun Wu <quark at fb.com> wrote:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1498422716 25200
> # Sun Jun 25 13:31:56 2017 -0700
> # Node ID c96acc70ab665ea76c4154b793c4071c4c57da1e
> # Parent b70ebbb5b3ffb417d8fe13548b39c9196fadda1f
> # Available At https://bitbucket.org/quark-zju/hg-draft
> # hg pull https://bitbucket.org/quark-zju/hg-draft -r c96acc70ab66
> scmutil: add a cleanupnodes method for developers
Thanks! I was thinking of something like this when I was looking at
your "hg split" patch.
I've already accepted the patch so it's in public phase, but I stil
have a few questions below.
> @@ -562,4 +565,66 @@ def origpath(ui, repo, filepath):
> return fullorigpath + ".orig"
>
> +def cleanupnodes(repo, mapping, operation):
> + """do common cleanups when old nodes are replaced by new nodes
> +
> + That includes writing obsmarkers or stripping nodes, and moving bookmarks.
> + (we might also want to move working directory parent in the future)
> +
> + mapping is {oldnode: [newnode]} or a iterable of nodes if they do not have
> + replacements. operation is a string, like "rebase".
> + """
> + if not util.safehasattr(mapping, 'items'):
> + mapping = {n: () for n in mapping}
> +
> + with repo.transaction('cleanup') as tr:
I wonder if we should pass in the transaction instead to force the
caller to think about the transaction scope, even if they all just
start a new transaction for the cleanup.
> + # Move bookmarks
> + bmarks = repo._bookmarks
> + bmarkchanged = False
> + for oldnode, newnodes in mapping.items():
> + oldbmarks = repo.nodebookmarks(oldnode)
> + if not oldbmarks:
> + continue
> + bmarkchanged = True
> + if len(newnodes) > 1:
> + heads = list(repo.set('heads(%ln)', newnodes))
> + if len(heads) != 1:
> + raise error.ProgrammingError(
> + 'cannot figure out bookmark movement')
> + newnode = heads[0].node()
> + elif len(newnodes) == 0:
> + # move bookmark backwards
> + roots = list(repo.set('max((::%n) - %ln)', oldnode,
> + list(mapping)))
If we're pruning a merge, this puts the bookmark on an "arbitrary"
parent. I see that that's how it already works and I suppose you just
preserved that behavior. Should we simply do the same for the
divergence case above? The ProgrammingError is a little unfortunate,
because the only way to prevent it is by not calling this method,
AFAICT (sure, you can handle that specific bookmark outside, but then
you're not saving that much). So maybe just use max() for the
divergence too?
> + if roots:
> + newnode = roots[0].node()
> + else:
> + newnode = nullid
> + else:
> + newnode = newnodes[0]
> + repo.ui.debug('moving bookmarks %r from %s to %s\n' %
> + (oldbmarks, hex(oldnode), hex(newnode)))
> + for name in oldbmarks:
> + bmarks[name] = newnode
> + if bmarkchanged:
> + bmarks.recordchange(tr)
> +
> + # Obsolete or strip nodes
> + if obsolete.isenabled(repo, obsolete.createmarkersopt):
> + # If a node is already obsoleted, and we want to obsolete it
> + # without a successor, skip that obssolete request since it's
> + # unnecessary. That's the "if s or not isobs(n)" check below.
> + # Also sort the node in topology order, that might be useful for
> + # some obsstore logic.
> + # NOTE: the filtering and sorting might belong to createmarkers.
> + isobs = repo.obsstore.successors.__contains__
> + sortfunc = lambda ns: repo.changelog.rev(ns[0])
> + rels = [(repo[n], (repo[m] for m in s))
> + for n, s in sorted(mapping.items(), key=sortfunc)
> + if s or not isobs(n)]
Nit: We could probably make that code a bit clearer so the comment
above about "if s or not isobs(n)" is not needed. Maybe something like
this:
rels = []
for node, successors in sorted(mapping.items(), key=sortfunc):
ctx = unfi[node]
if not successors and ctx.obsolete():
continue # already done
rels.append((ctx, (unfi[s] for s in successors)))
> + obsolete.createmarkers(repo, rels, operation=operation)
> + else:
> + from . import repair # avoid import cycle
> + repair.delayedstrip(repo.ui, repo, list(mapping), operation)
> +
> def addremove(repo, matcher, prefix, opts=None, dry_run=None, similarity=None):
> if opts is None:
> diff --git a/tests/test-strip.t b/tests/test-strip.t
> --- a/tests/test-strip.t
> +++ b/tests/test-strip.t
> @@ -956,4 +956,5 @@ Use delayedstrip to strip inside a trans
> > A Z
> > EOS
> + $ cp -R . ../scmutilcleanup
>
> $ hg up -C I
> @@ -991,2 +992,103 @@ Use delayedstrip to strip inside a trans
> o 0:426bada5c675 A
>
> +Test high-level scmutil.cleanupnodes API
> +
> + $ cd $TESTTMP/scmutilcleanup
> + $ hg debugdrawdag <<'EOS'
> + > D2 F2 G2 # D2, F2, G2 are replacements for D, F, G
> + > | | |
> + > C H G
> + > EOS
> + $ for i in B C D F G I Z; do
> + > hg bookmark -i -r $i b-$i
> + > done
> + $ cp -R . ../scmutilcleanup.obsstore
> +
> + $ cat > $TESTTMP/scmutilcleanup.py <<EOF
> + > from mercurial import scmutil
> + > def reposetup(ui, repo):
> + > def nodes(expr):
> + > return [repo.changelog.node(r) for r in repo.revs(expr)]
> + > def node(expr):
> + > return nodes(expr)[0]
> + > with repo.wlock():
> + > with repo.lock():
> + > with repo.transaction('delayedstrip'):
Same comment as on another patch: we're on Python 2.7, so can combine
these with-blocks.
> + > mapping = {node('F'): [node('F2')],
> + > node('D'): [node('D2')],
> + > node('G'): [node('G2')]}
> + > scmutil.cleanupnodes(repo, mapping, 'replace')
> + > scmutil.cleanupnodes(repo, nodes('((B::)+I+Z)-D2'), 'replace')
> + > EOF
> + $ hg log -r . -T '\n' --config extensions.t=$TESTTMP/scmutilcleanup.py
> + warning: orphaned descendants detected, not stripping 112478962961, 1fc8102cda62, 26805aba1e60
> + saved backup bundle to $TESTTMP/scmutilcleanup/.hg/strip-backup/f585351a92f8-73fb7c03-replace.hg (glob)
> +
> + $ hg log -G -T '{rev}:{node|short} {desc} {bookmarks}' -r 'sort(all(), topo)'
> + o 8:1473d4b996d1 G2 b-G
> + |
> + | o 7:d94e89b773b6 F2 b-F
> + | |
> + | o 5:7fe5bac4c918 H
> + |/|
> + | o 3:7fb047a69f22 E
> + | |
> + | | o 6:7c78f703e465 D2 b-D
> + | | |
> + | | o 4:26805aba1e60 C
> + | | |
> + | | o 2:112478962961 B
> + | |/
> + o | 1:1fc8102cda62 G
> + /
> + o 0:426bada5c675 A b-B b-C b-I
> +
> + $ hg bookmark
> + b-B 0:426bada5c675
> + b-C 0:426bada5c675
> + b-D 6:7c78f703e465
> + b-F 7:d94e89b773b6
> + b-G 8:1473d4b996d1
> + b-I 0:426bada5c675
> + b-Z -1:000000000000
> +
> +Test the above using obsstore "by the way". Not directly related to strip, but
> +we have reusable code here
> +
> + $ cd $TESTTMP/scmutilcleanup.obsstore
> + $ cat >> .hg/hgrc <<EOF
> + > [experimental]
> + > evolution=all
> + > evolution.track-operation=1
> + > EOF
> +
> + $ hg log -r . -T '\n' --config extensions.t=$TESTTMP/scmutilcleanup.py
> +
> + $ rm .hg/localtags
> + $ hg log -G -T '{rev}:{node|short} {desc} {bookmarks}' -r 'sort(all(), topo)'
> + o 12:1473d4b996d1 G2 b-G
> + |
> + | o 11:d94e89b773b6 F2 b-F
> + | |
> + | o 8:7fe5bac4c918 H
> + |/|
> + | o 4:7fb047a69f22 E
> + | |
> + | | o 10:7c78f703e465 D2 b-D
> + | | |
> + | | x 6:26805aba1e60 C
> + | | |
> + | | x 3:112478962961 B
> + | |/
> + x | 1:1fc8102cda62 G
> + /
> + o 0:426bada5c675 A b-B b-C b-I
> +
> + $ hg debugobsolete
> + 1fc8102cda6204549f031015641606ccf5513ec3 1473d4b996d1d1b121de6b39fab6a04fbf9d873e 0 (Thu Jan 01 00:00:00 1970 +0000) {'operation': 'replace', 'user': 'test'}
> + 64a8289d249234b9886244d379f15e6b650b28e3 d94e89b773b67e72642a931159ada8d1a9246998 0 (Thu Jan 01 00:00:00 1970 +0000) {'operation': 'replace', 'user': 'test'}
> + f585351a92f85104bff7c284233c338b10eb1df7 7c78f703e465d73102cc8780667ce269c5208a40 0 (Thu Jan 01 00:00:00 1970 +0000) {'operation': 'replace', 'user': 'test'}
> + 48b9aae0607f43ff110d84e6883c151942add5ab 0 {0000000000000000000000000000000000000000} (Thu Jan 01 00:00:00 1970 +0000) {'operation': 'replace', 'user': 'test'}
> + 112478962961147124edd43549aedd1a335e44bf 0 {426bada5c67598ca65036d57d9e4b64b0c1ce7a0} (Thu Jan 01 00:00:00 1970 +0000) {'operation': 'replace', 'user': 'test'}
> + 08ebfeb61bac6e3f12079de774d285a0d6689eba 0 {426bada5c67598ca65036d57d9e4b64b0c1ce7a0} (Thu Jan 01 00:00:00 1970 +0000) {'operation': 'replace', 'user': 'test'}
> + 26805aba1e600a82e93661149f2313866a221a7b 0 {112478962961147124edd43549aedd1a335e44bf} (Thu Jan 01 00:00:00 1970 +0000) {'operation': 'replace', 'user': 'test'}
> _______________________________________________
> 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