This is an archive of the discontinued Mercurial Phabricator instance.

Differential D157

hiddenoverride: be more conservative about pinning nodes
ClosedPublic

Authored by quark on Jul 20 2017, 5:55 PM.
Tags
None
Subscribers
stash

Details

Reviewers
durham
stash
Group Reviewers
Restricted Project
Commits
rFBHGX5fb55777b826: hiddenoverride: be more conservative about pinning nodes
Summary

I think the code works well but it might leave unnecessary pinned nodes
around. This patch makes it more conservative so that only a movement
(update or bookmark) on an obsolete commit would write new pinned nodes.

Diff Detail

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

Event Timeline

quark created this revision.Jul 20 2017, 5:55 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 20 2017, 5:55 PM
quark added a comment.EditedJul 21 2017, 1:21 AM

I'm pushing this since it seems helpful to solve some weird cases that people have reported. That might prevent another rollback on Friday's release.

stash accepted this revision.Jul 21 2017, 3:46 AM
stash added a subscriber: stash.
stash added inline comments.
hgext3rd/fbamend/hiddenoverride.py
115

Smth is wrong with a comment

This revision is now accepted and ready to land.Jul 21 2017, 3:46 AM
Closed by commit rFBHGX5fb55777b826: hiddenoverride: be more conservative about pinning nodes (authored by quark). · Explain WhyJul 21 2017, 11:10 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 370

hgext3rd/fbamend/hiddenoverride.py

nodemap = repo.changelog.nodemap nodemap = repo.changelog.nodemap
pinnednodes = set(loadpinnednodes(repo)) pinnednodes = set(loadpinnednodes(repo))
tounpin = getattr(repo, '_tounpinnodes', set()) tounpin = getattr(repo, '_tounpinnodes', set())
pinnednodes -= tounpin pinnednodes -= tounpin
revs.update(nodemap[n] for n in pinnednodes) revs.update(nodemap[n] for n in pinnednodes)
return revs return revs
def loadpinnednodes(repo): def loadpinnednodes(repo):
"""yield pinned nodes that should be visible""" """yield pinned nodes that are obsoleted and should be visible"""
if repo is None or not repo.local(): if repo is None or not repo.local():
return return
# the "pinned nodes" file name is "obsinhibit" for compatibility reason # the "pinned nodes" file name is "obsinhibit" for compatibility reason
content = repo.svfs.tryread('obsinhibit') or '' content = repo.svfs.tryread('obsinhibit') or ''
nodemap = repo.unfiltered().changelog.nodemap unfi = repo.unfiltered()
nodemap = unfi.changelog.nodemap
offset = 0 offset = 0
while True: while True:
node = content[offset:offset + 20] node = content[offset:offset + 20]
if not node: if not node:
break break
if node in nodemap: # remove unnecessary (non-obsoleted) nodes since pinnedrevs should only
# affect obsoleted revs.
if node in nodemap and unfi[node].obsolete():
yield node yield node
offset += 20 offset += 20
def shouldpinnodes(repo): def shouldpinnodes(repo):
"""get nodes that should be pinned: working parent + bookmarks for now""" """get nodes that should be pinned: working parent + bookmarks for now"""
result = set() result = set()
if repo and repo.local(): if repo and repo.local():
# working copy parent # working copy parent
with flock(repo.svfs.join('obsinhibit.lock')): with flock(repo.svfs.join('obsinhibit.lock')):
nodes = set(loadpinnednodes(repo)) nodes = set(loadpinnednodes(repo))
nodes |= set(newpin) nodes |= set(newpin)
nodes -= set(newunpin) nodes -= set(newunpin)
with util.atomictempfile(repo.svfs.join('obsinhibit')) as f: with util.atomictempfile(repo.svfs.join('obsinhibit')) as f:
f.write(''.join(nodes)) f.write(''.join(nodes))
def runcommand(orig, lui, repo, cmd, fullargs, *args): def runcommand(orig, lui, repo, cmd, fullargs, *args):
shouldpinbefore = shouldpinnodes(repo) # return directly for non-repo command
if not repo:
return orig(lui, repo, cmd, fullargs, *args)
shouldpinbefore = shouldpinnodes(repo) | set(loadpinnednodes(repo))
result = orig(lui, repo, cmd, fullargs, *args) result = orig(lui, repo, cmd, fullargs, *args)
# after a command completes, make sure working copy parent and all # after a command completes, make sure working copy parent and all
# bookmarks get "pinned". # bookmarks get "pinned".
newpin = shouldpinnodes(repo) - shouldpinbefore newpin = shouldpinnodes(repo) - shouldpinbefore
newunpin = getattr(repo, '_tounpinnodes', set()) newunpin = getattr(repo.unfiltered(), '_tounpinnodes', set())
# filter newpin by obsolte - ex. if newpin is on a non-obsoleted commit,
stashUnsubmitted
Not Done

