Merge lp:~spiv/bzr/checkout-tags-propagation-603395-2.2 into lp:bzr/2.2
Status: | Rejected |
---|---|
Rejected by: | Andrew Bennetts |
Proposed branch: | lp:~spiv/bzr/checkout-tags-propagation-603395-2.2 |
Merge into: | lp:bzr/2.2 |
Diff against target: |
423 lines (+272/-16) 8 files modified
NEWS (+13/-0) bzrlib/branch.py (+1/-1) bzrlib/builtins.py (+3/-1) bzrlib/tag.py (+69/-13) bzrlib/tests/blackbox/test_merge.py (+1/-1) bzrlib/tests/blackbox/test_tags.py (+12/-0) bzrlib/tests/per_branch/test_tags.py (+170/-0) bzrlib/tests/test_tag.py (+3/-0) |
To merge this branch: | bzr merge lp:~spiv/bzr/checkout-tags-propagation-603395-2.2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Vincent Ladeuil | Disapprove | ||
John A Meinel | Pending | ||
Review via email: mp+40406@code.launchpad.net |
This proposal supersedes a proposal from 2010-11-01.
Description of the change
This is part of the fix for bug 603395. It makes BasicTags.merge_to also merge to the master branch, if there is one. This is consistent with e.g. set_tag. bzr-builddeb directly calls that API, and presumably expects merge_to in a checkout branch to update the master as well. Because bzr-builddeb does call this API directly I don't believe <https:/
I've dealt with my reservations on the original merge proposal: merge_to now accepts an optional kwarg, ignore_master, that cmd_merge uses to avoid prematurely propagating changes to the master. So I think this avoids any chance of regressing at all on #99137 (bzr revert will still not remove the new tags from the child branch, but that's not a regression). This does mean that external implementations of merge_to should be updated. I've documented the API change in NEWS, and made sure the callsite in bzr that does pass that kwarg falls back to not passing the kwarg. I've tested the fallback manually with bzr-svn (which has an external implementation of merge_to), and it appears to work as intended.
The tests take care to deal with all the permutations, including those that can only be triggered by an unfixed bzr operating on the checkout. See the test comments for details.
I should point out: this does have the perhaps unwanted side-effect that "bzr merge" can cause tags to be added the master even if the next command is "bzr commit". That's really <https:/ /bugs.launchpad .net/bzr/ +bug/99137> (tags are "permanently" propagated by merge), and this patch is just making the overall behaviour more consistent. I can see an argument that undoing the accidental limiting of the the effect of bug 99137 might not be acceptable in a stable branch, though.
A compromise would be to add a new optional argument to merge_to (or perhaps a whole new method) that callers can use to explicitly enable/disable propagation. I'm not sure if it would be better to default to enabled (so that plugins like builddeb can automatically benefit), or disabled (to minimise the change in behaviour, and also perhaps to avoid needing to tweak the API layers between cmd_merge and br.tags.merge_to to pass the flag). With 'bzr merge' not propagating we'd then rely on <https:/ /code.launchpad .net/~spiv/ bzr/tags- commit- propagation- 603395- 2.2> to propagate at commit time rather than merge time.