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

Martin Geisler mg at aragost.com
Mon Jun 20 10:04:53 CDT 2011


Mads Kiilerich <mads at kiilerich.com> writes:

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

Yes, you got it.

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

Hmm, I did actually consider this. All I know is that my client came up
with this option in order to switch away from absolute subrepo URLs
since I told them that they are not recommended.

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

Definitely, the message I got is that the rewriting of relative subrepo
paths is quite useless, unfortunately.

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

You're right -- my client does indeed want to rewrite paths all over the
place. I made the a new lock extension for them[1] and they wanted to
rewrite the path to the lock server too.

[1]: http://mercurial.selenic.com/wiki/LockExtension/NewDesign

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

Finding the right word(s) is difficult and I would also be much happier
if we could skip this setting 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)?

You're probably right.

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

It is an undocumented feature, though I have forgotten if there were a
good usecase for it.

-- 
Martin Geisler

aragost Trifork
Professional Mercurial support
http://mercurial.aragost.com/kick-start/


More information about the Mercurial-devel mailing list