Patchwork share: replace the bookmarks.shared file with an entry on a new shared.conf file

login
register
mail settings
Submitter Angel Ezquerra
Date Jan. 11, 2015, 3:23 p.m.
Message ID <8eca5c61ad384fdd4640.1420989822@Angels-MacBook-Pro.local>
Download mbox | patch
Permalink /patch/7434/
State Changes Requested
Headers show

Comments

Angel Ezquerra - Jan. 11, 2015, 3:23 p.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1420989615 -3600
#      Sun Jan 11 16:20:15 2015 +0100
# Node ID 8eca5c61ad384fdd4640ff6ef0c51062d3171878
# Parent  678f53865c6860a950392691814766957ee89316
share: replace the bookmarks.shared file with an entry on a new shared.conf file

cd79fb4d75fd introduced a way to share bookmarks. When a repository share that
shares bookmarks was created, a .hg/bookmarks.shared file was created to mark
the repository share as one that shares its bookmarks.

We have plans to introduce other levels of sharing, including a "full share"
mode. Rather than creating a new ".shared" file for each new thing that we may
want to share It seems better to create a single share.conf file that will list
what is shared for a given file. This should make it much easier to get a list
of everything that is shared by a given shared repository.

The shared file is a config file that can be read with mercurial's config
module. I've decided against calling the config file "sharerc" to emphasize the
fact that this file is not meant to be edited by users. Each shared "item" is
added as an entry in the "share" section of the share.conf file. For now the
only possible entry in that section is "bookmarks".
Siddharth Agarwal - Jan. 12, 2015, 6:52 a.m.
On 01/11/2015 07:23 AM, Angel Ezquerra wrote:
> The shared file is a config file that can be read with mercurial's config
> module. I've decided against calling the config file "sharerc" to emphasize the
> fact that this file is not meant to be edited by users. Each shared "item" is
> added as an entry in the "share" section of the share.conf file. For now the
> only possible entry in that section is "bookmarks".

I don't think share.conf indicates non-editability any more than
sharerc. Maybe just a file called 'shares' or 'shared', similar to
'requires'?

Do you actually anticipate any use of full-blown configs? If not, why
not make it similar to the requires file?
Ryan McElroy - Jan. 12, 2015, 6:28 p.m.
On 1/11/2015 10:52 PM, Siddharth Agarwal wrote:
> On 01/11/2015 07:23 AM, Angel Ezquerra wrote:
>> The shared file is a config file that can be read with mercurial's config
>> module. I've decided against calling the config file "sharerc" to emphasize the
>> fact that this file is not meant to be edited by users. Each shared "item" is
>> added as an entry in the "share" section of the share.conf file. For now the
>> only possible entry in that section is "bookmarks".
> I don't think share.conf indicates non-editability any more than
> sharerc. Maybe just a file called 'shares' or 'shared', similar to
> 'requires'?
>
> Do you actually anticipate any use of full-blown configs? If not, why
> not make it similar to the requires file?
I agree that making it more similar to the requires file (eg, not in 
"configuration file format" with [sections]) makes more sense here.

I like the idea of having a single file for shared parts of the repo 
instead of a .shared file for each part though; this is definitely a 
good direction.
Angel Ezquerra - Jan. 12, 2015, 7:46 p.m.
El lunes, 12 de enero de 2015, Siddharth Agarwal <sid@less-broken.com>
escribió:

> On 01/11/2015 07:23 AM, Angel Ezquerra wrote:
> > The shared file is a config file that can be read with mercurial's config
> > module. I've decided against calling the config file "sharerc" to
> emphasize the
> > fact that this file is not meant to be edited by users. Each shared
> "item" is
> > added as an entry in the "share" section of the share.conf file. For now
> the
> > only possible entry in that section is "bookmarks".
>
> I don't think share.conf indicates non-editability any more than
> sharerc. Maybe just a file called 'shares' or 'shared', similar to
> 'requires'?


Ok, that makes sense.

Do you actually anticipate any use of full-blown configs? If not, why
> not make it similar to the requires file?
>

I won't have anything in mind right now. However I think there are a couple
of arguments in favor of using an in like format:

- mercurial already knows how to read config files
- it is a known format that does not require documenting how the info is
stored
- it is extensible, so if a new config need comes up in the future we won't
need to create some new separate file with some arbitrary name to hold it

That being said I don't feel super strongly about this. I just want to make
this change before the freeze.

Cheers,

Angel

Patch

diff --git a/hgext/share.py b/hgext/share.py
--- a/hgext/share.py
+++ b/hgext/share.py
@@ -6,7 +6,7 @@ 
 '''share a common history between several working directories'''
 
 from mercurial.i18n import _
-from mercurial import cmdutil, hg, util, extensions, bookmarks
+from mercurial import cmdutil, hg, util, extensions, bookmarks, config
 from mercurial.hg import repository, parseurl
 import errno
 
@@ -78,13 +78,11 @@ 
 
 def _hassharedbookmarks(repo):
     """Returns whether this repo has shared bookmarks"""
-    try:
-        repo.vfs.read('bookmarks.shared')
-        return True
-    except IOError, inst:
-        if inst.errno != errno.ENOENT:
-            raise
+    if not repo.vfs.exists('share.conf'):
         return False
+    cfg = config.config()
+    cfg.read(repo.vfs.join('share.conf'))
+    return cfg.get('share', 'bookmarks', None) is not None
 
 def _getsrcrepo(repo):
     """
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -226,7 +226,9 @@ 
         _update(r, uprev)
 
     if bookmarks:
-        r.opener('bookmarks.shared', 'w').close()
+        fp = r.opener('share.conf', 'w')
+        fp.write('[share]\nbookmarks=\n')
+        fp.close()
 
 def copystore(ui, srcrepo, destpath):
     '''copy files from store of srcrepo in destpath