A comment about taking out a WC write lock.
This implementation preserves the current behaviour of taking out a WC write lock aroung the whole reintegrate merge procedure, and does so by using SVN_WC__CALL_WITH_WRITE_LOCK within 'svn'. That's a bit of a layering violation; if we want 'svn' to take out such a lock then we should provide a libsvn_client function or macro for it to use. But although ideally we would like to ensure that no other process starts writing to this WC (or this WC subtree), there are other places within 'svn' where we ought to care too and I don't believe we have ever cared enough to do anything about it yet. If we're going to deal with this issue at all we ought to deal with it throughout 'svn'. From that perspective, it's a side issue.
- Julian
----- Original Message -----
> From: Julian Foad <julianfoad_at_btopenworld.com>
> To: "dev_at_subversion.apache.org" <dev_at_subversion.apache.org>
> Cc:
> Sent: Thursday, 8 December 2011, 12:17
> Subject: [PATCH] Split up the reintegrate merge API: first find what to do, then do it
>
>T he current 'reintegrate' merge consists conceptually of two stages:
>
> * Determine what the equivalent two-URL merge is.
>
> * Do the two-URL merge.
>
> as well as checking that the target WC is clean and the source and target are
> ancestrally related.
>
>
> My gut feeling tells me that the libsvn_client API would be better if it
> presented these as two separate functions. A small but distinctly useful
> benefit is that svn or a GUI client can then tell the user what two-URL merge it
> is doing, which I firmly believe will help users to understand merging and
> therefore to issue the right merge commands more often. That information is
> currently only available inside svn_client_merge_reintegrate().
>
> Modularity would be improved too: if we use the standard two-URL merge API for
> the "do it" stage, then we have fewer different code paths used in
> merging, so less likelihood of unintentional differences (bugs).
>
>
> The reintegrate merge up till now goes like this:
>
> client code (such as subversion/svn/merge-cmd.c)...
> |
> |- svn_client_merge_reintegrate()
> | |
> | |- check target WC is a clean single-revision WC
> | |- check source and target are related
> | |- determine URLs and revs of src, tgt, and common ancestor
> | |- do a 2-URL merge
>
> and with this patch it goes like this:
>
>
> client code (such as subversion/svn/merge-cmd.c)...
> |
> |- svn_client_ensure_wc_is_suitable_merge_target()
> | |
> | |- check target WC is a clean single-revision WC
> |
> |- svn_client_find_reintegrate_merge()
> | |
> | |- check source and target are related
> | |- determine URLs and revs of src, tgt, and common ancestor
> |
> |- Tell the user what equivalent two-URL merge we're doing
> |
>
> |- svn_client_do_reintegrate_merge()
>
> | |- do a 2-URL merge
>
> The last stage is currently exposed as a dedicated 'do_reintegrate'
> function because I'm not yet completely sure whether we want it to be
> semantically exactly the same as an 'ordinary' two-URL merge. If it
> should be semantically identical, then we just need to adjust the code;
> otherwise we may need to modify the two-URL API. That's secondary work, but
> certainly important to ensure that we have a nice clean set of API functions.
>
> Any comments on this approach?
>
> - Julian
>
Received on 2011-12-08 13:29:00 CET