Re: revprop changes and hooks

Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:

> Philip Martin wrote on Tue, 15 Jun 2010 at 11:16 -0000:
>
>> 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.
>>
>
> If we just upgrade the svn_fs_change_rev_prop() call in
> svn_repos_fs_change_rev_prop3() to svn_fs_change_rev_prop2() (a
> single-line change),

Slightly more than one line if we make libsvn_repos absorb the
BAD_PROPERTY and loop.

If we don't loop then if neither of use_pre_revprop_change_hook and
use_post_revprop_change_hook is set then perhaps we should not pass
the old value. That preserves the existing behaviour and the checking
is pointless if there are no hook scripts

> then we will be guaranteed that the 'action'
> parameter (A/D/M) will be accurate. (However, the pre- hook script doesn't
> know the old property value.)

And getting the old value into the script is tricky. It's binary so
it cannot be passed as a parameter directly. It either needs to be
encoded, or passed via a file descriptor (and not stdin since that is
used for the new value).

> This is sufficient for the svnsync use case (issue 3546), where "allow
> adds/deletes only" will provide the necessary mutual exclusion.
>
> But I wonder if, while here, we could go further and obtain the
> "expected old property value" from the RA layer (and pass it to the pre-
> hook). (This probably means revving svn_ra_change_rev_prop() the same
> way svn_fs_change_rev_prop() was revved.) That will allow "svn propset
> k v --if-old-value-is=vprime" to work...

Perhaps. It would require both client and server upgrades and I don't
think there is any way we can provide backward compatibility, so a new
client would have to detect old servers and report some not-supported
error.

-- 
Philip