This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4557

hg: don't reuse repo instance after unshare()
ClosedPublic

Authored by indygreg on Sep 12 2018, 10:02 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGc5e6c1ba1c79: hg: don't reuse repo instance after unshare()
Summary

Unsharing a repository is a pretty invasive procedure and fundamentally
changes the behavior of the repository.

Currently, hg.unshare() calls into localrepository.init to
re-initialize the repository instance. This is a bit hacky. And
future commits that refactor how localrepository instances are
constructed will make this difficult to support.

This commit changes unshare() so it constructs a new repo instance
once the unshare I/O has completed. It then poisons the old repo
instance so any further use will result in error.

Surprisingly, nothing in core appears to access a repo instance
after it has been unshared!

.. api::

``hg.unshare()`` now poisons the repo instance so it can't be used.

It also returns a new repo instance suitable for interacting with
the unshared repository.

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

indygreg created this revision.Sep 12 2018, 10:02 PM
Closed by commit rHGc5e6c1ba1c79: hg: don't reuse repo instance after unshare() (authored by indygreg). · Explain WhySep 13 2018, 11:42 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 11000

mercurial/hg.py

postshare(srcrepo, r, bookmarks=bookmarks, defaultpath=defaultpath) postshare(srcrepo, r, bookmarks=bookmarks, defaultpath=defaultpath)
_postshareupdate(r, update, checkout=checkout) _postshareupdate(r, update, checkout=checkout)
return r return r
def unshare(ui, repo): def unshare(ui, repo):
"""convert a shared repository to a normal one """convert a shared repository to a normal one
Copy the store data to the repo and remove the sharedpath data. Copy the store data to the repo and remove the sharedpath data.
Returns a new repository object representing the unshared repository.
The passed repository object is not usable after this function is
called.
""" """
destlock = lock = None destlock = lock = None
lock = repo.lock() lock = repo.lock()
try: try:
# we use locks here because if we race with commit, we # we use locks here because if we race with commit, we
# can end up with extra data in the cloned revlogs that's # can end up with extra data in the cloned revlogs that's
# not pointed to by changesets, thus causing verify to # not pointed to by changesets, thus causing verify to
# fail # fail
destlock = copystore(ui, repo, repo.path) destlock = copystore(ui, repo, repo.path)
sharefile = repo.vfs.join('sharedpath') sharefile = repo.vfs.join('sharedpath')
util.rename(sharefile, sharefile + '.old') util.rename(sharefile, sharefile + '.old')
repo.requirements.discard('shared') repo.requirements.discard('shared')
repo.requirements.discard('relshared') repo.requirements.discard('relshared')
repo._writerequirements() repo._writerequirements()
finally: finally:
destlock and destlock.release() destlock and destlock.release()
lock and lock.release() lock and lock.release()
# update store, spath, svfs and sjoin of repo # Removing share changes some fundamental properties of the repo instance.
repo.unfiltered().__init__(repo.baseui, repo.root) # So we instantiate a new repo object and operate on it rather than
# try to keep the existing repo usable.
newrepo = repository(repo.baseui, repo.root, create=False)
# TODO: figure out how to access subrepos that exist, but were previously # TODO: figure out how to access subrepos that exist, but were previously
# removed from .hgsub # removed from .hgsub
c = repo['.'] c = newrepo['.']
subs = c.substate subs = c.substate
for s in sorted(subs): for s in sorted(subs):
c.sub(s).unshare() c.sub(s).unshare()
localrepo.poisonrepository(repo)
return newrepo
def postshare(sourcerepo, destrepo, bookmarks=True, defaultpath=None): def postshare(sourcerepo, destrepo, bookmarks=True, defaultpath=None):
"""Called after a new shared repo is created. """Called after a new shared repo is created.
The new repo only has a requirements file and pointer to the source. The new repo only has a requirements file and pointer to the source.
This function configures additional shared data. This function configures additional shared data.
Extensions can wrap this function and write additional entries to Extensions can wrap this function and write additional entries to
destrepo/.hg/shared to indicate additional pieces of data to be shared. destrepo/.hg/shared to indicate additional pieces of data to be shared.

mercurial/localrepo.py

# #
# The revlog header has version 2, which won't be recognized by # The revlog header has version 2, which won't be recognized by
# such old clients. # such old clients.
hgvfs.append(b'00changelog.i', hgvfs.append(b'00changelog.i',
b'\0\0\0\2 dummy changelog to prevent using the old repo ' b'\0\0\0\2 dummy changelog to prevent using the old repo '
b'layout') b'layout')
scmutil.writerequires(hgvfs, requirements) scmutil.writerequires(hgvfs, requirements)
def poisonrepository(repo):
"""Poison a repository instance so it can no longer be used."""
# Perform any cleanup on the instance.
repo.close()
# Our strategy is to replace the type of the object with one that
# has all attribute lookups result in error.
#
# But we have to allow the close() method because some constructors
# of repos call close() on repo references.
class poisonedrepository(object):
def __getattribute__(self, item):
if item == r'close':
return object.__getattribute__(self, item)
raise error.ProgrammingError('repo instances should not be used '
'after unshare')
def close(self):
pass
# We may have a repoview, which intercepts __setattr__. So be sure
# we operate at the lowest level possible.
object.__setattr__(repo, r'__class__', poisonedrepository)