On Fri, Sep 30, 2011 at 7:24 AM, Julian Foad <julian.foad_at_wandisco.com> wrote:
> I've been thinking about a bunch of issues related to merging and how to
> help users to avoid some of the common pitfalls. One subset of my
> thoughts is on how we can present information about merges in a more
> user-friendly high-level way so that each time a user runs "svn merge"
> or "svn mergeinfo" they get feedback that relates to their high-level
> idea of what should be happening, and thus helps them to see at a glance
> whether they issued the right command for what they wanted to achieve.
>
> So I'm mocking up stuff like this (some data faked):
>
> $ cd 1.6.x
>
> $ svn mergeinfo $BRANCHES_URL/1.6.x-r878590
> Source branch: ^/subversion/branches/1.6.x-r878590 (r1177182)
> Target branch: ^/subversion/branches/1.6.x (wc)
> Source and target are marked as branches of the same project.
> Merged revision ranges:
> START-1104299,1104304,1104333-END
> Merged operative revisions:
> 1104267,1104304,1104336,1104339,1104346
>
> and this:
>
> $ svn mergeinfo $BRANCHES_URL/1.6.x
> Source branch: ^/subversion/branches/1.6.x (r1177182)
> Target branch: ^/subversion/branches/1.6.x (wc)
> svn: E195016: Source and target are the same branch
>
> and this:
>
> $ svn mergeinfo
> Assuming source branch is copied-from source of target branch.
> Source branch: ^/subversion/trunk (r875961)
> Target branch: ^/subversion/branches/1.6.x (wc)
> [...]
>
> I'm making a branch to try out ideas like these.
>
> I'm writing down my thoughts in this public document
> <https://docs.google.com/a/wandisco.com/document/d/1nrU3IkP8Q1NbFgH1ou0w3N6QkghbJv6AnK8wlnPAgC0/edit?hl=en_GB>; see especially under the "Reporting ..." section headings.
Hi Julian,
So as to keep my thoughts on the medium of record, I'm pasting my
comments on your doc here, rather than as comments in google docs :
(Which raises a question, why aren't we keeping these notes in our own
repository?)
[[[
> Guiding Users into Simple and Safe Merging
> Aim
> Make it much harder to merge the “Wrong Way” by accident.
> Identify the typical merging tasks, which may vary depending on the
> requirements of the user’s team. Make the typical merging tasks easy
> to perform; look at TortoiseSVN and other existing GUIs for examples.
> Clearly distinguish tasks that have different meanings but may
> currently have similar or identical syntax, to reduce the risk of a
> user performing a merge that produces the desired output now but will
> adversely affect future merges. Make it harder to accidentally
> specify merges that are unusual and not well supported.
> Implement these measures by building onto Subversion’s existing
> library API, command-line UI, client-server interface and/or any other
> suitable level. If this requires knowledge of the user’s branching
> model and rules, provide a simple way to configure this and provide
> one or more typical configurations.
> A pre-requisite for designing and implementing such guidance is a
> clear statement of what merging scenarios Subversion supports. See
> the document Supported Merge Scenarios.
> Pitfalls & Quick Fixes
> The aim here is to list things that users might do in Subversion that
> are not Right — that is, things that will not lead to consistent and
> useful results in merging later on — that could be avoided by some
> high-level checks or by better informing the user.
> Using branch naming terminology from the ‘Terminology’ section below.
>
> On a dev
You need to define a 'dev' branch in your terminology section.
Assuming you mean "feature" branch.
> branch, attempting to merge to parent without
> “--reintegrate” is wrong, (except perhaps for a cherry-pick, and that
> would be unusual). Catch this case. Possibly with hook script? Need
> awareness of what kind of branch ‘this’ is.
Awareness of "branch-roots" might be as, or more, useful or at least a
first step to identifying branch type.
> Catch other cases from the ‘Branching and Merging Policy’ section below.
> On any branch, a reverse-merge of a merge in this branch’s own
> history will not do what we expect, I think. The correct (better?) way
> to roll back a merge is to reverse-merge the original changes from the
> source branch.
A couple advantages to simply reverse merging from the branch's own history:
1) Avoidance of any conflicts the original merge caused.
2) If the merge we want to reverse was a reintegrate merge from a
backport branch, the exact merge to perform is non-obvious. Take for
example r1176459 on 1.7.x:
[[[
Reintergrate the 1.7.x-r1173425 branch:
* r1173425, r1173639, r1174797
<SNIP>
]]]
To "reverse-merge the original changes from the source branch" we'd
need this 2-URL merge:
C:\SVN\src-branch-1.7.x>svn merge
^/subversion/branches/1.7.x-r1173425_at_1176447
^/subversion/branches/1.7.x_at_1173428 .
(Where r1176447 is the last change to the 1.7.x-r1173425 branch before
it was deleted and r1173428 is the rev for 1.7.x that the
1.7.x-r1173425 branch was copied from)
While we can get the same result from simply doing this:
C:\SVN\src-branch-1.7.x>svn merge ^/subversion/branches/1.7.x . -c-1176459
> At least that gives the corresponding mergeinfo update.
> If you cherry-pick from a dev branch to its trunk, that’s valid
> for an original change but not if the change that you’re picking is
> itself a merge (at least if it’s a merge from trunk). We can detect
> this by seeing whether that change includes a mergeinfo diff.
>
> Common misconceptions
>
> That merging from A to B will make B look like A.
> A client said “I merged A to B, then modified B, and now need
> to revert the change on B; maybe I could do that by merging A to B
> again”.
Is 'A' a revision, a branch or both? “I merged A to B" makes it sound
like a revision (or revisions), but then it clearly is referring to a
branch.
> In that case, I think the merge from A to B was a
> “reintegrate” which does indeed pretty much make B look like A.
> (Source: WD support ticket #3013.)
Probably pointless nitpick, but is this really a "common" misconception?
> To help avoid this, ‘merge’ should display messages about what
> it’s doing, using terminology that makes clear that changes are being
> merged rather than state.
>
> Reporting -- What “svn merge” Should Say
> “svn merge” should first of all give a summary of what it is about to
> do, using words that make sense to the user. Consider stopping to ask
> for confirmation (unless --no-interactive). Some of the output should
> be a summary of what is calculated as needing to be merged. That is
> not only a chance to check the user’s expectation, but in a merge that
> involves significant calculating of source revisions, it is also a
> chance for the user to see that Subversion has been doing something
> useful up to this point
A hearty (and somewhat self-serving) +1
> and is about to move on to the next stage of
> actually merging those revisions.
> So for example:
>
> $ svn co $URL/branches/dev1 wc1
>
> $ cd wc1
>
> $ svn merge --reintegrate ^/branches/feature2
>
> Reintegrating ‘^/branches/feature2’ into working copy of ‘^/branches/dev1’
> and then a summary of the current merging status:
If a user is unaware of what working copy they are operating on, then
this notification is unlikely to help them. I don't see much harm in
it alone, but as we pile on more notifications (below) I think we run
the risk of overloading the merge output (more than it is already!) to
the point where users reflexively ignore it.
> Previous merges from ‘^/branches/feature2’ into ‘^/branches/dev1’:
> 4 source changes (latest: r144) in 2 merges (latest: r145)
I think we are crossing into "too much information" territory if 'svn
merge' starts telling what merges have *previously* been committed.
Isn't that the domain of 'svn mergeinfo'? See my above concern about
notification overload.
If I'm in the minority on this view, I'd suggest that these
notifications either appear after the merge is complete or be repeated
again at the end; so as to be more noticeable.
> and any warnings:
> warning: working copy contains modifications (all in ‘docs/’)
> warning: working copy is not up to date
> warning: ‘^/branches/feature2’ is not up to date with ‘^/branches/dev1’:
Doh, can't believe we've never thought of this before, but recall how
'svn switch' now disallows switching to an unrelated branch:
C:\SVN\src-branch-1.7.x>svn sw
https://svn.apache.org/repos/asf/subversion/site .
..\..\..\subversion\svn\switch-cmd.c:168: (apr_err=195012)
svn: E195012: Path '.' does not share common version control ancestry
with the requested switch location. Use --ignore-ancestry to disable
this check.
..\..\..\subversion\libsvn_client\switch.c:214: (apr_err=195012)
svn: E195012: 'https://svn.apache.org/repos/asf/subversion/site'
shares no common ancestry with 'C:/SVN/src-branch-1.7.x'
We probably want something analogous for 'svn merge', which happily
allows me to do something nonsensical like this:
C:\SVN\src-branch-1.7.x>svn merge
https://svn.apache.org/repos/asf/subversion/site . -c1177291
--- Merging r1177291 into '.':
C publish
--- Recording mergeinfo for merge of r1177291 into '.':
U .
Summary of conflicts:
Tree conflicts: 1
> 2 changes (latest: r166) have not yet been merged
>
> Run ‘svn mergeinfo’ for details.
>
> Reporting -- What “svn mergeinfo” Should Say
> In order for a user to understand what merges they have done and need
> to do, it would be most helpful if Subversion could tell them the
> current state of a branch with regard to merges. The existing “svn
> mergeinfo” command is near useless for such understanding: it just
> outputs a list of revnums without saying what that means in terms of
> whether the target is fully caught up or not; it doesn’t report
> anything meaningful about subtree merges and doesn’t even notice them
> by default; it doesn’t check that you specified a sensible source
> branch and simply says nothing if you accidentally specified the
> target branch.
> In a WC of feature branch “B”, which has had catch-ups through
> trunk_at_1234 except trunk_at_1215:
>
> Mergeinfo summary:
> $ svn mergeinfo ^/trunk
> Origin: ^/branches/B_at_1200 (from ^/trunk_at_1190)
> Merged from ^/trunk:
> Origin-1214, 1216-1234
Keep in mind that for long-lived release branches this information is
never going to be very tidy -- the desire for tidy (i.e. human
readable) output for svn mergeinfo being a common theme elsewhere in
this thread. Consider our 1.6.x branch, how would we show this:
svn mergeinfo --show-revs=merged ^/subversion/trunk
^/subversion/branches/1.6.x
Here's the mergeinfo representation of what has been merged from from
trunk to 1.6.x, it's not much more concise than the output of the
above command because there are so few spanning ranges:
/subversion/trunk:875965,875968,876004,876012,876017,876019,876022,876024,876032,876041-876042,876048,876051,876055-876056,876059,876083,876091,876097,876101,876104,876109,876123-876125,876129,876132,876138,876160,876167,876175,876180,876185,876205,876223-876225,876230,876233,876245,876252,876256,876283,876287,876312,876326-876327,876330,876366,876372,876374,876376,876383,876386,876442,876456-876457,876462-876464,876467,876469,876480,876486,876495-876497,876516-876518,876524,876526,876583,876601,876614-876615,876628,876633,876641,876645,876659,876687,876689,876705,876715,876726,876760,876763,876794,876804,876815-876816,876821,876825,876837,876840-876841,876843,876849,876857-876858,876862,876873,876890,876897,876905,876908,876925,876931,876934,876948-876949,876953,876987,876993,877011,877014,877016,877028-877029,877038,877119,877127,877146,877157,877191,877195,877203,877211,877230,877234,877237,877243,877249,877259,877261,877304,877319,877407,877437,877441-877442,877453,877459,877472,877544,877553,877565,8775
68,877573,877593,877595,877597,877601,877612,877665,877667,877681,877692,877696,877701,877720,877730,877784,877793,877797,877809,877814-877815,877819,877821,877842,877848,877853,877867,877869,877873,877901,877909,877916,877931,877942,877953,877964,877968,877970,877981-877982,878005,878013,878015,878020,878046,878053,878062,878074,878080,878089,878091,878093,878095,878127,878129,878131,878142,878173-878176,878216,878240,878242,878255,878269,878272,878279,878296-878297,878303,878321,878335,878338,878341,878343,878353,878364,878367-878368,878385,878399,878423,878426,878447,878462,878484,878491,878498,878532,878590,878595,878607,878625-878627,878646,878659,878673,878682-878683,878690-878691,878693,878723,878760-878761,878873,878875,878877,878879,878905,878910-878911,878915-878916,878924-878925,878946,878949,878955,878960,878970,878981,879001,879033,879056,879074,879076,879081-879082,879093,879105,879126,879148,879170,879198-879199,879201,879271,879293,879357,879375-879376,879403,879631,879635-879636,879688,87970
9-879711,879747,879902,879916,879954,879961,879966,879971,880027,880082,880095,880105,880146,880162,880226,880274-880275,880370,880450,880461,880474,880525-880526,880552,881905,884842,886164,886197,888715,888979,889081,889840,891672,892050,892085,895514,895653,896522,896915,898048,898963,899826,899828,900797,901304,901752,902093,902467,904301,904394,904594,905303,905326,906256,906305,906587,907644,908980-908981,917523,917640,918211,922516,923389,923391,926151,926167,927323,927328,931209,931211,931392,931568,932942,933299,934599,934603,935631,935992,935996,937610,939000,939002,939375-939376,944635,945350,946355,946767,947006,948512,948916,949307,950931,950933,951753,952992,953317,955369,957507,958024,959004,959760,961055,961970,962377-962378,964167,964349,964767,965405,965469,965508,979045,979429,980811,981449,981921,984565,984928,984931,991534,992114,996383,996884,997026,997070,997457,997466,997471,997474,1000038,1000060,1000607,1000612,1001009,1002094,1005446,1022675,1024269,1027957,1028084,1028108,1028125,
1031165,1031186,1032808,1033166,1033290,1033665,1033685,1033921,1034557,1035745,1036429,1036534,1036978,1037762,1038792,1039040,1041438,1041638,1051632,1051638,1051733,1051744-1051745,1051751,1051761,1051763,1051775,1051778,1051968,1051978,1051988,1052029,1052041,1052068,1053185,1053208,1053233,1053499,1053984,1058269,1058722,1063572-1063573,1063592,1063870,1063946,1064839,1066249,1066270,1066276,1068988,1070912,1071239,1071307,1072084,1072953,1074572,1076730,1076759,1076826,1078954,1081255,1084575,1084581,1084764,1084962,1084978,1086222,1094692,1095654,1098608,1102803,1103665,1104309,1125983,1125998,1126007,1126810,1130303,1130448,1134734,1144316,1146870,1151036
> Automatic identification of the parent branch:
> $ svn mergeinfo
> Parent branch is ^/trunk.
> [... continue as for “svn mergeinfo ^/trunk” …]
> Failed to identify a parent branch:
> $ svn mergeinfo
> svn: No parent branch configured for ^/branches/B
> When no relationship is configured between this and the specified branch:
> $ svn mergeinfo ^/branches/B2
> No declared relationship between ^/branches/B and ^/branches/B2.
> Merged from ^/branches/B2:
> 905,970-977
>
>
> Mergeinfo should be “recursive” by default, reporting any sub-tree
> differences, where presently it does not.
One possible point in favor of keeping 'svn mergeinfo' non-recursive
by default: That recursion takes time, possibly a lot of time if you
have 1000's of subtrees with explicit mergeinfo. But that's pretty
minor since 'svn merge' operates at depth=infinity by default, so 'svn
mergeinfo' probably should too...so I think that means I agree!
> How should the output look in the presence of subtree merges?
> $ svn mergeinfo
>
> Mergeinfo Diff (bug)
> Diff, when displaying an added mergeinfo property on a sub-tree,
> should diff against the previously inherited mergeinfo, not say that
> all this new mergeinfo represents merges that have just been done.
> Similarly for a deleted prop.
Might be worth putting a link to the already lengthy discussion on
this topic: http://svn.haxx.se/dev/archive-2011-09/0201.shtml
> Terminology - A Typical Branching and Merging Policy
> A simple and useful branching policy. This is presented here just in
> order to give a framework of terminology for describing guidance
> measures for merging, not as a restriction on what is to be supported.
> Assume a partial ordering among branches, such that any given branch
> has (0 or more) Feature Branches that are less stable than it, and
> Release Branches that are more stable than it, and it is considered
> the parent of each of those. A merge may be performed between a
> parent branch and one of its immediate Feature or Release branches,
> and nowhere else.
>
> A merge to a Feature Branch from its parent is normally a catch-up.
The term 'sync' merge is (more?) frequently used to describe this type
of merge too.
> A merge from a Feature Branch to its parent is normally a reintegrate.
> A merge to a Release Branch from its parent is normally a cherry-pick.
The term 'Cherry-pick' has typically referred to a merge where an
explicitly defined set of revisions is merged (rather than taking all
eligible revisions). See
https://svn.apache.org/repos/asf/subversion/trunk/notes/merge-tracking/requirements.html#cherry-picking
Of course this doesn't contradict what you said, most merges to a
release branch *are* cherry picks, but we should define 'cherry-pick'.
> A merge from a Release Branch to its parent could be a catch-up or
> a cherry-pick. (### Does a catch-up work properly if you’ve done
> cherry-picks in the other direction? Might not.)
I'm not sure I see the use case where we'd want to do a sync/catch-up
merge from a release branch (as you've defined it) back to the parent.
Does it work properly? I'm not sure what you'd even be trying to
accomplish here, so I can't say. Think of this:
svn merge ^/subversion/branches/1.7.x trunk-WC --accept=postpone
This is going to merge all the changes on 1.7.x since it was copied
from trunk and HEAD back to trunk.
> A merge to or from a Release Branch may be forbidden in one direction,
> according to local policy.
Are you getting at the fact that we may want to discourage merges from
release branches back to the parent? Or something else?
]]]
> This is very much in flux with a mixture of simple and more radical
> thoughts, but the direction I'm looking at is adding some relatively
> simple but high level "intelligence" on top of the existing merge
> subsystem to guide users better into understanding what they're doing.
>
> Thoughts welcome.
>
> - Julian
Received on 2011-09-30 18:46:35 CEST