Patchwork [1,of,6,V2,RFC] localrepo: introduce shared method to check if a repository is shared

login
register
mail settings
Submitter Angel Ezquerra
Date Dec. 26, 2014, 11:46 a.m.
Message ID <b213f4439d5427cfff6c.1419594409@108.1.168.192.in-addr.arpa>
Download mbox | patch
Permalink /patch/7242/
State Accepted
Commit 965788d9ae09887b688a6c6b2ffa4e170f00f1d5
Headers show

Comments

Angel Ezquerra - Dec. 26, 2014, 11:46 a.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1419117550 -3600
#      Sun Dec 21 00:19:10 2014 +0100
# Node ID b213f4439d5427cfff6c7fa28927c25e4422a948
# Parent  67d63ec85eb72187508692e52f14be46101707a5
localrepo: introduce shared method to check if a repository is shared

Up until now we compared the "path" and "sharedpath" repository properties to
check if a repository is shared. This was relying an implementation detail of
shared repositories. In order to make it possible to change the way shared
repositories are implemented, we encapsulate this check into its own localrepo
method, called shared.

This new method returns None if the repository is shared, and something else
(for now a string describing the short of share) otherwise.

The reason why I did not call this method "isshared" and made it return a
boolean is that I plan to introduce a new type of shared repository soon.

# NOTES:

This is the first patch in a series whose purpose is to add support for
creating "full repository shares", which are repositories that share everything
with the repository source except their current revision, branch and bookmark.

This series is RFC because I am not very sure of some of the solutions I took.
Comments are welcome!
Pierre-Yves David - Dec. 30, 2014, 10 p.m.
On 12/26/2014 03:46 AM, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra@gmail.com>
> # Date 1419117550 -3600
> #      Sun Dec 21 00:19:10 2014 +0100
> # Node ID b213f4439d5427cfff6c7fa28927c25e4422a948
> # Parent  67d63ec85eb72187508692e52f14be46101707a5
> localrepo: introduce shared method to check if a repository is shared

According to the "Localrepo Diet Act" adding rarely used method on the 
localrepository class is a criminal offense. Can we stick this as a 
function in the shared extensions instead?

Moreover, we recently added the option to share bookmark. Could we use a 
different "type" for this case too?
Angel Ezquerra - Dec. 31, 2014, 12:46 a.m.
On Tue, Dec 30, 2014 at 11:00 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 12/26/2014 03:46 AM, Angel Ezquerra wrote:
>>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra@gmail.com>
>> # Date 1419117550 -3600
>> #      Sun Dec 21 00:19:10 2014 +0100
>> # Node ID b213f4439d5427cfff6c7fa28927c25e4422a948
>> # Parent  67d63ec85eb72187508692e52f14be46101707a5
>> localrepo: introduce shared method to check if a repository is shared
>
>
> According to the "Localrepo Diet Act" adding rarely used method on the
> localrepository class is a criminal offense. Can we stick this as a function
> in the shared extensions instead?

The problem is that the localrepository.sharedpath property is not
private and people are already using it, comparing it to
localrepository.path, to check if a repository is shared. I think this
is bad form and also limits the ways in which we can change the way
shared subrepos are implemented. My solution was to commit the crime
and add a new method to localrepository :-> (then we could make
sharedpath private). Do you think adding a function in the share
extension would solve the problem? My feeling is that it would not.

> Moreover, we recently added the option to share bookmark. Could we use a
> different "type" for this case too?

Possibly. Other options would be for the share method to return a list
of what is shared.

Angel

Patch

diff --git a/hgext/share.py b/hgext/share.py
--- a/hgext/share.py
+++ b/hgext/share.py
@@ -46,7 +46,7 @@ 
     Copy the store data to the repo and remove the sharedpath data.
     """
 
-    if repo.sharedpath == repo.path:
+    if not repo.shared():
         raise util.Abort(_("this is not a shared repo"))
 
     destlock = lock = None
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -758,6 +758,12 @@ 
         # if publishing we can't copy if there is filtered content
         return not self.filtered('visible').changelog.filteredrevs
 
+    def shared(self):
+        '''the type of shared repository (None if not shared)'''
+        if self.sharedpath != self.path:
+            return 'store'
+        return None
+
     def join(self, f, *insidef):
         return os.path.join(self.path, f, *insidef)
 
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -302,7 +302,7 @@ 
             return repo.ui.config('paths', 'default-push')
         if repo.ui.config('paths', 'default'):
             return repo.ui.config('paths', 'default')
-        if repo.sharedpath != repo.path:
+        if repo.shared():
             # chop off the .hg component to get the default path form
             return os.path.dirname(repo.sharedpath)
     if abort: