This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5398

narrow: keep bookmarks temporarily stripped for as long as commits are
ClosedPublic

Authored by martinvonz on Dec 9 2018, 3:09 AM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG109a267acc1e: narrow: keep bookmarks temporarily stripped for as long as commits are
Summary

The narrow extension also has support for shallowness and for
inserting older commits on pull. It works by temporarily stripping
newer commits, adding the older commits, then re-applying the stripped
bundle. The regular Mercurial server uses that when you widen,
although it shouldn't be necessary there. Our Google-internal server
does it when the user requests an older commit.

Our Google-internal tests fail since 7caf632e30c3 (filecache:
unimplement set() and delete() (API), 2018-10-20). I haven't
quite understood the problem, but it's related to the way we
temporarily hide bookmarks while the commits they point to are
stripped. When a transaction is started, Mercurial tries to read
various things from the repo for the transaction summary. That leads
to computation of hidden commits, which leads to an attempt to find
commits pinned by bookmarks. This is the reason we temporarily hide
the bookmarks. I think the aforementioned commit makes the restored
bookmarks visible earlier than before (which seems like an
improvement), so we can no longer incorrectly rely on the
repo._bookmarks field being cached too long (IIUC).

This patch makes it so we restore the temporarily hidden bookmarks
only after the temporary bundle has been re-applied. It also adapts
the code to update the repo.__bookmarks field using the pattern
described in the aforementioned commit instead of writing directly to
the fiels.

I have spent many hours trying to understand what was going on here,
but I still don't know if this can also happen without our custom
server. So this patch unfortunately does not add any tests; I have
only been able to test the fix using our Google-internal tests.

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.Dec 9 2018, 3:09 AM
Closed by commit rHG109a267acc1e: narrow: keep bookmarks temporarily stripped for as long as commits are (authored by martinvonz). · Explain WhyDec 9 2018, 7:06 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/narrow/narrowbundle2.py (11 lines)

Diff 12763

hgext/narrow/narrowbundle2.py

bin, bin,
nullid, nullid,
) )
from mercurial import ( from mercurial import (
bundle2, bundle2,
changegroup, changegroup,
error, error,
exchange, exchange,
localrepo,
narrowspec, narrowspec,
repair, repair,
repository, repository,
util, util,
wireprototypes, wireprototypes,
) )
from mercurial.utils import ( from mercurial.utils import (
stringutil, stringutil,
clkills.add(ck) clkills.add(ck)
else: else:
raise error.Abort( raise error.Abort(
_('unexpected changespec node chunk type: %s') % chunksignal) _('unexpected changespec node chunk type: %s') % chunksignal)
chunksignal = changegroup.readexactly(inpart, 4) chunksignal = changegroup.readexactly(inpart, 4)
if clkills: if clkills:
# preserve bookmarks that repair.strip() would otherwise strip # preserve bookmarks that repair.strip() would otherwise strip
bmstore = repo._bookmarks op._bookmarksbackup = repo._bookmarks
class dummybmstore(dict): class dummybmstore(dict):
def applychanges(self, repo, tr, changes): def applychanges(self, repo, tr, changes):
pass pass
repo._bookmarks = dummybmstore() localrepo.localrepository._bookmarks.set(repo, dummybmstore())
chgrpfile = repair.strip(op.ui, repo, list(clkills), backup=True, chgrpfile = repair.strip(op.ui, repo, list(clkills), backup=True,
topic='widen') topic='widen')
repo._bookmarks = bmstore
if chgrpfile: if chgrpfile:
op._widen_uninterr = repo.ui.uninterruptable() op._widen_uninterr = repo.ui.uninterruptable()
op._widen_uninterr.__enter__() op._widen_uninterr.__enter__()
# presence of _widen_bundle attribute activates widen handler later # presence of _widen_bundle attribute activates widen handler later
op._widen_bundle = chgrpfile op._widen_bundle = chgrpfile
# Set the new narrowspec if we're widening. The setnewnarrowpats() method # Set the new narrowspec if we're widening. The setnewnarrowpats() method
# will currently always be there when using the core+narrowhg server, but # will currently always be there when using the core+narrowhg server, but
# other servers may include a changespec part even when not widening (e.g. # other servers may include a changespec part even when not widening (e.g.
exchange.getbundle2partsmapping['changegroup'] = wrappedcgfn exchange.getbundle2partsmapping['changegroup'] = wrappedcgfn
# Extend changegroup receiver so client can fixup after widen requests. # Extend changegroup receiver so client can fixup after widen requests.
origcghandler = bundle2.parthandlermapping['changegroup'] origcghandler = bundle2.parthandlermapping['changegroup']
def wrappedcghandler(op, inpart): def wrappedcghandler(op, inpart):
origcghandler(op, inpart) origcghandler(op, inpart)
if util.safehasattr(op, '_widen_bundle'): if util.safehasattr(op, '_widen_bundle'):
handlechangegroup_widen(op, inpart) handlechangegroup_widen(op, inpart)
if util.safehasattr(op, '_bookmarksbackup'):
localrepo.localrepository._bookmarks.set(op.repo,
op._bookmarksbackup)
del op._bookmarksbackup
wrappedcghandler.params = origcghandler.params wrappedcghandler.params = origcghandler.params
bundle2.parthandlermapping['changegroup'] = wrappedcghandler bundle2.parthandlermapping['changegroup'] = wrappedcghandler