Philip Martin <philip.mar...@wandisco.com> writes: > Multiple clients. It arises from my idea to solve issue 3546 > > http://subversion.tigris.org/issues/show_bug.cgi?id=3546 > > In svn_repos_fs_change_rev_prop3 the code first gets the old property > value which it uses to calculate the action: 'A', 'M' or 'D'. Then it > passes the action to the pre-revprop-change hook, then it changes the > property and finally it runs the post-revprop-change hook. Some other > process can change the revprop at any time so although the > pre-revprop-change hook might get passed an 'A' say, when the change > is made it could be effectively an 'M'. The action passed to the hook > is not a reliable indication of the change to be made. > > Putting the get, pre hook and set into a transaction would allow the > action in the hook to accurately reflect the change made (or not made > if the transaction fails).
We don't need a new transaction to fix this, we can rev the svn_fs_change_rev_prop interface instead: svn_error_t * svn_fs_change_rev_prop(svn_fs_t *fs, svn_revnum_t rev, const char *name, const svn_string_t *value, apr_pool_t *pool); to include the current value of the revprop, and then reject the change if the current value does not match. That should be simple because the FSFS implementation already takes a write lock and the BDB implementation already uses a transaction. Then the repos layer can loop (in practice only if the use_pre_revprop_change_hook flag is set): do svn_fs_revision_prop(¤t_value) action = ... svn_repos__hooks_pre_revprop_change(action) error = svn_fs_change_rev_prop2(current_value, new_value) while error is current value doesn't match This doesn't alter the fact that the revprop can change at any time during the loop but that doesn't matter. The revprop is unversioned so only the current state matters and the above will guarantee that the current state when the change is made is equal to the state validated by the pre-revprop-change hook. -- Philip