[PATCH 1 of 2 V2] share: implement shared bookmark functionality
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Tue Dec 2 17:48:16 CST 2014
On 12/02/2014 01:39 PM, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy at fb.com>
> # Date 1417078134 28800
> # Thu Nov 27 00:48:54 2014 -0800
> # Node ID 5fc6fc3b954321f2aed07d7270f560e7c4c867be
> # Parent c4f7d3fbe855041951dfccb40b05031def426a2b
> share: implement shared bookmark functionality
>
> The hg.share function now creates a bookmarks.shared file in the destination
> repository with contents pointing to the source repository with which the
> destination should share bookmarks.
>
> This enables bookmarks to be shared between shared repositories by extending
> the bookmarks.bmstore methods that read and write bookmarks to take into account
> a bookmarks.shared file that can point to another repository.
>
> diff --git a/hgext/share.py b/hgext/share.py
> --- a/hgext/share.py
> +++ b/hgext/share.py
> @@ -6,7 +6,9 @@
> '''share a common history between several working directories'''
>
> from mercurial.i18n import _
> -from mercurial import cmdutil, hg, util
> +from mercurial import cmdutil, hg, util, extensions, bookmarks
> +from mercurial.hg import repository, parseurl
> +import errno
>
> cmdtable = {}
> command = cmdutil.command(cmdtable)
> @@ -20,7 +22,7 @@ def share(ui, source, dest=None, noupdat
> """create a new shared repository
>
> Initialize a new repository and working directory that shares its
> - history with another repository.
> + history and bookmarks with another repository.
I think we can pretend bookmarks are part of history (Matt is going to
object but they are). We we probably not need to update this.
> .. note::
>
> @@ -67,3 +69,45 @@ def unshare(ui, repo):
>
> # update store, spath, sopener and sjoin of repo
> repo.unfiltered().__init__(repo.baseui, repo.root)
> +
> +def extsetup(ui):
> + extensions.wrapfunction(bookmarks.bmstore, 'getbkfile', getbkfile)
> + extensions.wrapfunction(bookmarks.bmstore, 'recordchange', recordchange)
> + extensions.wrapfunction(bookmarks.bmstore, 'write', write)
> +
> +def getsrcrepo(repo):
This function is doing some smart stuff. It would be nice to document it.
> + srcrepo = None
> + try:
> + source = repo.vfs.read('bookmarks.shared')
> + srcurl, branches = parseurl(source)
> + srcrepo = repository(repo.ui, srcurl)
> + except IOError, inst:
> + if inst.errno != errno.ENOENT:
> + raise
> + return srcrepo
> +
> +def getbkfile(orig, self, repo):
> + repo = getsrcrepo(repo) or repo
Meh, we tend to avoid the `A or B` python magic because it is confusing
to new comer.
> + return orig(self, repo)
> +
> +def recordchange(orig, self, tr):
> + tr.addfilegenerator('bookmarks', ('bookmarks',), self._write)
> + orig(self, tr)
> + _write(self)
So as pointer in a comment on V1 (sent after you sent a V2), your extra
write is not going to be transactional here. You should hook yourself to
the transaction finalization.
Also a bit confused about why the first function line is useful. Look
absolutly redundant with the first one.
> +
> +def write(orig, self):
> + # First write a bookmarks file in case we ever unshare
> + orig(self)
> + _write(self)
> +
> +def _write(self):
You probably what to document what is going on here.
> + repo = getsrcrepo(self._repo) or self._repo
> +
> + wlock = repo.wlock()
> + try:
> + f = repo.vfs('bookmarks', 'w', atomictemp=True)
> + self._write(f)
> + f.close()
> +
> + finally:
> + wlock.release()
You should rely on the existing mechanism when not in the shared case.
Otherwise you'll most probably mess up transaction semantic.
> diff --git a/mercurial/hg.py b/mercurial/hg.py
> --- a/mercurial/hg.py
> +++ b/mercurial/hg.py
> @@ -225,6 +225,18 @@ def share(ui, source, dest=None, update=
> continue
> _update(r, uprev)
>
> + 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)
> +
I'm a bit confused about what is going one here. It seems that we
already have a sharedpatch file. Why do we needs an extra one for bookmarks?
> def copystore(ui, srcrepo, destpath):
> '''copy files from store of srcrepo in destpath
>
> diff --git a/tests/test-share.t b/tests/test-share.t
> --- a/tests/test-share.t
> +++ b/tests/test-share.t
> @@ -128,6 +128,114 @@ check that a change does not propagate
>
> $ cd ..
>
> +
> +test sharing bookmarks
> +
> + $ hg share repo1 repo3
> + updating working directory
> + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
> + $ cd repo1
> + $ hg bookmark bm1
> + $ hg bookmarks
> + * bm1 2:c2e0ac586386
> + $ cd ../repo2
> + $ hg book bm2
> + $ hg bookmarks
> + bm1 2:c2e0ac586386
> + * bm2 3:0e6e70d1d5f1
> + $ cd ../repo3
> + $ hg bookmarks
> + bm1 2:c2e0ac586386
> + $ hg book bm3
> + $ hg bookmarks
> + bm1 2:c2e0ac586386
> + * bm3 2:c2e0ac586386
> + $ cd ../repo1
> + $ hg bookmarks
> + * bm1 2:c2e0ac586386
> + bm3 2:c2e0ac586386
> +
> +test that commits work
> +
> + $ echo 'shared bookmarks' > a
> + $ hg commit -m 'testing shared bookmarks'
> + $ hg bookmarks
> + * bm1 3:b87954705719
> + bm3 2:c2e0ac586386
> + $ cd ../repo3
> + $ hg bookmarks
> + bm1 3:b87954705719
> + * bm3 2:c2e0ac586386
> + $ echo 'more shared bookmarks' > a
> + $ hg commit -m 'testing shared bookmarks'
> + created new head
> + $ hg bookmarks
> + bm1 3:b87954705719
> + * bm3 4:62f4ded848e4
> + $ cd ../repo1
> + $ hg bookmarks
> + * bm1 3:b87954705719
> + bm3 4:62f4ded848e4
> + $ cd ..
> +
> +test pushing bookmarks works
> +
> + $ hg clone repo3 repo4
> + updating to branch default
> + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
> + $ cd repo4
> + $ hg boo bm4
> + $ echo foo > b
> + $ hg commit -m 'foo in b'
> + $ hg boo
> + bm1 3:b87954705719
> + bm3 4:62f4ded848e4
> + * bm4 5:92793bfc8cad
> + $ hg push -B bm4
> + pushing to $TESTTMP/repo3 (glob)
> + searching for changes
> + adding changesets
> + adding manifests
> + adding file changes
> + added 1 changesets with 1 changes to 1 files
> + exporting bookmark bm4
> + $ cd ../repo1
> + $ hg bookmarks
> + * bm1 3:b87954705719
> + bm3 4:62f4ded848e4
> + bm4 5:92793bfc8cad
> + $ cd ../repo3
> + $ hg bookmarks
> + bm1 3:b87954705719
> + * bm3 4:62f4ded848e4
> + bm4 5:92793bfc8cad
> + $ cd ..
> +
> +test behavior when sharing a shared repo
> +
> + $ hg share repo3 repo5
> + updating working directory
> + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
> + $ cd repo5
> + $ hg book
> + bm1 3:b87954705719
> + bm3 4:62f4ded848e4
> + bm4 5:92793bfc8cad
> + $ cd ..
> +
> +test what happens when an active bookmark is deleted
> +
> + $ cd repo1
> + $ hg boo -d bm3
> + $ hg boo
> + * bm1 3:b87954705719
> + bm4 5:92793bfc8cad
> + $ cd ../repo3
> + $ hg boo
> + bm1 3:b87954705719
> + bm4 5:92793bfc8cad
> +
> +
> Explicitly kill daemons to let the test exit on Windows
>
> $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS
You probably what to add a test to check the bookmark are not updated if
the transaction is aborted.
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list