D2880: bundle: add the possibility to bundle bookmarks (issue5792)
This revision now requires changes to proceed. baymax added a comment. baymax requested changes to this revision. There seems to have been no activities on this Diff for the past 3 Months. By policy, we are automatically moving it out of the `need-review` state. Please, move it back to `need-review` without hesitation if this diff should still be discussed. :baymax:need-review-idle: REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D2880/new/ REVISION DETAIL https://phab.mercurial-scm.org/D2880 To: lothiraldan, #hg-reviewers, indygreg, baymax Cc: martinvonz, indygreg, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
martinvonz added a comment. Just a reminder that this has not been queued yet. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2880 To: lothiraldan, #hg-reviewers, indygreg Cc: martinvonz, indygreg, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
lothiraldan added inline comments. INLINE COMMENTS > martinvonz wrote in test-bundle-bookmarks.t:37-63 > I think it should be the same thing as pulling from another repo. Try putting > the same changes in another repo and pulling from it without giving the path > a name (nothing in [paths] config). I haven't checked what suffix will be > used on the divergent bookmark, but it probably makes sense to use the same > here (perhaps it's just going to be "https://phab.mercurial-scm.org/D1@1; as > I guessed before). I tried adding such scenarios but failed to produce a divergence scenario, no idea why. I don't think I will have time to debug the issue before the freeze, so let's skip this changeset for 4.6 REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2880 To: lothiraldan, #hg-reviewers, indygreg Cc: martinvonz, indygreg, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
martinvonz added inline comments. INLINE COMMENTS > lothiraldan wrote in test-bundle-bookmarks.t:37-63 > I have added such test and indeed this case is not handled right now. I'm not > aware of how Mercurial handle this case when exchanging bookmarks, I will > need to take a look at the code to find out. > > What is the correct behavior here? Change the existing bookmark or the > bookmark from the bundle? I think it should be the same thing as pulling from another repo. Try putting the same changes in another repo and pulling from it without giving the path a name (nothing in [paths] config). I haven't checked what suffix will be used on the divergent bookmark, but it probably makes sense to use the same here (perhaps it's just going to be "https://phab.mercurial-scm.org/D1@1; as I guessed before). REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2880 To: lothiraldan, #hg-reviewers, indygreg Cc: martinvonz, indygreg, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
lothiraldan added inline comments.
INLINE COMMENTS
> martinvonz wrote in test-bundle-bookmarks.t:37-63
> Can we have a similar test case where we create divergence? Create a fork in
> the graph in the debugdrawdag call above. Let's say you have commit F that
> branches off of B, then do something like this:
>
> $ hg bundle --all bundle
> $ hg strip --no-backup C
> $ hg bookmarks -f -r F D1
> $ hg unbundle -q bundle
> $ hg log -G -T '{desc} {bookmarks}\n'
> o E
> |
> o D D1 D2
> |
> o C
> |
> | o F D1@1
> |/
> o B
> |
> o A A1
>
> I don't know if "@1" is the appropriate divergence marker, but I can't think
> of a better one. I haven't even looked at your code to try to guess what it
> would be in practice (if it would work at all).
I have added such test and indeed this case is not handled right now. I'm not
aware of how Mercurial handle this case when exchanging bookmarks, I will need
to take a look at the code to find out.
What is the correct behavior here? Change the existing bookmark or the bookmark
from the bundle?
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2880
To: lothiraldan, #hg-reviewers, indygreg
Cc: martinvonz, indygreg, pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
lothiraldan updated this revision to Diff 8263.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D2880?vs=7077=8263
REVISION DETAIL
https://phab.mercurial-scm.org/D2880
AFFECTED FILES
mercurial/bundle2.py
mercurial/commands.py
mercurial/configitems.py
mercurial/debugcommands.py
tests/test-bundle-bookmarks.t
CHANGE DETAILS
diff --git a/tests/test-bundle-bookmarks.t b/tests/test-bundle-bookmarks.t
new file mode 100644
--- /dev/null
+++ b/tests/test-bundle-bookmarks.t
@@ -0,0 +1,116 @@
+ $ cat >> $HGRCPATH < [experimental]
+ > bundle-bookmarks=yes
+ > [extensions]
+ > strip=
+ > drawdag=$TESTDIR/drawdag.py
+ > EOF
+
+Set up repo with linear history
+ $ hg init linear
+ $ cd linear
+ $ hg debugdrawdag <<'EOF'
+ > E
+ > |
+ > D
+ > |
+ > C
+ > |
+ > B
+ > |
+ > A
+ > EOF
+ $ hg bookmarks -r A "A1"
+ $ hg bookmarks -r D "D1"
+ $ hg bookmarks -r D "D2"
+ $ hg log -G -T '{desc} {bookmarks}\n'
+ o E
+ |
+ o D D1 D2
+ |
+ o C
+ |
+ o B
+ |
+ o A A1
+
+Bookmarks are restored when unbundling
+ $ hg bundle --all bundle
+ 5 changesets found
+ $ hg debugbundle bundle
+ Stream params: {Compression: BZ}
+ changegroup -- {nbchanges: 5, version: 02}
+ 426bada5c67598ca65036d57d9e4b64b0c1ce7a0
+ 112478962961147124edd43549aedd1a335e44bf
+ 26805aba1e600a82e93661149f2313866a221a7b
+ f585351a92f85104bff7c284233c338b10eb1df7
+ 9bc730a19041f9ec7cb33c626e811aa233efb18c
+ bookmarks -- {}
+ A1: Bk\xad\xa5\xc6u\x98\xcae\x03mW\xd9\xe4\xb6K\x0c\x1c\xe7\xa0 (esc)
+ D1: \xf5\x855\x1a\x92\xf8Q\x04\xbf\xf7\xc2\x84#<3\x8b\x10\xeb\x1d\xf7
(esc)
+ D2: \xf5\x855\x1a\x92\xf8Q\x04\xbf\xf7\xc2\x84#<3\x8b\x10\xeb\x1d\xf7
(esc)
+ $ hg strip --no-backup C
+ $ hg unbundle -q bundle
+ $ hg log -G -T '{desc} {bookmarks}\n'
+ o E
+ |
+ o D D1 D2
+ |
+ o C
+ |
+ o B
+ |
+ o A A1
+
+Bookmarks doesn't conflict with local bookmarks
+
+ $ hg bookmarks -d A1
+ $ hg bookmarks -r A "A2"
+ $ hg unbundle -q bundle
+ $ hg log -G -T '{desc} {bookmarks}\n'
+ o E
+ |
+ o D D1 D2
+ |
+ o C
+ |
+ o B
+ |
+ o A A1 A2
+
+Test bookmarks divergence
+
+ $ hg bundle --all bundle
+ 5 changesets found
+ $ hg strip --no-backup C
+
+Create soem divergence
+ $ hg up B
+ 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+ $ touch F
+ $ hg add F
+ $ hg commit -m "F"
+ $ hg bookmarks -f -r "desc(F)" D1
+
+ $ hg log -G -T '{desc} {bookmarks}\n'
+ @ F D1
+ |
+ o B D2
+ |
+ o A A1 A2
+
+
+ $ hg unbundle -q bundle
+ $ hg log -G -T '{desc} {bookmarks}\n'
+ o E
+ |
+ o D D1 D2
+ |
+ o C
+ |
+ | @ F
+ |/
+ o B
+ |
+ o A A1 A2
+
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -33,6 +33,7 @@
short,
)
from . import (
+bookmarks,
bundle2,
changegroup,
cmdutil,
@@ -326,6 +327,14 @@
ui.write(indent_string)
ui.write('%s %s\n' % (hex(head), phases.phasenames[phase]))
+def _debugbookmarks(ui, data, indent=0):
+"""display version and markers contained in 'data'"""
+indent_string = ' ' * indent
+bm = bookmarks.binarydecode(data)
+for bookmark in sorted(bm):
+ui.write(indent_string)
+ui.write('%s: %s\n' % (bookmark[0], bookmark[1]))
+
def _quasirepr(thing):
if isinstance(thing, (dict, util.sortdict, collections.OrderedDict)):
return '{%s}' % (
@@ -353,6 +362,9 @@
if part.type == 'phase-heads':
if not ui.quiet:
_debugphaseheads(ui, part, indent=4)
+if part.type == 'bookmarks':
+if not ui.quiet:
+_debugbookmarks(ui, part, indent=4)
@command('debugbundle',
[('a', 'all', None, _('show all details')),
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -419,6 +419,9 @@
coreconfigitem('experimental', 'archivemetatemplate',
default=dynamicdefault,
)
+coreconfigitem('experimental', 'bundle-bookmarks',
+default=False,
+)
coreconfigitem('experimental', 'bundle-phases',
default=False,
)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1267,6 +1267,8 @@
contentopts['obsolescence'] = True
if repo.ui.configbool('experimental', 'bundle-phases'):
contentopts['phases'] = True
+if repo.ui.configbool('experimental', 'bundle-bookmarks'):
+contentopts['bookmarks'] = True
bundle2.writenewbundle(ui, repo, 'bundle', fname, bversion, outgoing,
contentopts, compression=bcompression,
compopts=compopts)
@@ -5406,7 +5408,7 @@
"""
fnames = (fname1,) + fnames
-with repo.lock():
+with repo.wlock(), repo.lock():
for fname in fnames:
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
lothiraldan added a comment. In https://phab.mercurial-scm.org/D2880#52559, @indygreg wrote: > What's the status of this patch? Is it still reviewable? If so, let's get a rebased version submitted, just in case things have changed. I need to update the patch according to @martinvoz comment. I was also waiting on your comment on https://phab.mercurial-scm.org/D2880#46263, do you think we will keep the option once the bundlespec system is revamped? If yes, we can make the config option a non-experimental one. But if it will be dropped soon, I think keeping it experimental until it's dropped is preferable. What do you think? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2880 To: lothiraldan, #hg-reviewers, indygreg Cc: martinvonz, indygreg, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
indygreg requested changes to this revision. indygreg added a comment. This revision now requires changes to proceed. What's the status of this patch? Is it still reviewable? If so, let's get a rebased version submitted, just in case things have changed. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2880 To: lothiraldan, #hg-reviewers, indygreg Cc: martinvonz, indygreg, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
martinvonz added inline comments.
INLINE COMMENTS
> test-bundle-bookmarks.t:37-63
> +Bookmarks are restored when unbundling
> + $ hg bundle --all bundle
> + 5 changesets found
> + $ hg debugbundle bundle
> + Stream params: {Compression: BZ}
> + changegroup -- {nbchanges: 5, version: 02}
> + 426bada5c67598ca65036d57d9e4b64b0c1ce7a0
Can we have a similar test case where we create divergence? Create a fork in
the graph in the debugdrawdag call above. Let's say you have commit F that
branches off of B, then do something like this:
$ hg bundle --all bundle
$ hg strip --no-backup C
$ hg bookmarks -f -r F D1
$ hg unbundle -q bundle
$ hg log -G -T '{desc} {bookmarks}\n'
o E
|
o D D1 D2
|
o C
|
| o F D1@1
|/
o B
|
o A A1
I don't know if "@1" is the appropriate divergence marker, but I can't think of
a better one. I haven't even looked at your code to try to guess what it would
be in practice (if it would work at all).
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2880
To: lothiraldan, #hg-reviewers
Cc: martinvonz, indygreg, pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
lothiraldan added a subscriber: indygreg. lothiraldan added inline comments. INLINE COMMENTS > pulkit wrote in configitems.py:422 > Any reason why we have all these config option as experimental? We talked at > sprint about moving things out of experimental and we agreed on > `experimental.bundle-phases`. This one sounds similar. I was afraid we might have to delete this option once we have the discussion about bundle spec format with @indygreg. If that would be the case, I would feel more comfortable deleting an experimental option. If we agreed on moving bundle-phases out of experimental, I guess we can do the same for bundle-bookmarks. @indygreg Do you think we would still have this option around once we have new-style bundle spec? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2880 To: lothiraldan, #hg-reviewers Cc: indygreg, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
pulkit added inline comments.
INLINE COMMENTS
> configitems.py:422
> )
> +coreconfigitem('experimental', 'bundle-bookmarks',
> +default=False,
Any reason why we have all these config option as experimental? We talked at
sprint about moving things out of experimental and we agreed on
`experimental.bundle-phases`. This one sounds similar.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2880
To: lothiraldan, #hg-reviewers
Cc: pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D2880: bundle: add the possibility to bundle bookmarks (issue5792)
lothiraldan created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
Also take the wlock when unbundling. It shouldn't have a big impact on
performance.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2880
AFFECTED FILES
mercurial/bundle2.py
mercurial/commands.py
mercurial/configitems.py
mercurial/debugcommands.py
tests/test-bundle-bookmarks.t
CHANGE DETAILS
diff --git a/tests/test-bundle-bookmarks.t b/tests/test-bundle-bookmarks.t
new file mode 100644
--- /dev/null
+++ b/tests/test-bundle-bookmarks.t
@@ -0,0 +1,80 @@
+ $ cat >> $HGRCPATH < [experimental]
+ > bundle-bookmarks=yes
+ > [extensions]
+ > strip=
+ > drawdag=$TESTDIR/drawdag.py
+ > EOF
+
+Set up repo with linear history
+ $ hg init linear
+ $ cd linear
+ $ hg debugdrawdag <<'EOF'
+ > E
+ > |
+ > D
+ > |
+ > C
+ > |
+ > B
+ > |
+ > A
+ > EOF
+ $ hg bookmarks -r A "A1"
+ $ hg bookmarks -r D "D1"
+ $ hg bookmarks -r D "D2"
+ $ hg log -G -T '{desc} {bookmarks}\n'
+ o E
+ |
+ o D D1 D2
+ |
+ o C
+ |
+ o B
+ |
+ o A A1
+
+Bookmarks are restored when unbundling
+ $ hg bundle --all bundle
+ 5 changesets found
+ $ hg debugbundle bundle
+ Stream params: {Compression: BZ}
+ changegroup -- {nbchanges: 5, version: 02}
+ 426bada5c67598ca65036d57d9e4b64b0c1ce7a0
+ 112478962961147124edd43549aedd1a335e44bf
+ 26805aba1e600a82e93661149f2313866a221a7b
+ f585351a92f85104bff7c284233c338b10eb1df7
+ 9bc730a19041f9ec7cb33c626e811aa233efb18c
+ bookmarks -- {}
+ A1: Bk\xad\xa5\xc6u\x98\xcae\x03mW\xd9\xe4\xb6K\x0c\x1c\xe7\xa0 (esc)
+ D1: \xf5\x855\x1a\x92\xf8Q\x04\xbf\xf7\xc2\x84#<3\x8b\x10\xeb\x1d\xf7
(esc)
+ D2: \xf5\x855\x1a\x92\xf8Q\x04\xbf\xf7\xc2\x84#<3\x8b\x10\xeb\x1d\xf7
(esc)
+ $ hg strip --no-backup C
+ $ hg unbundle -q bundle
+ $ hg log -G -T '{desc} {bookmarks}\n'
+ o E
+ |
+ o D D1 D2
+ |
+ o C
+ |
+ o B
+ |
+ o A A1
+
+Bookmarks doesn't conflict with local bookmarks
+
+ $ hg bookmarks -d A1
+ $ hg bookmarks -r A "A2"
+ $ hg unbundle -q bundle
+ $ hg log -G -T '{desc} {bookmarks}\n'
+ o E
+ |
+ o D D1 D2
+ |
+ o C
+ |
+ o B
+ |
+ o A A1 A2
+
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -33,6 +33,7 @@
short,
)
from . import (
+bookmarks,
bundle2,
changegroup,
cmdutil,
@@ -326,6 +327,14 @@
ui.write(indent_string)
ui.write('%s %s\n' % (hex(head), phases.phasenames[phase]))
+def _debugbookmarks(ui, data, indent=0):
+"""display version and markers contained in 'data'"""
+indent_string = ' ' * indent
+bm = bookmarks.binarydecode(data)
+for bookmark in sorted(bm):
+ui.write(indent_string)
+ui.write('%s: %s\n' % (bookmark[0], bookmark[1]))
+
def _quasirepr(thing):
if isinstance(thing, (dict, util.sortdict, collections.OrderedDict)):
return '{%s}' % (
@@ -353,6 +362,9 @@
if part.type == 'phase-heads':
if not ui.quiet:
_debugphaseheads(ui, part, indent=4)
+if part.type == 'bookmarks':
+if not ui.quiet:
+_debugbookmarks(ui, part, indent=4)
@command('debugbundle',
[('a', 'all', None, _('show all details')),
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -419,6 +419,9 @@
coreconfigitem('experimental', 'archivemetatemplate',
default=dynamicdefault,
)
+coreconfigitem('experimental', 'bundle-bookmarks',
+default=False,
+)
coreconfigitem('experimental', 'bundle-phases',
default=False,
)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1267,6 +1267,8 @@
contentopts['obsolescence'] = True
if repo.ui.configbool('experimental', 'bundle-phases'):
contentopts['phases'] = True
+if repo.ui.configbool('experimental', 'bundle-bookmarks'):
+contentopts['bookmarks'] = True
bundle2.writenewbundle(ui, repo, 'bundle', fname, bversion, outgoing,
contentopts, compression=bcompression,
compopts=compopts)
@@ -5406,7 +5408,7 @@
"""
fnames = (fname1,) + fnames
-with repo.lock():
+with repo.wlock(), repo.lock():
for fname in fnames:
f = hg.openpath(ui, fname)
gen = exchange.readbundle(ui, f, fname)
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1599,6 +1599,14 @@
phasedata = phases.binaryencode(headsbyphase)
bundler.newpart('phase-heads', data=phasedata)
+if opts.get('bookmarks', False):
+# From exchange._pushb2bookmarkspart
+data = []
+for book, new in
