subrepo: handle 'C:' style paths on the command line (issue5770)
authorMatt Harbison <matt_harbison@yahoo.com>
Sun, 21 Jan 2018 13:54:05 -0500
changeset 35777 0c0689a7565e
parent 35776 75bae69747f0
child 35778 128dd940bedc
subrepo: handle 'C:' style paths on the command line (issue5770) If you think 'C:' and 'C:\' are equivalent paths, see the inline comment before proceeding. The problem here was that several commands that take a URL argument (incoming, outgoing, pull, and push) will use that value to set 'repo._subtoppath' on the repository object after command specific manipulation of it, but before converting it to an absolute path. When an operation is performed on a relative subrepo, subrepo._abssource() will posixpath.join() this value with the relative subrepo path. That adds a '/' after the drive letter, changing how it is evaluated by abspath()/realpath() in vfsmod.vfs(..., realpath=True) as the subrepo is instantiated. I initially tried sanitizing the path in url.localpath(), because url.isabs() only checks that it starts with a drive letter. By the sample behavior, this is clearly not an absolute path. (Though the comment in isabs() is weasely- this style path can't be joined either.) But not everything funnels through there, and it required explicitly calling localpath() in hg.parseurl() and assigning to url.path to fix. But then tests failed with urls like 'a#0'. Next up was sanitizing the path in the url constructor. That caused doctest failures, because there are drive letter tests, so those got expanded in system specific ways. Yuya correctly pointed out that util.url is a parser, and shouldn't be substituting the path too. Rather than fixing every command call site, just convert it in the common subrepo location. I don't see any sanitizing on the path config options, so I fixed those too. Note that while the behavior is fixed here, there are still places where 'comparing with C:' gets printed out, and that's not great for debugging purposes. (Specifically I saw it in `hg incoming -B C:`, without subrepos.) While clone will write out an absolute default path, I wonder what would happen if a user edited that path to be 'C:'. (I don't think supporting relative paths in .hgrc is a sane thing to do, but while we're poking holes in things...) Since this is such an oddball case, it still leaks through in places, and there seems to be a lot of duplicate url parsing, maybe the url parsing should be moved to dispatch, and provide the command with a url object? Then we could convert this to an absolute path once, and not have to worry about it in the rest of the code. I also checked '--cwd C:' on the command line, and it was previously working because os.chdir() will DTRT. Finally, one other note from the url.localpath() experimenting. I don't see any cases where 'self._hostport' can hold a drive letter. So I'm wondering if that is wrong/old code.
mercurial/subrepo.py
--- a/mercurial/subrepo.py	Mon Jan 22 00:39:42 2018 -0500
+++ b/mercurial/subrepo.py	Sun Jan 21 13:54:05 2018 -0500
@@ -398,15 +398,35 @@
             parent.path = posixpath.normpath(parent.path)
             return bytes(parent)
     else: # recursion reached top repo
+        path = None
         if util.safehasattr(repo, '_subtoppath'):
-            return repo._subtoppath
-        if push and repo.ui.config('paths', 'default-push'):
-            return repo.ui.config('paths', 'default-push')
-        if repo.ui.config('paths', 'default'):
-            return repo.ui.config('paths', 'default')
-        if repo.shared():
-            # chop off the .hg component to get the default path form
+            path = repo._subtoppath
+        elif push and repo.ui.config('paths', 'default-push'):
+            path = repo.ui.config('paths', 'default-push')
+        elif repo.ui.config('paths', 'default'):
+            path = repo.ui.config('paths', 'default')
+        elif repo.shared():
+            # chop off the .hg component to get the default path form.  This has
+            # already run through vfsmod.vfs(..., realpath=True), so it doesn't
+            # have problems with 'C:'
             return os.path.dirname(repo.sharedpath)
+        if path:
+            # issue5770: 'C:\' and 'C:' are not equivalent paths.  The former is
+            # as expected: an absolute path to the root of the C: drive.  The
+            # latter is a relative path, and works like so:
+            #
+            #   C:\>cd C:\some\path
+            #   C:\>D:
+            #   D:\>python -c "import os; print os.path.abspath('C:')"
+            #   C:\some\path
+            #
+            #   D:\>python -c "import os; print os.path.abspath('C:relative')"
+            #   C:\some\path\relative
+            if util.hasdriveletter(path):
+                if len(path) == 2 or path[2:3] not in br'\/':
+                    path = os.path.abspath(path)
+            return path
+
     if abort:
         raise error.Abort(_("default path for subrepository not found"))