On 29.07.2014 13:34, Stefan Fuhrmann wrote: > On Mon, Jul 28, 2014 at 7:38 PM, Branko Čibej <br...@wandisco.com> wrote: >> On 28.07.2014 19:19, stef...@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. br...@wandisco.com
