subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko Čibej <br...@wandisco.com>
Subject Re: svn commit: r1614080 - in /subversion/branches/authzperf: BRANCH-README subversion/libsvn_repos/authz.c subversion/libsvn_subr/config.c subversion/libsvn_subr/config_impl.h
Date Tue, 29 Jul 2014 15:55:04 GMT
On 29.07.2014 15:55, Stefan Fuhrmann wrote:
>
> On Tue, Jul 29, 2014 at 3:28 PM, Branko Čibej <brane@wandisco.com
> <mailto:brane@wandisco.com>> wrote:
>
>     On 29.07.2014 14:55, Stefan Fuhrmann wrote:
>>     On Tue, Jul 29, 2014 at 1:52 PM, Branko Čibej <brane@wandisco.com
>>     <mailto:brane@wandisco.com>> wrote:
>>
>>         On 29.07.2014 13:34, Stefan Fuhrmann wrote:
>>>         On Mon, Jul 28, 2014 at 7:38 PM, Branko Čibej <brane@wandisco.com>
<mailto:brane@wandisco.com> wrote:
>>>>         On 28.07.2014 19:19, stefan2@apache.org <mailto:stefan2@apache.org>
wrote:
>>>>>         Author: stefan2
>>>>>         Date: Mon Jul 28 17:19:47 2014
>>>>>         New Revision: 1614080
>>>>>
>>>>>         URL: http://svn.apache.org/r1614080
>>>>>         Log:
>>>>>         On the authzperf branch: Implement the notion of path rule ordering
>>>>>         by making svn_config_t iterate through sections in declaration
order.
>>>>>         This is done using a simple linked list because we can't remove
>>>>>         sections but only add them.
>>>>         No no no no!
>>>         What exactly did my change break?
>>>
>>>         Right now, the svn_config interface returns the sections in random order.
>>>         I changed the code such that this random order happens to be the same
>>>         as the (static and dynamic) declaration order.
>>>
>>>>         Changing the svn_config_t structure is not the right to do this.
The correct
>>>>         approach here is to parametrise the svn_config parser with a constructor
>>>>         method, then create a new constructor for the authz parser which
will then
>>>>         get all entries in the file order. Please revert these svn_config
changes.
>>>         I reverted the changes for now as I agree that the new behaviour
>>>         should be somewhat more explicit.
>>>
>>>         However, since this is not about construction (the result would
>>>         still be a hash)
>>
>>         You are making way too many assumptions about that. See my
>>         svn_config parser parametrization in r1614213
>>
> [...]
>
>>
>>>     The second point is more severe, though, as it prevents a nicer
>>>     intermediate
>>>     model than what svn_config_t already provides. The following is
>>>     a legal authz
>>>     file:
>>
>     I'm going to skip commenting on this because I don't see how it's
>     relevant.
>
>  
> The point is that you can't interpret any of the stream
> beyond the key/value model already implemented by
> svn_config_t until you have read the whole stream.

That's true for config files, but not necessarily for authz files.


> So, while you can without doubt parse the file streamily,
> you cannot begin to build a data model with authz
> semantics from it streamily. We can go down the path
> of inventing some intermediate model that performs
> slightly better than the current svn_config_t but that
> is a lot of coding simply to avoid a perfectly reasonable
> API bump.

Actually, it's not as perfectly reasonable as all that. Consider:

[foo]
x = 1

[bar]
y = 2

[foo]
z = 3

So ... in what order should these sections be enumerated?

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com

Mime
View raw message