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
Lint Skipped
Unit
Unit Tests Skipped

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 347

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