[PATCH] subrepo: allow remapping subpaths using the "final" path

Mads Kiilerich mads at kiilerich.com
Mon Jun 20 08:50:20 CDT 2011


On 06/20/2011 11:44 AM, Martin Geisler wrote:
> # HG changeset patch
> # User Martin Geisler<mg at aragost.com>
> # Date 1308563027 -7200
> # Branch stable
> # Node ID ebd533376a4604dfdf52a40c81e0842587156703
> # Parent  e4d3370fa23473b137edcd95fa843c7ecf6da64e
> subrepo: allow remapping subpaths using the "final" path
>
> The way subpath remapping happens now, the right-hand side of a .hgsub
> entry is used, as is, to match the left-hand side of a subpaths entry.
> The new idea is to expand the .hgsub entry by prefixing the parent
> repo path *before* the match occurs - i.e. to operate on the final
> path including the parent.

In other words: Without this patch the subpath remapping works on .hgsub 
right hand sides which may be absolute urls or relative paths. You 
propose an option that makes it work on the the absolute path (which 
might be the result of expanding relative paths).

> For example,
>
>    .hgsub entry:     src/foo = src/foo
>    parent repo path: http://example.net/parent
>    expansion:        http://example.net/parent/src/foo
>
> The rewriting proceeds like normal on the expanded path. The advantage
> of this is more context is available for the rewriting when the
> recommended trivial subrepo paths are used in the .hgsub file.
>
> A new option named ui.subpaths-remapping controls this. It has two
> values so far:
>
> * "use-intermediate-path", which is the old behavior that applies the
>    remapping rules before the expansion with the parent path occurs.
>    This is the default.
>
> * "use-final-path", which triggers the new behavior.

AFAICS the new behaviour is fully covered by the behaviour described in 
the existing documentation. There is no indication that it for relative 
paths works on the unprocessed right hand side. We are thus in the 
slightly grey area where the new behaviour can be seen as a bugfix and 
we can avoid this unfortunate flag for getting the old behaviour.

 From an alternative point of view: Wouldn't it be better to introduce a 
new configuration section with the new behaviour and deprecate the old 
one, instead of introducing a flag?

And: Why is this functionality limited to subrepos? That might be its 
primary use, but there might be other examples where it could be 
convenient to have general functionality for rewriting urls. One example 
could a global configuration for rewriting http to https, another could 
be if google code decides to change from 
https://foo.bar.googlecode.com/hg/ to https://googlecode.com/hg/foo.bar/ .

> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1074,6 +1074,17 @@
>   ``style``
>       Name of style to use for command output.
>
> +``subpaths-remaping``
> +    Controls remapping of relative subrepository paths as per the
> +    ``[subpaths]`` section. Relative paths are turned into absolute
> +    paths, this setting controls if the rewrite rules are applied
> +    before or after this expansion.
> +
> +    Set to ``use-intermediate-path`` to apply the remapping rules
> +    before the expansion with the parent path occurs. This is the
> +    default. Set it to ``use-final-path`` to rewrite the final expaned
> +    path.

We usually don't use "-" in configurations (even though there are 
exceptions to that rule). These names are not necessarily the first that 
comes to mind even when you know the concept. Single words like 
"specified" and "absolute" might be better. But as mentioned above I 
would rather avoid this configuration completely.

>   ``timeout``
>       The timeout used when a lock is held (in seconds), a negative value
>       means no timeout. Default is 600.
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -59,6 +59,12 @@
>               kind, src = src.split(']', 1)
>               kind = kind[1:]
>
> +        if ui.config('ui', 'subpaths-remapping') == 'use-final-path':
> +            if not os.path.isabs(src) and "://" not in src:
> +                parent = _abssource(ctx._repo, abort=False)
> +                if parent:
> +                    src = posixpath.join(parent, src)

This seems like an unfortunate almost-but-not-completely duplication of 
_abssource functionality for handling relative paths. Wouldn't it be 
better to make it a part of _abssource (unless it is moved to the url 
class)?

>           for pattern, repl in p.items('subpaths'):
>               # Turn r'C:\foo\bar' into r'C:\\foo\\bar' since re.sub
>               # does a string decode.

ps: [subpaths] can apparently be specified in .hgsub too. Is that a bug 
or an undocumented feature? (IMHO it is unfortunate and qualifies as a 
bug. It is a good idea to specify non-obvious (and thus volatile) 
subrepo paths outside the repository history, but having it in .hgsub is 
unnecessary complexity.)

/Mads


More information about the Mercurial-devel mailing list