This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10264

path: forbid chaining `path://` definitions
ClosedPublic

Authored by marmoute on Mar 24 2021, 4:52 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG1ecf082386b7: path: forbid chaining `path://` definitions
Summary

To have path://xxx referencing paths that use path:// too, we need to
analyze dependencies to initialize them in the right order (and to detect
cycle).

I don't want to deal with that right now, so I just disallow it for now.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

marmoute created this revision.Mar 24 2021, 4:52 AM
marmoute added a commit: rHG1ecf082386b7: path: forbid chaining `path://` definitions.Mar 25 2021, 5:29 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG1ecf082386b7: path: forbid chaining `path://` definitions (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/ui.py (6 lines)
Mmercurial/util.py (15 lines)
Mtests/test-paths.t (36 lines)

Diff 26584

mercurial/ui.py

# Locations may define branches via syntax <base>#<branch>. # Locations may define branches via syntax <base>#<branch>.
u = util.url(rawloc) u = util.url(rawloc)
branch = None branch = None
if u.fragment: if u.fragment:
branch = u.fragment branch = u.fragment
u.fragment = None u.fragment = None
self.url = u self.url = u
# the url from the config/command line before dealing with `path://`
self.raw_url = u.copy()
self.branch = branch self.branch = branch
self.name = name self.name = name
self.rawloc = rawloc self.rawloc = rawloc
self.loc = b'%s' % u self.loc = b'%s' % u
self._validate_path() self._validate_path()
_path, sub_opts = ui.configsuboptions(b'paths', b'*') _path, sub_opts = ui.configsuboptions(b'paths', b'*')
self._own_sub_opts = {} self._own_sub_opts = {}
if suboptions is not None: if suboptions is not None:
self._own_sub_opts = suboptions.copy() self._own_sub_opts = suboptions.copy()
sub_opts.update(suboptions) sub_opts.update(suboptions)
self._all_sub_opts = sub_opts.copy() self._all_sub_opts = sub_opts.copy()
self._apply_suboptions(ui, sub_opts) self._apply_suboptions(ui, sub_opts)
def chain_path(self, ui, paths): def chain_path(self, ui, paths):
if self.url.scheme == b'path': if self.url.scheme == b'path':
assert self.url.path is None assert self.url.path is None
subpath = paths[self.url.host] subpath = paths[self.url.host]
if subpath.raw_url.scheme == b'path':
m = _('cannot use `%s`, "%s" is also define as a `path://`')
m %= (self.rawloc, self.url.host)
raise error.Abort(m)
self.url = subpath.url self.url = subpath.url
self.rawloc = subpath.rawloc self.rawloc = subpath.rawloc
self.loc = subpath.loc self.loc = subpath.loc
if self.branch is None: if self.branch is None:
self.branch = subpath.branch self.branch = subpath.branch
else: else:
base = self.rawloc.rsplit(b'#', 1)[0] base = self.rawloc.rsplit(b'#', 1)[0]
self.rawloc = b'%s#%s' % (base, self.branch) self.rawloc = b'%s#%s' % (base, self.branch)

mercurial/util.py

self.path = path self.path = path
# leave the query string escaped # leave the query string escaped
for a in (b'user', b'passwd', b'host', b'port', b'path', b'fragment'): for a in (b'user', b'passwd', b'host', b'port', b'path', b'fragment'):
v = getattr(self, a) v = getattr(self, a)
if v is not None: if v is not None:
setattr(self, a, urlreq.unquote(v)) setattr(self, a, urlreq.unquote(v))
def copy(self):
u = url(b'temporary useless value')
u.path = self.path
u.scheme = self.scheme
u.user = self.user
u.passwd = self.passwd
u.host = self.host
u.path = self.path
u.query = self.query
u.fragment = self.fragment
u._localpath = self._localpath
u._hostport = self._hostport
u._origpath = self._origpath
return u
@encoding.strmethod @encoding.strmethod
def __repr__(self): def __repr__(self):
attrs = [] attrs = []
for a in ( for a in (
b'scheme', b'scheme',
b'user', b'user',
b'passwd', b'passwd',
b'host', b'host',

tests/test-paths.t

[1] [1]
$ hg push pushdest-overwrite --new-branch $ hg push pushdest-overwrite --new-branch
pushing to $TESTTMP/push-dest pushing to $TESTTMP/push-dest
searching for changes searching for changes
adding changesets adding changesets
adding manifests adding manifests
adding file changes adding file changes
added 5 changesets with 0 changes to 0 files (+1 heads) added 5 changesets with 0 changes to 0 files (+1 heads)
Test chaining path:// definition
--------------------------------
This is currently unsupported, but feel free to implement the necessary
dependency detection.
$ cat << EOF >> .hg/hgrc
> chain_path=path://other_default
> EOF
$ hg id
000000000000
$ hg path
abort: cannot use `path://other_default`, "other_default" is also define as a `path://`
[255]
$ hg pull chain_path
abort: cannot use `path://other_default`, "other_default" is also define as a `path://`
[255]
Doing an actual circle should always be an issue
$ cat << EOF >> .hg/hgrc
> rock=path://cissors
> cissors=path://paper
> paper=://rock
> EOF
$ hg id
000000000000
$ hg path
abort: cannot use `path://other_default`, "other_default" is also define as a `path://`
[255]
$ hg pull chain_path
abort: cannot use `path://other_default`, "other_default" is also define as a `path://`
[255]