Re: Merge policies

Stefan Fuhrmann wrote:

> The following pre-commit scripts / policies would be useful.
>
> * Common parts [not a policy]
>   We first check whether the commit contains a changed
>   svn:merge-info property. This limits the performance
>   impact on non-merge commits and we need to identify
>   all changed svn:merge-info anyway.
>
>   Also, the merges that happened on the source branch
>   from a different location than the target branch are
>   of no interest for the policy checkers. E.g.:
>
>   r20: merge r19 from ^/sub-branch to ^/branch
>   txn: merge r10-20 from ^/branch to ^/trunk
>   Both merges will show up in the merge-info delta but
>   we only need to evaluate the second one.

It took me a few minutes to realize that you mean the hook is analyzing a commit to ^/trunk and is seeing mergeinfo additions that indicate both a merge from ^/sub-branch and a merge from ^/branch.  And in this example the merge from ^/sub-branch was in fact a merge to ^/branch, which was committed in r20, and which is now propagating to ^/trunk.

In order for the hook to figure that out, starting from just looking at the mergeinfo change (which doesn't say what the target of each merge was), it has to examine revisions 10-20 of ^branch and notice that one of those revisions (r20, in fact) was the other merge that it's considering.

That's OK, it can do that; I'm just clarifying how it can figure out which merges "happened on the source branch".

I'll assume the admin can configure each of these policies to apply differently on different branches.  That's mostly just a matter of inventing a suitable configuration scheme, presumably with branch name pattern matching or similar, and that would be totally necessary in a large project.

> * Strict merge hierarchy
>   A merge from A->B is only allowed, if the copy-from
>   of A is B or vice versa and the copy source has not
>   been replaced since the copy). This prevents circular
>   merges and others (note 1).

Agreed, that policy would often be useful, even in that simple form ...

>   In a more sophisticated implementation, we could identify /
>   allow for renamed branches as well as A and B having
>   the same relative path to some parents that form a
>   direct branch (i.e. allow sub-tree merges).

... and very widely useful with those additions.

> * No sub-tree merges
>   Like the above but without the check for parents.

Agreed: that policy would often be useful.

> * No aggregate merges
>   There must only be one source branch, i.e. we can't
>   merge from branches A and B to C in the same revision.

Agreed: that policy would often be useful.

> * No distributive merges
>   For each path being merged (i.e. having a merge-info
>   delta), the relative paths in source and target must
>   correspond (i.e. start as the same and then may get
>   renamed etc.). This is basically the same as the
>   "sophisticated" part of the check for strict merges.

In other words: in any subtree merge (that is, when a subtree of the branch root is being merged separately or differently from its parent or the branch root), the 'source-left' subtree and the 'target' subtree must be traceable back to a common ancestor.  In the absence of any renames or replacements inside either branch, this would mean that the two subtrees have the same relpath within each respective branch.

What we're rejecting here is any attempt such as:

  mkdir trunk/subtree1 trunk/subtree2
  copy  trunk branch
  merge "trunk/subtree1" into "branch/subtree2"

Agreed, that policy is both almost universally useful, and is required from a functional point of view when we're doing "automatic" merges because the merge code needs to be able to trace them toward a common ancestor.

So I think such a merge would be rejected by the current (1.7) automatic-merge code already.  (Haven't tried.)

Note: We also support a non-automatic merge, called a "2-URL" merge, where merge tracking is not attempted, and that (as I understand it) may allow such a merge.

> * No cherry picking
>   Check that the source branch does not contain revisions
>   that lie before the last to-be-merged revision but
>   have neither been merged before nor are being merged
>   right now.

Agreed: that's sometimes useful. Maybe not so often as most of the other suggestions.

> * No criss-crossing
>   Prevent situations like the criss-cross examples here:
>   http://wiki.apache.org/subversion/SymmetricMerge
>
>   For a merge A->B, abort if there has been a merge
>   B->A after the last revision of A to be merged to B.

Agreed.  I believe getting into a criss-cross situation would very rarely be intentional and would in general make the next merge more difficult than it would otherwise be.

>   This only valid for non-cherry-picking merges and
>   only if the change sets of both merges overlap.

Yes; I haven't fully grokked the detail, but I agree there's no need to stop cherry-picks crossing over.

> Except for the last one those checks will simply verify
> that the user followed certain policies. They should,
> therefore, rarely reject a commit.

Why do you suggest "no criss-crossing" is an exception?  It seems also to be just verifying that the user followed certain policies.

> Again, the user shall be free to combine (or not use)
> these policies although not all combinations are meaningful.
>
> Thoughts?
>
> -- Stefan^2.
>
>
> Note 1:
>
>   One thing that we might want to support is integration
>   branches where a temporary branch is being used as
>   an intermediate merge target:
>
>   integrate A->B as
>   rN: copy B->A_integration
>   rN+1: merge A->A_integration
>   rN+x: ... various changes on A_integration
>   rN+y: merge A_integration->B
>   rN+y+1: delete A_integration

That's a classic case of merging among three branches.  After doing that integration, the user will then expect to be able to merge A->B and B->A, and later do another similar integration via a new branch, B -> A_integration_2 -> A.

The only way in which I can see this scenario can be simpler than the general case, is that it would be reasonable to disallow A->B and B->A merges during the time while B->integ->A is happening.

>   These checks become more complicated, requires
>   naming conventions for the integration branches etc.

- Julian
Received on 2012-04-19 16:02:38 CEST