On 29.07.2014 13:34, Stefan Fuhrmann wrote:
> On Mon, Jul 28, 2014 at 7:38 PM, Branko Čibej <brane_at_wandisco.com> wrote:
>> On 28.07.2014 19:19, stefan2_at_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.
> but about iteration, I see only two basic options:
>
> 1. Invent a new data structure that duplicates 80+% of svn_config_t
> to provide the same functionality but different ordering through
> its 'enumerate sections' interface. This basically would be r1614080
> minus a few bits that we happen not to call in the authz context.
>
> 2. Reapply r1614080 but let 'enumerate sections' return data in
> hash order as on /trunk. Bump the 'enumerate sections' API
> to include a "use declaration order' option.
3. Feed the authz in-memory representation directly from the config
parser, without using an intermediate svn_config_t. This is what we
should do. The authz structure is inherently different from a config
structure, because it has different semantics and access patterns.
> IMO, number 2 is the better choice as it gives full control over the
> added functionality without adding significant maintenance costs.
You're already creating an in-memory structure that is significantly
different from svn_config_t, and does not require the presence of an
svn_config_t, nor does it have to enumerate anything, since it can be
constructed incrementally and in-order during parsing.
[...]
>> Are you saying that a wildcard path should match before a concrete path when
>> it happens to appear later in the authz file? Perish the thought. Exact
>> matches should always override fuzzy matches, anything else is not intuitive
>> at all and we'll receive a ton of spurious bug reports about how authz files
>> don't work.
> No, that is not what I'm saying. My rules are simple:
Let's please have the rules documented in the wiki, then we can comment
on them.
> * Select the path rule(s) that cover the largest section of the path.
> This is what we do today and there can be only one such match
> w/o the use of wildcards.
>
> * If there is more than one such rule, apply the one declared last.
> This seems to be the easiest rule to follow and understand.
Well, I disagree, I'd expect an exact match to override a wildcard match.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane_at_wandisco.com
Received on 2014-07-29 13:53:15 CEST