Responses in-line, but first a reminder of why I'm doing this.

It's because I've been looking at making 'svn merge' give better diagnostics 
and confirmations.  Things like issuing an error or warning if the source and 
target are in fact the same 'branch', or if the user is trying to reintegrate 
in the wrong direction (the same direction as some sync merges have been 
done).  To do this, the client needs to ask questions like 'what branches and 
revisions would this merge affect?' before executing the merge.  Similar issues 
apply to all kinds of merges, but I happen to have taken the reintegrate first, 
since it's known to be a wrapper for a two-URL merge.

I expect kind of control this would be especially valuable for GUIs, and I 
would appreciate feedback on this matter.

I don't suggest that every client should be required to execute the steps 
separately.  I think we should keep the 'svn_client_merge_reintegrate' API as a 
shortcut, and not deprecate it.  Opinions welcome.


Daniel Shahaf wrote:

> OK.  I can't find reintegrate_merge_locked(),


Oops, I meant 'merge_reintegrate_locked'.

And:
> Julian Foad wrote:
>>  > 
>>  >     svn_client_find_reintegrate_merge(&url1, &rev1, &url2, &rev2, ...);
>>  >     svn_client_merge4(url1, rev1, url2, rev2, ...);
>> 
>>  One issue is that there is some duplication of work in this
>>  formulation.  Both of these functions check that the target WC is
>>  suitable for merging into.  Both open up RA sessions.  Both determine
>>  the youngest common ancestor.
> 
> I don't think "opening an RA session" is a problem for a function 
> that's about to do a merge operation :)

Yes, even for a small merge the cost of that can't be of much importance.

> Checking the WC's sanity, presumably that's necessary since the WC may
> be modified after svn_client_find_reintegrate_merge() but before
> svn_client_merge4()?  Especially if the user is prompted to +1 the
> output of svn_client_find_reintegrate_merge() before the actual merge
> is done.  (and then goes to lunch, comes back, makes some commits, finds
> a loose TSVN dialog and OKs it)

Good thoughts.  The current situation in this patch (v2) is that step 1 
(_find_reintegrate_merge) checks the WC is:

  * single-revision
  * unmodified
  * unswitched

and step 2 (_merge4) checks the WC is:

  * single-revision

because that's all that a normal (non-reintegrate) invocation of 
svn_client_merge4() needs.

If step 2 could repeat the same check as step 1, that would be useful for your 
scenario, but otherwise there's little point in step 2 doing a partial check.  
So for now I'll take the simple solution which is to call 
svn_client_merge4(allow_mixed_rev=TRUE) to bypass the check.

If and when we want to perform the full check in step 2, then we'll want to 
alter svn_client_merge4() in some way to provide more control over those 
checks, or else expose the check function separately.  The best way might 
become clearer with further work on the APIs.

- Julian

Reply via email to