| Submitter | Ryan McElroy |
|---|---|
| Date | Dec. 1, 2014, 8:57 p.m. |
| Message ID | <beb7a11c50c34e38adea.1417467443@devbig105.prn2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/6935/ |
| State | Changes Requested |
| Headers | show |
Comments
On Mon, Dec 1, 2014 at 9:57 PM, Ryan McElroy <rm@fb.com> wrote: > # HG changeset patch > # User Ryan McElroy <rmcelroy@fb.com> > # Date 1417078134 28800 > # Thu Nov 27 00:48:54 2014 -0800 > # Node ID beb7a11c50c34e38adea9d8dd3650d42d16cf0ff > # Parent c4f7d3fbe855041951dfccb40b05031def426a2b > share: accept optional bookmarks parameter > > This modifies the share function in the hg module so it takes a boolean > bookmarks parameter and plumbs this through to the share command added by > the extension. If this parameter is true, the function will create a > bookmarks.shared file in the destination repository with contents pointing to > the source repository with which the destination should share bookmarks. > > The actual bookmark sharing functionality will be added in a later patch. Why do you need to write the source path on the bookmarks.shared file on the destination repository? Shouldn't it be the same path as the one stored on the sharedpath file? > diff --git a/hgext/share.py b/hgext/share.py > --- a/hgext/share.py > +++ b/hgext/share.py > @@ -13,10 +13,12 @@ > testedwith = 'internal' > > @command('share', > - [('U', 'noupdate', None, _('do not create a working copy'))], > - _('[-U] SOURCE [DEST]'), > + [('U', 'noupdate', None, _('do not create a working copy')), > + ('B', 'bookmarks', None, _('share bookmarks with source repository')), > + ], > + _('[-U] [-B] SOURCE [DEST]'), > norepo=True) > -def share(ui, source, dest=None, noupdate=False): > +def share(ui, source, dest=None, noupdate=False, bookmarks=False): > """create a new shared repository > > Initialize a new repository and working directory that shares its > @@ -34,7 +36,7 @@ > the broken clone to reset it to a changeset that still exists. > """ > > - return hg.share(ui, source, dest, not noupdate) > + return hg.share(ui, source, dest, not noupdate, bookmarks) > > @command('unshare', [], '') > def unshare(ui, repo): > diff --git a/mercurial/hg.py b/mercurial/hg.py > --- a/mercurial/hg.py > +++ b/mercurial/hg.py > @@ -158,7 +158,7 @@ > return '' > return os.path.basename(os.path.normpath(path)) > > -def share(ui, source, dest=None, update=True): > +def share(ui, source, dest=None, update=True, bookmarks=False): > '''create a shared repository''' > > if not islocal(source): > @@ -225,6 +225,19 @@ > continue > _update(r, uprev) > > + if bookmarks: > + bookpath = srcrepo.wvfs.base > + # follow shared repo to source > + try: > + bookpath = srcrepo.vfs.read('bookmarks.shared') > + ui.debug('bookmark path changed to %s' % bookpath) > + except IOError, inst: > + ui.debug('bookmkar path left as %s' % bookpath) > + if inst.errno != errno.ENOENT: > + raise There is a typo on the ui.debug message. > + destvfs.write('bookmarks.shared', bookpath) > + > def copystore(ui, srcrepo, destpath): > '''copy files from store of srcrepo in destpath In addition to these comments I'd like to know if you plan to add additional sharing options (e.g. mq patches). On the last sprint I told mpm that I would look into doing something similar to this. The idea is to make it possible to use shared repos to create a sort of subrepository "cache" (although that is not really the right word). The idea is to add a new "full share" mode that would create shares that share everything with their source except the minimum amount of things that are needed to maintain two separate working directories. In my implementation these full shares would work by making the destination repository use a vfs that would access the source repository in all cases except a few exceptions. This seems quite different to your approach. I'd like to make sure that what you do does not clash with what I am trying to do. I already have most of this implemented, but I have been quite busy with RL these past few months so I am not ready to share it yet (no pun intended :-) ). I hope to have some time in the next few weeks though. Cheers, Angel
> -----Original Message----- > From: ezquerra@gmail.com [mailto:ezquerra@gmail.com] On Behalf Of > Angel Ezquerra > Sent: Tuesday, December 2, 2014 2:24 PM > To: Ryan McElroy > Cc: mercurial-devel > Subject: Re: [PATCH 2 of 3] share: accept optional bookmarks parameter > > On Mon, Dec 1, 2014 at 9:57 PM, Ryan McElroy <rm@fb.com> wrote: > > # HG changeset patch > > # User Ryan McElroy <rmcelroy@fb.com> > > # Date 1417078134 28800 > > # Thu Nov 27 00:48:54 2014 -0800 > > # Node ID beb7a11c50c34e38adea9d8dd3650d42d16cf0ff > > # Parent c4f7d3fbe855041951dfccb40b05031def426a2b > > share: accept optional bookmarks parameter > > > > This modifies the share function in the hg module so it takes a > > boolean bookmarks parameter and plumbs this through to the share > > command added by the extension. If this parameter is true, the > > function will create a bookmarks.shared file in the destination > > repository with contents pointing to the source repository with which the > destination should share bookmarks. > > > > The actual bookmark sharing functionality will be added in a later patch. > > Why do you need to write the source path on the bookmarks.shared file on > the destination repository? Shouldn't it be the same path as the one stored > on the sharedpath file? In the original version, sharing bookmarks was optional, and the path in the sharedpath file pointed to not-quite-the-right place so I considered it easier to store exactly what I wanted in this file. However, if it's going to be non-optional (looks like it), I agree that it makes sense to just reuse the sharedpath file and do the appropriate manipulations on the path stored there. I'll make that change in V3. > > > diff --git a/hgext/share.py b/hgext/share.py > > --- a/hgext/share.py > > +++ b/hgext/share.py > > @@ -13,10 +13,12 @@ > > testedwith = 'internal' > > > > @command('share', > > - [('U', 'noupdate', None, _('do not create a working copy'))], > > - _('[-U] SOURCE [DEST]'), > > + [('U', 'noupdate', None, _('do not create a working copy')), > > + ('B', 'bookmarks', None, _('share bookmarks with source repository')), > > + ], > > + _('[-U] [-B] SOURCE [DEST]'), > > norepo=True) > > -def share(ui, source, dest=None, noupdate=False): > > +def share(ui, source, dest=None, noupdate=False, bookmarks=False): > > """create a new shared repository > > > > Initialize a new repository and working directory that shares its > > @@ -34,7 +36,7 @@ > > the broken clone to reset it to a changeset that still exists. > > """ > > > > - return hg.share(ui, source, dest, not noupdate) > > + return hg.share(ui, source, dest, not noupdate, bookmarks) > > > > @command('unshare', [], '') > > def unshare(ui, repo): > > diff --git a/mercurial/hg.py b/mercurial/hg.py > > --- a/mercurial/hg.py > > +++ b/mercurial/hg.py > > @@ -158,7 +158,7 @@ > > return '' > > return os.path.basename(os.path.normpath(path)) > > > > -def share(ui, source, dest=None, update=True): > > +def share(ui, source, dest=None, update=True, bookmarks=False): > > '''create a shared repository''' > > > > if not islocal(source): > > @@ -225,6 +225,19 @@ > > continue > > _update(r, uprev) > > > > + if bookmarks: > > + bookpath = srcrepo.wvfs.base > > + # follow shared repo to source > > + try: > > + bookpath = srcrepo.vfs.read('bookmarks.shared') > > + ui.debug('bookmark path changed to %s' % bookpath) > > + except IOError, inst: > > + ui.debug('bookmkar path left as %s' % bookpath) > > + if inst.errno != errno.ENOENT: > > + raise > > There is a typo on the ui.debug message. Thanks for noticing that, I will fix it. > > > + destvfs.write('bookmarks.shared', bookpath) > > + > > def copystore(ui, srcrepo, destpath): > > '''copy files from store of srcrepo in destpath > > > In addition to these comments I'd like to know if you plan to add additional > sharing options (e.g. mq patches). On the last sprint I told mpm that I would > look into doing something similar to this. The idea is to make it possible to use > shared repos to create a sort of subrepository "cache" (although that is not > really the right word). > > The idea is to add a new "full share" mode that would create shares that > share everything with their source except the minimum amount of things > that are needed to maintain two separate working directories. > In my implementation these full shares would work by making the > destination repository use a vfs that would access the source repository in all > cases except a few exceptions. This seems quite different to your approach. > I'd like to make sure that what you do does not clash with what I am trying to > do. > > I already have most of this implemented, but I have been quite busy with RL > these past few months so I am not ready to share it yet (no pun intended :-) > ). I hope to have some time in the next few weeks though. I don't have plans beyond bookmarks. If you have something coming down the pipe that does much more complete sharing than this, that sounds pretty promising and I would be happy to see my approach here reverted when that comes in. Do you feel landing a revised version of this patch would harm what you're trying to do? > > Cheers, > > Angel
On 12/02/2014 02:23 PM, Angel Ezquerra wrote: > In addition to these comments I'd like to know if you plan to add > additional sharing options (e.g. mq patches). On the last sprint I > told mpm that I would look into doing something similar to this. The > idea is to make it possible to use shared repos to create a sort of > subrepository "cache" (although that is not really the right word). > > The idea is to add a new "full share" mode that would create shares > that share everything with their source except the minimum amount of > things that are needed to maintain two separate working directories. > In my implementation these full shares would work by making the > destination repository use a vfs that would access the source > repository in all cases except a few exceptions. This seems quite > different to your approach. I'd like to make sure that what you do > does not clash with what I am trying to do. I'm working on reworking the transaction logic and the repository format. My initial aims is to make the transaction atomic, but I will try to clean things up and help the shared case in the process. We should probably talk about your usecase and need.
El 03/12/2014 00:56, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> escribió: > > > > On 12/02/2014 02:23 PM, Angel Ezquerra wrote: >> >> In addition to these comments I'd like to know if you plan to add >> additional sharing options (e.g. mq patches). On the last sprint I >> told mpm that I would look into doing something similar to this. The >> idea is to make it possible to use shared repos to create a sort of >> subrepository "cache" (although that is not really the right word). >> >> The idea is to add a new "full share" mode that would create shares >> that share everything with their source except the minimum amount of >> things that are needed to maintain two separate working directories. >> In my implementation these full shares would work by making the >> destination repository use a vfs that would access the source >> repository in all cases except a few exceptions. This seems quite >> different to your approach. I'd like to make sure that what you do >> does not clash with what I am trying to do. > > > I'm working on reworking the transaction logic and the repository format. My initial aims is to make the transaction atomic, but I will try to clean things up and help the shared case in the process. We should probably talk about your usecase and need. I'm literally going to the hospital right now, to have our second baby, so I'll be busy for a couple of days! :-) After that we could schedule some time on IRC to chat about this in some detail? Angel
Patch
diff --git a/hgext/share.py b/hgext/share.py --- a/hgext/share.py +++ b/hgext/share.py @@ -13,10 +13,12 @@ testedwith = 'internal' @command('share', - [('U', 'noupdate', None, _('do not create a working copy'))], - _('[-U] SOURCE [DEST]'), + [('U', 'noupdate', None, _('do not create a working copy')), + ('B', 'bookmarks', None, _('share bookmarks with source repository')), + ], + _('[-U] [-B] SOURCE [DEST]'), norepo=True) -def share(ui, source, dest=None, noupdate=False): +def share(ui, source, dest=None, noupdate=False, bookmarks=False): """create a new shared repository Initialize a new repository and working directory that shares its @@ -34,7 +36,7 @@ the broken clone to reset it to a changeset that still exists. """ - return hg.share(ui, source, dest, not noupdate) + return hg.share(ui, source, dest, not noupdate, bookmarks) @command('unshare', [], '') def unshare(ui, repo): diff --git a/mercurial/hg.py b/mercurial/hg.py --- a/mercurial/hg.py +++ b/mercurial/hg.py @@ -158,7 +158,7 @@ return '' return os.path.basename(os.path.normpath(path)) -def share(ui, source, dest=None, update=True): +def share(ui, source, dest=None, update=True, bookmarks=False): '''create a shared repository''' if not islocal(source): @@ -225,6 +225,19 @@ continue _update(r, uprev) + if bookmarks: + bookpath = srcrepo.wvfs.base + # follow shared repo to source + try: + bookpath = srcrepo.vfs.read('bookmarks.shared') + ui.debug('bookmark path changed to %s' % bookpath) + except IOError, inst: + ui.debug('bookmkar path left as %s' % bookpath) + if inst.errno != errno.ENOENT: + raise + + destvfs.write('bookmarks.shared', bookpath) + def copystore(ui, srcrepo, destpath): '''copy files from store of srcrepo in destpath