Philip Martin wrote on Wed, Mar 19, 2014 at 13:17:08 +0000:
> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
>
> > I would vote "+1 to backport" right here and now, but I'm a bit confused
> > by the patch: it seems the client and server send and expect a bare
> > boolean, whereas libsvn_ra_svn/protocol calls for a boolean wrapped in a
> > tuple.
> >
> > (i.e., '( foo bar true ) ' vs '( foo bar ( true ) ) '.
>
> It's ugly. I believe the want-iprops flag was added to the get-dir and
> get-file commands before the get-iprops command was added. Our client
> now uses get-iprops excusively and never sends want-iprops with get-dir
> or get-file. I suspect want-iprops should not have been released.
>
> We have:
>
> - the documented protocol
>
> (? want-iprops:boolean )
>
> - the released server implementation of the protocol
>
> ? want-iprops:boolean
>
> - the released behaviour
>
> properties always sent
>
> - the trunk behaviour
>
> properties only sent when want-iprops is true
>
> A third party client could already be using want-iprops with a 1.8
> server to get inherited props, even with the broken behaviour.
Not our problem. We have no obligation to support third parties who
reimplement our protocol. We also have no obligation to make the server
accept constructions that no released client ever generated.
If I understand correctly, you're saying that no *released* client[1]
ever sent a want-iprops boolean on get-file and get-dir; so we have zero
obligation to support a want-iprops parameter there, period. (To clarify,
we have no obligation to support either "?B" or "(?B)" or "?(?B)".)
We may choose to support the form that the 1.8.0-1.8.9 server code
accepts (that is, "?B"), or we may choose to declare that a bug in
1.8.0-1.8.9.
In the former case we may apply the patch.
In the latter case, we shouldn't apply the patch, and moreover we will
have to "seal" get-file and get-dir --- that is, document that no
further optional arguments may be appended to them. (That's not a
problem --- it just means that if we want to add an argument at the end,
we create a new protocol command "get-file2" and "get-dir2", or
condition the new argument on a new server capability. That's to ensure
we never send the extra arguments to a 1.8.0-1.8.9 server (which will
misinterpret them).)
Also in the latter case, we can either remove the "?B" from the server's
code, or keep it --- released clients and third-parties following the
documented protocol should behave the same with or without it, since
they never send a bare "B" there.
In either case, we update libsvn_ra_svn/protocol to document the
divergence, and may choose to issue an API errata (in spite of the
protocol spec nominally being a private API).
> [...]
> but that would mean accepting the "? foo" form rather than "(? foo)" and
> I seem to recall that the later is preferred as it's easier to extend.
Yup. See change-rev-prop and change-rev-prop2 for an example. (In that
particular case, revving the command enabled atomic revprop deletions.)
Daniel
[1] Not including alphas / betas / RCs.
> --
> Philip Martin | Subversion Committer
> WANdisco // *Non-Stop Data*
Received on 2014-03-19 22:00:01 CET