Smth is wrong with a comment

stash: Smth is wrong with a comment
# ignore it.
unfi = repo.unfiltered()
newpin = set(n for n in newpin if unfi[n].obsolete())
# only do a write if something has changed # only do a write if something has changed
if newpin or newunpin: if newpin or newunpin:
savepinnednodes(repo, newpin, newunpin) savepinnednodes(repo, newpin, newunpin)
return result return result
def createmarkers(orig, repo, rels, *args, **kwargs): def createmarkers(orig, repo, rels, *args, **kwargs):
# this is a way to unpin revs - precursors are unpinned # this is a way to unpin revs - precursors are unpinned
# note: hg debugobsolete does not call this function
unfi = repo.unfiltered() unfi = repo.unfiltered()
tounpin = getattr(unfi, '_tounpinnodes', set()) tounpin = getattr(unfi, '_tounpinnodes', set())
for r in rels: for r in rels:
try: try:
tounpin.add(r[0].node()) tounpin.add(r[0].node())
except error.RepoLookupError: except error.RepoLookupError:
pass pass
unfi._tounpinnodes = tounpin unfi._tounpinnodes = tounpin
return orig(repo, rels, *args, **kwargs) return orig(repo, rels, *args, **kwargs)

tests/test-fbamend-hiddenoverride.t

$ hg bookmark -d BOOK -q $ hg bookmark -d BOOK -q
$ hg log -G -T '{rev} {desc}\n' $ hg log -G -T '{rev} {desc}\n'
o 2 C o 2 C
| |
| x 1 B | x 1 B
|/ |/
@ 0 A @ 0 A
The order matters - putting bookmarks or moving working copy on non-obsoleted
commits do not pin them. Test this using "debugobsolete" which will not call
"createmarkers".
Obsolete working copy, and move working copy away should make things disappear
$ rm -rf .hg && hg init && hg debugdrawdag <<'EOS'
> C E
> | |
> B D
> |/
> A
> EOS
$ hg up -q E
$ hg debugobsolete `HGPLAIN=1 hg log -r E -T '{node}'`
obsoleted 1 changesets
$ hg tag --local --remove E
$ hg log -G -T '{rev} {desc}\n'
@ 4 E
|
| o 3 C
| |
o | 2 D
| |
| o 1 B
|/
o 0 A
$ hg debugobsolete `HGPLAIN=1 hg log -r D -T '{node}'`
obsoleted 1 changesets
$ hg tag --local --remove D
$ hg log -G -T '{rev} {desc}\n'
@ 4 E
|
| o 3 C
| |
x | 2 D
| |
| o 1 B
|/
o 0 A
$ hg update -q C
$ hg log -G -T '{rev} {desc}\n'
@ 3 C
|
o 1 B
|
o 0 A
Having a bookmark on a commit, obsolete the commit, remove the bookmark
$ rm -rf .hg && hg init && hg debugdrawdag <<'EOS'
> C E
> | |
> B D
> |/
> A
> EOS
$ hg bookmark -i book-e -r E
$ hg debugobsolete `HGPLAIN=1 hg log -r D -T '{node}'`
obsoleted 1 changesets
$ hg debugobsolete `HGPLAIN=1 hg log -r E -T '{node}'`
obsoleted 1 changesets
$ rm .hg/localtags
$ hg log -G -T '{rev} {desc} {bookmarks}\n'
x 4 E book-e
|
| o 3 C
| |
x | 2 D
| |
| o 1 B
|/
o 0 A
$ hg bookmark -d book-e
$ hg log -G -T '{rev} {desc} {bookmarks}\n'
o 3 C
|
o 1 B
|
o 0 A