Julian Foad wrote on Thu, Dec 08, 2011 at 12:28:23 +0000:
> 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.
>
In short: calling the non-public API SVN_WC__CALL_WITH_WRITE_LOCK() is
fine because it improves upon a bad situation where we wouldn't be
taking a lock at all.
I don't argue with that, but how do 3rd party clients solve this problem
today? Do they also ignore the need to grab a write-lock, like you say
the rest of subversion/svn/ does today?
> - 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:41:36 CET