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

Aaron Bentley aaron.bentley at utoronto.ca
Mon Sep 18 17:26:06 BST 2006


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

James Henstridge wrote:
> On 14/09/06, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
>> But if they want to set policy for their own use, they can subclass
>> BranchConfig, as pqm-submit does.
> 
> 
> As John mentioned, pqm-submit doesn't subclass BranchConfig anymore.
> Furthermore, I don't think we should completely discount plugins
> sharing configuration.

[...]

> If the caller is going to be making policy decisions about how the key
> is looked up (recusive vs. non-recursive), then it would be a bug for
> two bits of code to use different policies.  It would be nice to catch
> these sorts of problems.  Upfront registration of the policy is one
> way of doing this.

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?

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.



> I've attached an updated branch that exposes the choice of whether to
> do a recursive lookup for the key or not in get_user_option().  I've
> set the default for this argument to "True", which most closely
> matches the current behaviour of using parent directory
> configurations.
> 
> I haven't looked at updating any code to pick the recurse=False
> policy, but Branch.get_push_location() is one candidate.

I think a better approach is to set the "recurse=False" flag when
setting this value.  That way, a user can can configure push to be
recursive for a particular repository.  By analogy with submit, it seems
reaonable to make that behaviour possible, even if it is not the default
for automatic values.

> -            if len(section_names) < len(location_names):
> -                try:
> -                    if not self._get_parser()[section].as_bool('recurse'):
> -                        continue

It appears you have disabled the "recurse=" functionality.  I think this
is a bad idea, because it is a useful feature, and because doing this
will break existing configurations.

> +        for (length, section, extra_path) in matches:
> +            sections.append((section, extra_path))
> +            # should we stop looking for parent configs here?
> +            try:
> +                if self._get_parser()[section].as_bool('ignore_parents'):
> +                    break

If recurse=False were set for all automatic values, do you think
ignore_parents would still be useful?

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

iD8DBQFFDsid0F+nu1YWqI0RAs+KAJ98wZuVcbTkrwFbH/A9cBe3hryl6gCePYIi
nT9LpE6zFfcYLloWGAiOGGc=
=TRYf
-----END PGP SIGNATURE-----




More information about the bazaar mailing list