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

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

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Tue, 29 Jul 2014 15:55:19 +0200

On Tue, Jul 29, 2014 at 3:28 PM, Branko Čibej <brane_at_wandisco.com> wrote:

> On 29.07.2014 14:55, Stefan Fuhrmann wrote:
> On Tue, Jul 29, 2014 at 1:52 PM, Branko Čibej <brane_at_wandisco.com> wrote:
>> On 29.07.2014 13:34, Stefan Fuhrmann wrote:
>> On Mon, Jul 28, 2014 at 7:38 PM, Branko Čibej <brane_at_wandisco.com> <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
> [...]

> 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.

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.

> [...]
> Well, I disagree, I'd expect an exact match to override a wildcard
> match.
> I agree that we could impose such a rule. However, it's semantics
> may not be obvious to everyone because we always apply rules to whole
> subtrees. Consider:
> [/foo/bar]
> user = r
> [/**/bar]
> user =
> ** is a special case because it matches more than one path segment, so in
> this case I would expect the longest match to take precedence.
> I agree though that it complicates matters, not so much in the
> implementation, but in documentation; explaining to users why '*' behaves
> differently than '**' might be harder than explaining why a wildcard
> defined later in the file completely overrides an exact match defined
> earlier.

Well, I agree that we can implement it either way and even
change our mind later on without real extra effort and I
personally don't care too much which behavior we are
going to release eventually.

That said, I still believe that sorting ones rules by path is
good practice and then having the "last match wins" rule
should be obvious and easier to apply by the user. I don't
think that the "pattern path rules are inferior" rule is hard
to understand. My concern is more that there is way for
the user to invert that behavior.

-- Stefan^2.
Received on 2014-07-29 15:55:49 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.