[PATCH] cascading lookup support in LocationConfig (bug 33430)

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

James Henstridge wrote:
> On 19/09/06, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
>> Are you proposing that both plugins would register the policy?  Because
>> otherwise, you'd have an undesirable dependency, IMHO.  And so the
>> policy registration system would have to permit multiple registrations,
>> but raise an exception if two conflicting policies were registered.
>>
>> Is that what you had in mind?
> 
> That's exactly what I was thinking of.  Also, by registering the
> policy once it makes sure that the key is accessed in the same way
> everywhere, which would be more robust than having plugins or bzrlib
> code specify the policy each time they use a config value.

Okay, I'm now leaning toward having the policy come from the config
file.  It seems like a simpler change, and one that gives the user
greater control, while allowing plugins to have consistent policy.

One problem is that the "must be trusted" policy can't really be set
this way.

>> The other way for plugins to collaborate on policy is for them to set
>> and respect the recurse="True"/"False" flag in locations.conf
>>
>> This also allows the configuration system to be more flexible, so that
>> per-location configurations are not recursive by default, but users can
>> set a repository-wide value if they choose.
> 
> The problem I see with this is that it affects all configuration data
> inside the section.  

Yes, this is a problem I saw also.

> I suppose another method would be to encode the policy into the
> section name.  Maybe something like this?
> 
>    [/some/branch]
>    email = My email
> 
>    [/some/branch#no-recurse]
>    push_location = sftp://...

I was also considering "[/some/branch/*]".    One problem is that
.bzr/branch.conf and ~/.bazaar/global.conf don't have sections.

But another alternative would be to configure values individually.  I'm
not sure what that would look like.  I guess it could be:

[/some/branch]
submit_location = sftp://..
recursive_submit_location = True
push_location = sftp://..
recursive_push_location = True

or

[/some/branch]
submit_location = sftp://..
push_location = sftp://..
# ConfigObj understands lists natively
recursive = ["push_location", "submit_location"]

> Something similar could be done for the "append remaining path
> components to config value" policy you mentioned earlier in this
> thread.  An alternative to the above syntax might be "?recurse=false"
> -- both couldn't be confused with URI path components.

Hmm.  It would mean that people couldn't use query strings in branch
paths, but it would mean we could be more flexible in specifying policy
there.

>> If recurse=False were set for all automatic values, do you think
>> ignore_parents would still be useful?
> 
> It depends on whether anyone was relying on the current shadowing
> behaviour or not.  Maybe it would be better to leave it out and see if
> anyone asks for the feature.

Okay.  So I apologise for turning your patch into a design discussion.
Let's not let cascading block on config syntax discussion.  If you don't
mind producing a version that retains the existing recurse
functionality, I think I can approve that.  It's your choice whether to
include ignore_parents.

We can deal with syntax enhancement separately.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFFD+hr0F+nu1YWqI0RArU5AJ0YhO+08S7uJAYcPK4QR/lkmvYD2wCfVvWU
vG7ThjCIiwQhjCchBTlRq2Q=
=AjuV
-----END PGP SIGNATURE-----