Philip Martin wrote on Wed, Nov 27, 2013 at 22:21:00 +0000:
> Daniel Shahaf <d...@daniel.shahaf.name> writes:
>
> > Philip Martin wrote on Wed, Nov 27, 2013 at 18:57:13 +0000:
> >
> >> We already have the code to transfer a revision property over the
> >> network and set it on a transaction (the client traps attempts to set
> >> svn:date but otherwise it works). Currently attempts to set svn:date
> >> are silently ignored by the server. This would remain the default
> >> behaviour unless the administrator enables the new feature, probably
> >> controlled by a setting in the Apache config file and svnserve.conf.
> >>
> >
> > So we would have a config file setting which allows users to bypass the
> > pre-revprop-change hook?
>
> I wasn't intending to invoke the pre-revprop-change hook. Other
> revision properties set as part of a commit do not invoke the hook.
>
Fair point.
> > Perhaps the "temporary property" (the property
> > which tells the FS layer not to overwrite svn:date) should instead be
> > set by the pre-commit hook?
> >
> >That would be:
> >
> > 1. When user sets svn:date on a commit, the RA layer just sets svn:date
> > in the props hash, like is done with every other property.
> >
> > 2. In the FS, where today svn:date is being overridden, only do that if
> > the "temporary property" you mentioned is set. The temporary property
> > is always removed during commit, just like SVN_FS__PROP_TXN_CHECK_LOCKS.
> >
> > 3. The temporary property must be impossible to over the wire.
>
> Not sure I follow that. The client changes svn:date before the
> pre-commit hook but the pre-commit cannot currently distingush the
> initial svn:date set at txn create from an svn:date set by the client.
> We need some indication that the client has explicitly set svn:date,
> including deleting it to indicate a revision with no date.
>
That indication could be the client's authenticated username being "svnsync".
> Also we can't assume that the server receives the requested svn:date at
> the time the transaction is created. svnserve has that restriction but
> for mod_dav_svn the client can set svn:date by sending a PROPPATCH at
> any time during the commit.
>
Why is that a problem? By the time the pre-commit hook is invoked,
svn:date will have the right svn:date value, right? And so when the txn
becomes a revision, it will be "born" with the right svn:date vaue.
> So the txn gets created with a date that is not wanted. At some point
> later the date may get changed to a value that is wanted. When the
> commit is finalised we have to use the txn date only if it has been
> changed, otherwise we use 'now'. Either we mark the txn at the start
> and remove the mark when the date is set, or we start with no mark and
> add it when the date is set. Either way we have to remove the mark if
> it is present when finalising the commit. The pre-commit hook can look
> for the presence/absence of the mark to determine whether this
> particular txn is setting the date.
>
The pre-commit hook can *set* the mark itself, for example:
if [ x"$AUTHOR" = x"svnsync-user" ]; then
svnadmin setrevprop svn:fs-commit-dont-overwrite-date-property yes -t $TXN
fi
With this approach, non-svnsync commits (to other repositories) can't
backdate themselves. I'm not sure whether that's also true for your
approach, it would depend on the details of the config knobs.
> I suppose we could put restrictions on the client: if it wants to set
> svn:date it must be sent at txn create, later svn:date changes are
> ignored, etc. That might lead to a marginally simpler implementation
> but I wasn't intending to restrict it like that.
>
Neither did I.
