| Submitter | Boris Feld |
|---|---|
| Date | Nov. 20, 2017, 4:52 p.m. |
| Message ID | <7e610585998e3eff1d04.1511196721@FB> |
| Download | mbox | patch |
| Permalink | /patch/25663/ |
| State | Accepted |
| Headers | show |
Comments
Potential bug mentioned inline On 11/20/17 8:52 AM, Boris Feld wrote: > # HG changeset patch > # User Boris Feld <boris.feld@octobus.net> > # Date 1508246776 -7200 > # Tue Oct 17 15:26:16 2017 +0200 > # Node ID 7e610585998e3eff1d0498a6ce024350bb04fc23 > # Parent ab0fe3f91e7b9ed7bd508a9affa0b91128facf13 > # EXP-Topic b2.bookmarks > # Available At https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_octobus_mercurial-2Ddevel_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=EyaOozG7mqYFV7-uTtZuxV3zYCWicwAVyqzUZmeNyFQ&s=6vm3KUlcZ0dR7UkeLmcuPjYetLgy50kNjAlNrys3aEI&e= > # hg pull https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_octobus_mercurial-2Ddevel_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=EyaOozG7mqYFV7-uTtZuxV3zYCWicwAVyqzUZmeNyFQ&s=6vm3KUlcZ0dR7UkeLmcuPjYetLgy50kNjAlNrys3aEI&e= -r 7e610585998e > bundle2: support a 'records' mode for the 'bookmarks' part > > In this mode, the bookmarks changes are record in the 'bundleoperation' records > instead of inflicted to the repository. This is necessary to use the part when > pulling. > > diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py > --- a/mercurial/bundle2.py > +++ b/mercurial/bundle2.py > @@ -1898,40 +1898,55 @@ def handlepushkey(op, inpart): > def handlebookmark(op, inpart): > """transmit bookmark information > > - The part contains binary encoded bookmark information. The bookmark > - information is applied as is to the unbundling repository. Make sure a > - 'check:bookmarks' part is issued earlier to check for race condition in > - such update. > + The part contains binary encoded bookmark information. > + > + The exact behavior of this part can be controlled by the 'bookmarks' mode > + on the bundle operation. > > - This behavior is suitable for pushing. Semantic adjustment will be needed > - for pull. > + When mode is 'apply' (the default) the bookmark information is applied as > + is to the unbundling repository. Make sure a 'check:bookmarks' part is > + issued earlier to check for push races in such update. This behavior is > + suitable for pushing. > + > + When mode is 'records', the information is recorded into the 'bookmarks' > + records of the bundle operation. This behavior is suitable for pulling. > """ > changes = bookmarks.binarydecode(inpart) > > - tr = op.gettransaction() > - bookstore = op.repo._bookmarks > + pushkeycompat = op.repo.ui.configbool('server', 'bookmarks-pushkey-compat') > + bookmarksmode = op.modes.get('bookmarks', 'apply') > > - pushkeycompat = op.repo.ui.configbool('server', 'bookmarks-pushkey-compat') > - if pushkeycompat: > - allhooks = [] > + if bookmarksmode == 'apply': > + tr = op.gettransaction() > + bookstore = op.repo._bookmarks > + if pushkeycompat: > + allhooks = [] > + for book, node in changes: > + hookargs = tr.hookargs.copy() > + hookargs['pushkeycompat'] = '1' > + hookargs['namespace'] = 'bookmark' > + hookargs['key'] = book > + hookargs['old'] = nodemod.hex(bookstore.get(book, '')) > + hookargs['new'] = nodemod.hex(node if node is not None else '') > + allhooks.append(hookargs) > + > + for hookargs in allhooks: > + op.repo.hook('prepushkey', throw=True, **hookargs) > + > + bookstore.applychanges(op.repo, op.gettransaction(), changes) > + > + if pushkeycompat: > + def runhook(): > + for hookargs in allhooks: > + op.repo.hook('prepushkey', **hookargs) Should this be 'pushkey' instead of 'prepushkey'? Since it happens after the lock? That would match localrepository.pushkey()'s behavior. > + op.repo._afterlock(runhook) > + > + elif bookmarksmode == 'records': > for book, node in changes: > - hookargs = tr.hookargs.copy() > - hookargs['pushkeycompat'] = '1' > - hookargs['namespace'] = 'bookmark' > - hookargs['key'] = book > - hookargs['old'] = nodemod.hex(bookstore.get(book, '')) > - hookargs['new'] = nodemod.hex(node if node is not None else '') > - allhooks.append(hookargs) > - for hookargs in allhooks: > - op.repo.hook('prepushkey', throw=True, **hookargs) > - > - bookstore.applychanges(op.repo, tr, changes) > - > - if pushkeycompat: > - def runhook(): > - for hookargs in allhooks: > - op.repo.hook('prepushkey', **hookargs) > - op.repo._afterlock(runhook) > + record = {'bookmark': book, 'node': node} > + op.records.add('bookmarks', record) > + else: > + raise error.ProgrammingError('unkown bookmark mode: %s' % bookmarksmode) > > @parthandler('phase-heads') > def handlephases(op, inpart): > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=EyaOozG7mqYFV7-uTtZuxV3zYCWicwAVyqzUZmeNyFQ&s=7YABt1cm45Mp__z7bxjPwnISb9XFpHkbVFYIoVgsA9U&e= >
Thank you for the report, it was already spotted and fixed by https://www.mercurial-scm.org/repo/hg/rev/2f54a3e228ff 2 months ago. On 22/03/2018 18:49, Durham Goode wrote: > Potential bug mentioned inline > > On 11/20/17 8:52 AM, Boris Feld wrote: >> # HG changeset patch >> # User Boris Feld <boris.feld@octobus.net> >> # Date 1508246776 -7200 >> # Tue Oct 17 15:26:16 2017 +0200 >> # Node ID 7e610585998e3eff1d0498a6ce024350bb04fc23 >> # Parent ab0fe3f91e7b9ed7bd508a9affa0b91128facf13 >> # EXP-Topic b2.bookmarks >> # Available At >> https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_octobus_mercurial-2Ddevel_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=EyaOozG7mqYFV7-uTtZuxV3zYCWicwAVyqzUZmeNyFQ&s=6vm3KUlcZ0dR7UkeLmcuPjYetLgy50kNjAlNrys3aEI&e= >> # hg pull >> https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_octobus_mercurial-2Ddevel_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=EyaOozG7mqYFV7-uTtZuxV3zYCWicwAVyqzUZmeNyFQ&s=6vm3KUlcZ0dR7UkeLmcuPjYetLgy50kNjAlNrys3aEI&e= >> -r 7e610585998e >> bundle2: support a 'records' mode for the 'bookmarks' part >> >> In this mode, the bookmarks changes are record in the >> 'bundleoperation' records >> instead of inflicted to the repository. This is necessary to use the >> part when >> pulling. >> >> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py >> --- a/mercurial/bundle2.py >> +++ b/mercurial/bundle2.py >> @@ -1898,40 +1898,55 @@ def handlepushkey(op, inpart): >> def handlebookmark(op, inpart): >> """transmit bookmark information >> >> - The part contains binary encoded bookmark information. The bookmark >> - information is applied as is to the unbundling repository. Make >> sure a >> - 'check:bookmarks' part is issued earlier to check for race >> condition in >> - such update. >> + The part contains binary encoded bookmark information. >> + >> + The exact behavior of this part can be controlled by the >> 'bookmarks' mode >> + on the bundle operation. >> >> - This behavior is suitable for pushing. Semantic adjustment will >> be needed >> - for pull. >> + When mode is 'apply' (the default) the bookmark information is >> applied as >> + is to the unbundling repository. Make sure a 'check:bookmarks' >> part is >> + issued earlier to check for push races in such update. This >> behavior is >> + suitable for pushing. >> + >> + When mode is 'records', the information is recorded into the >> 'bookmarks' >> + records of the bundle operation. This behavior is suitable for >> pulling. >> """ >> changes = bookmarks.binarydecode(inpart) >> >> - tr = op.gettransaction() >> - bookstore = op.repo._bookmarks >> + pushkeycompat = op.repo.ui.configbool('server', >> 'bookmarks-pushkey-compat') >> + bookmarksmode = op.modes.get('bookmarks', 'apply') >> >> - pushkeycompat = op.repo.ui.configbool('server', >> 'bookmarks-pushkey-compat') >> - if pushkeycompat: >> - allhooks = [] >> + if bookmarksmode == 'apply': >> + tr = op.gettransaction() >> + bookstore = op.repo._bookmarks >> + if pushkeycompat: >> + allhooks = [] >> + for book, node in changes: >> + hookargs = tr.hookargs.copy() >> + hookargs['pushkeycompat'] = '1' >> + hookargs['namespace'] = 'bookmark' >> + hookargs['key'] = book >> + hookargs['old'] = nodemod.hex(bookstore.get(book, '')) >> + hookargs['new'] = nodemod.hex(node if node is not >> None else '') >> + allhooks.append(hookargs) >> + >> + for hookargs in allhooks: >> + op.repo.hook('prepushkey', throw=True, **hookargs) >> + >> + bookstore.applychanges(op.repo, op.gettransaction(), changes) >> + >> + if pushkeycompat: >> + def runhook(): >> + for hookargs in allhooks: >> + op.repo.hook('prepushkey', **hookargs) > > Should this be 'pushkey' instead of 'prepushkey'? Since it happens > after the lock? That would match localrepository.pushkey()'s behavior. > >> + op.repo._afterlock(runhook) >> + >> + elif bookmarksmode == 'records': >> for book, node in changes: >> - hookargs = tr.hookargs.copy() >> - hookargs['pushkeycompat'] = '1' >> - hookargs['namespace'] = 'bookmark' >> - hookargs['key'] = book >> - hookargs['old'] = nodemod.hex(bookstore.get(book, '')) >> - hookargs['new'] = nodemod.hex(node if node is not None >> else '') >> - allhooks.append(hookargs) >> - for hookargs in allhooks: >> - op.repo.hook('prepushkey', throw=True, **hookargs) >> - >> - bookstore.applychanges(op.repo, tr, changes) >> - >> - if pushkeycompat: >> - def runhook(): >> - for hookargs in allhooks: >> - op.repo.hook('prepushkey', **hookargs) >> - op.repo._afterlock(runhook) >> + record = {'bookmark': book, 'node': node} >> + op.records.add('bookmarks', record) >> + else: >> + raise error.ProgrammingError('unkown bookmark mode: %s' % >> bookmarksmode) >> >> @parthandler('phase-heads') >> def handlephases(op, inpart): >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=EyaOozG7mqYFV7-uTtZuxV3zYCWicwAVyqzUZmeNyFQ&s=7YABt1cm45Mp__z7bxjPwnISb9XFpHkbVFYIoVgsA9U&e= >> >> > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py --- a/mercurial/bundle2.py +++ b/mercurial/bundle2.py @@ -1898,40 +1898,55 @@ def handlepushkey(op, inpart): def handlebookmark(op, inpart): """transmit bookmark information - The part contains binary encoded bookmark information. The bookmark - information is applied as is to the unbundling repository. Make sure a - 'check:bookmarks' part is issued earlier to check for race condition in - such update. + The part contains binary encoded bookmark information. + + The exact behavior of this part can be controlled by the 'bookmarks' mode + on the bundle operation. - This behavior is suitable for pushing. Semantic adjustment will be needed - for pull. + When mode is 'apply' (the default) the bookmark information is applied as + is to the unbundling repository. Make sure a 'check:bookmarks' part is + issued earlier to check for push races in such update. This behavior is + suitable for pushing. + + When mode is 'records', the information is recorded into the 'bookmarks' + records of the bundle operation. This behavior is suitable for pulling. """ changes = bookmarks.binarydecode(inpart) - tr = op.gettransaction() - bookstore = op.repo._bookmarks + pushkeycompat = op.repo.ui.configbool('server', 'bookmarks-pushkey-compat') + bookmarksmode = op.modes.get('bookmarks', 'apply') - pushkeycompat = op.repo.ui.configbool('server', 'bookmarks-pushkey-compat') - if pushkeycompat: - allhooks = [] + if bookmarksmode == 'apply': + tr = op.gettransaction() + bookstore = op.repo._bookmarks + if pushkeycompat: + allhooks = [] + for book, node in changes: + hookargs = tr.hookargs.copy() + hookargs['pushkeycompat'] = '1' + hookargs['namespace'] = 'bookmark' + hookargs['key'] = book + hookargs['old'] = nodemod.hex(bookstore.get(book, '')) + hookargs['new'] = nodemod.hex(node if node is not None else '') + allhooks.append(hookargs) + + for hookargs in allhooks: + op.repo.hook('prepushkey', throw=True, **hookargs) + + bookstore.applychanges(op.repo, op.gettransaction(), changes) + + if pushkeycompat: + def runhook(): + for hookargs in allhooks: + op.repo.hook('prepushkey', **hookargs) + op.repo._afterlock(runhook) + + elif bookmarksmode == 'records': for book, node in changes: - hookargs = tr.hookargs.copy() - hookargs['pushkeycompat'] = '1' - hookargs['namespace'] = 'bookmark' - hookargs['key'] = book - hookargs['old'] = nodemod.hex(bookstore.get(book, '')) - hookargs['new'] = nodemod.hex(node if node is not None else '') - allhooks.append(hookargs) - for hookargs in allhooks: - op.repo.hook('prepushkey', throw=True, **hookargs) - - bookstore.applychanges(op.repo, tr, changes) - - if pushkeycompat: - def runhook(): - for hookargs in allhooks: - op.repo.hook('prepushkey', **hookargs) - op.repo._afterlock(runhook) + record = {'bookmark': book, 'node': node} + op.records.add('bookmarks', record) + else: + raise error.ProgrammingError('unkown bookmark mode: %s' % bookmarksmode) @parthandler('phase-heads') def handlephases(op, inpart):