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

2018-09-13 Thread indygreg (Gregory Szorc)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGc5e6c1ba1c79: hg: dont reuse repo instance after 
unshare() (authored by indygreg, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D4557?vs=10989=11000

REVISION DETAIL
  https://phab.mercurial-scm.org/D4557

AFFECTED FILES
  mercurial/hg.py
  mercurial/localrepo.py

CHANGE DETAILS

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2503,3 +2503,28 @@
  b'layout')
 
 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)
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -307,6 +307,11 @@
 """convert a shared repository to a normal one
 
 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
@@ -329,16 +334,22 @@
 destlock and destlock.release()
 lock and lock.release()
 
-# update store, spath, svfs and sjoin of repo
-repo.unfiltered().__init__(repo.baseui, repo.root)
+# Removing share changes some fundamental properties of the repo instance.
+# 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
 #   removed from .hgsub
-c = repo['.']
+c = newrepo['.']
 subs = c.substate
 for s in sorted(subs):
 c.sub(s).unshare()
 
+localrepo.poisonrepository(repo)
+
+return newrepo
+
 def postshare(sourcerepo, destrepo, bookmarks=True, defaultpath=None):
 """Called after a new shared repo is created.
 



To: indygreg, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


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

2018-09-12 Thread indygreg (Gregory Szorc)
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION 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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4557

AFFECTED FILES
  mercurial/hg.py
  mercurial/localrepo.py

CHANGE DETAILS

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -2503,3 +2503,28 @@
  b'layout')
 
 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)
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -307,6 +307,11 @@
 """convert a shared repository to a normal one
 
 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
@@ -329,16 +334,22 @@
 destlock and destlock.release()
 lock and lock.release()
 
-# update store, spath, svfs and sjoin of repo
-repo.unfiltered().__init__(repo.baseui, repo.root)
+# Removing share changes some fundamental properties of the repo instance.
+# 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
 #   removed from .hgsub
-c = repo['.']
+c = newrepo['.']
 subs = c.substate
 for s in sorted(subs):
 c.sub(s).unshare()
 
+localrepo.poisonrepository(repo)
+
+return newrepo
+
 def postshare(sourcerepo, destrepo, bookmarks=True, defaultpath=None):
 """Called after a new shared repo is created.
 



To: indygreg, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel