vfs: add support for repo names with `$` when using with env vars (issue5739)
Needs ReviewPublic

Authored by navaneeth.suresh on Jan 7 2019, 3:38 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

$ foo=bar hg root fails to recognise the repo $foo. I stopped expanding env
vars from vfs when there exists a repo with the same name as the env var.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped
navaneeth.suresh retitled this revision from vfs: add support for repo names with `$` sign while using with env vars (issue5739) to vfs: add support for repo names with `$` when using with env vars (issue5739).
yuja added a subscriber: yuja.Jan 7 2019, 8:32 AM
def __init__(self, base, audit=True, cacheaudited=False, expandpath=False,
             realpath=False):

+ if '$' in base and os.path.isdir(base):
+ # when there exists a repo '$foo' and an env var foo=bar, stop
+ # expanding path. refer issue5739.
+ expandpath = False

This is logically incorrect. The problem is that we're doing variable
expansion at too lower layer. vfs(expand(user_specified_path)) makes
some sense, but vfs(expand(getcwd())) is clearly wrong. And the vfs class
can't know where the base comes from.

This is logically incorrect. The problem is that we're doing variable
expansion at too lower layer. vfs(expand(user_specified_path)) makes
some sense, but vfs(expand(getcwd())) is clearly wrong. And the vfs class
can't know where the base comes from.

If I add a condition for expanding env var if present in hgrc, can this work as a fix?
If the only solution for this is shifting path expansion from vfs class, where do you
think it can be?

yuja added a comment.Jan 9 2019, 9:12 AM
> This is logically incorrect. The problem is that we're doing variable
>  expansion at too lower layer. `vfs(expand(user_specified_path))` makes
>  some sense, but `vfs(expand(getcwd()))` is clearly wrong. And the vfs class
>  can't know where the `base` comes from.

If I add a condition for expanding env var if present in `hgrc`, can this work as a fix?

Maybe no. Where do you intend to add such code? vfs doesn't know where the path
comes from. Neither does localrepo.

If the only solution for this is shifting path expansion from `vfs` class, where do you
think it can be?

Somewhere specifying repository path read from hgrc or user input.