Hi Daniel. Thanks for the feedback and encouragement.
Daniel Shahaf wrote: > At a high level, agreed that explicitly exposing more details of > reintegrate's semantics would be a good thing. > > However, I'm not convinced that having three separate APIs as you > suggest is a good idea. Are they generally reusable, or do they only > make sense when called in sequence, one after the other? > > (In the latter case they should get namespaced appropriately, or made > into callback functions that the public reintegrate API calls.) I want them to be reusable, and I expect that there should be opportunities for re-use sooner or later. For example, the client should be able to use ensure_wc_is_suitable_...() before other kinds of merge, and even in other non-merge contexts. The second stage, find_reintegrate_merge() obviously has a purpose that is specific to a reintegrate merge, but still it should as far as possible return results that have some meaning to the caller rather than an opaque set of outputs that are only suitable for feeding into do_reintegrate_merge(). And I'll investigate now whether and how the third stage can call the existing two-URL merge API instead of introducing a reintegrate-specific API function. If the current patch falls short of these ideals I want to improve it. > Julian Foad wrote: >> A comment about taking out a WC write lock. >> [...] > 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. OK, thanks for that perspective. > 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? Note that because the 'svn' client tends to only do one simple operation per execution, the issue doesn't arise much. It would be interesting to know whether and how other clients attempt to solve the issue, in a separate thread. - Julian
