Re: revprop changes and hooks

Philip Martin <philip.martin_at_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(&current_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