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&id=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 rep