[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1767197 - in /subversion/trunk/subversion: include/private/svn_log.h include/svn_ra_svn.h libsvn_ra_svn/client.c libsvn_ra_svn/protocol libsvn_subr/log.c svnserve/serve.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sat, 5 Nov 2016 16:41:08 +0000

Stefan Fuhrmann wrote on Sat, Nov 05, 2016 at 13:52:49 +0100:
> On 31.10.2016 01:45, Daniel Shahaf wrote:
> >stefan2_at_apache.org wrote on Sun, Oct 30, 2016 at 23:43:07 -0000:
> >>+ dirent: ( rel-path:string kind:node-kind ? ( size:number
> >>+ has-props:bool created-rev:number
> >>+ [ created-date:string ] [ last-author:string ] ) )
> >>+ | done
> >
> >Why should size,has-props,created-rev be all present or all absent?
> >Wouldn't it be better to let each element (other than the first two) be
> >optional, i.e.,
> >
> > dirent: ( rel-path:string kind:node-kind
> > ? [ size:number ]
> > [ has-props:bool ]
> > [ created-rev:number ]
> > [ created-date:string ]
> > [ last-author:string ] )
> >
> >? This decouples the protocol from the backend (specifically, from what
> >bits the backend has cheaply available).
>
> Yes, that would be better.
>
> It would require to either make has_props a tristate
> or extending the protocol to allow for optional bools.
>
> I think we should do the latter because regardless of
> whether we have them at the protocol level or not, we
> must specify a value in the output data struct at the
> receiver side. So, we might as well define it to FALSE
> for n/a data.

I don't quite follow, sorry. Let me just clarify my idea:

We'll use the spec I gave above, with «[ foo:bool ]», which is an
optional bool. At the protocol level, that suffices; it is equivalent
to a non-optional «foo:tristate», but without needing to add
a "tristate" type to the wire protocol. At the API level, we'd provide
two parsing options: '3' to parse the optional bool into an
svn_tristate_t, and 'B' to parse the optional bool into an
svn_boolean_t, converting svn_tristate_unknown to FALSE. Using 'b' to
parse an optional error should be a fatal error (an SVN_ERR_ASSERT).

My point is that we don't need the wire protocol to have a notion of "a
missing bool is interpreted as FALSE"; we can leave that decision with
each individual call to the parser function.

Makes sense? Could you clarify your idea as well?

> This does not effect any existing protocol usage or
> code as the format patterns of the existing commands
> do not change. They are represented as "words" in
> the pre-parsed "item" structure, so even old code
> receiving an optional boolean would parse them fine -
> in case we were to add optional booleans to existing
> command responses in the future.
>

I'm not sure what you're trying to say. Of course old code will cope
just fine with optional booleans; from that code's perspective, the
optional booleans will occur "past the end of a tuple", so it will
ignore them entirely. The old code just needs to be able to tokenize
optional bools, so it can skip past them. That's how the protocol is
designed.

> And while we are at it, we should make vwrite_tuple()
> actually support optional tristates and numbers.

What do you mean by "optional tristate"? It sounds like you're talking
about a «[ foo:tristate ]», which doesn't appear in the current protocol.
If the question is, how would one call vwrite_tuple() to generate
a tuple specified as «[ foo:bool ]», then I think there are three ways:

* vwrite_tuple("()")
* vwrite_tuple("(b)", (svn_boolean_t)foo)
* vwrite_tuple("3", (svn_tristate_t)bar)

where the third option generates an empty tuple for svn_tristate_unknown
and a single-element tuple for svn_tristate_false and svn_tristate_true.

Cheers,

Daniel
Received on 2016-11-05 17:41:22 CET

This is an archived mail posted to the Subversion Dev mailing list.