Merge pull request #7593 from uranusjr/hg-under-git

Detect all registered VCS and choose inner-most

news/3988.bugfix

@@ -0,0 +1 @@
+Correctly freeze a VCS editable package when it is nested inside another VCS repository.

src/pip/_internal/vcs/git.py

@@ -11,7 +11,7 @@
 from pip._vendor.six.moves.urllib import parse as urllib_parse
 from pip._vendor.six.moves.urllib import request as urllib_request
 
-from pip._internal.exceptions import BadCommand
+from pip._internal.exceptions import BadCommand, InstallationError
 from pip._internal.utils.misc import display_path, hide_url
 from pip._internal.utils.subprocess import make_command
 from pip._internal.utils.temp_dir import TempDirectory
@@ -370,20 +370,25 @@ def update_submodules(cls, location):
         )
 
     @classmethod
-    def controls_location(cls, location):
-        if super(Git, cls).controls_location(location):
-            return True
+    def get_repository_root(cls, location):
+        loc = super(Git, cls).get_repository_root(location)
+        if loc:
+            return loc
         try:
-            r = cls.run_command(['rev-parse'],
-                                cwd=location,
-                                show_stdout=False,
-                                on_returncode='ignore',
-                                log_failed_cmd=False)
-            return not r
+            r = cls.run_command(
+                ['rev-parse', '--show-toplevel'],
+                cwd=location,
+                show_stdout=False,
+                on_returncode='raise',
+                log_failed_cmd=False,
+            )
         except BadCommand:
             logger.debug("could not determine if %s is under git control "
                          "because git is not available", location)
-            return False
+            return None
+        except InstallationError:
+            return None
+        return os.path.normpath(r.rstrip('\r\n'))
 
 
 vcs.register(Git)

src/pip/_internal/vcs/mercurial.py

@@ -137,19 +137,25 @@ def get_subdirectory(cls, location):
         return find_path_to_setup_from_repo_root(location, repo_root)
 
     @classmethod
-    def controls_location(cls, location):
-        if super(Mercurial, cls).controls_location(location):
-            return True
+    def get_repository_root(cls, location):
+        loc = super(Mercurial, cls).get_repository_root(location)
+        if loc:
+            return loc
         try:
-            cls.run_command(
-                ['identify'],
+            r = cls.run_command(
+                ['root'],
                 cwd=location,
                 show_stdout=False,
                 on_returncode='raise',
-                log_failed_cmd=False)
-            return True
-        except (BadCommand, InstallationError):
-            return False
+                log_failed_cmd=False,
+            )
+        except BadCommand:
+            logger.debug("could not determine if %s is under hg control "
+                         "because hg is not available", location)
+            return None
+        except InstallationError:
+            return None
+        return os.path.normpath(r.rstrip('\r\n'))
 
 
 vcs.register(Mercurial)

src/pip/_internal/vcs/versioncontrol.py

@@ -232,12 +232,24 @@ def get_backend_for_dir(self, location):
         Return a VersionControl object if a repository of that type is found
         at the given directory.
         """
+        vcs_backends = {}
         for vcs_backend in self._registry.values():
-            if vcs_backend.controls_location(location):
-                logger.debug('Determine that %s uses VCS: %s',
-                             location, vcs_backend.name)
-                return vcs_backend
-        return None
+            repo_path = vcs_backend.get_repository_root(location)
+            if not repo_path:
+                continue
+            logger.debug('Determine that %s uses VCS: %s',
+                         location, vcs_backend.name)
+            vcs_backends[repo_path] = vcs_backend
+
+        if not vcs_backends:
+            return None
+
+        # Choose the VCS in the inner-most directory. Since all repository
+        # roots found here would be either `location` or one of its
+        # parents, the longest path should have the most path components,
+        # i.e. the backend representing the inner-most repository.
+        inner_most_repo_path = max(vcs_backends, key=len)
+        return vcs_backends[inner_most_repo_path]
 
     def get_backend_for_scheme(self, scheme):
         # type: (str) -> Optional[VersionControl]
@@ -688,14 +700,18 @@ def is_repository_directory(cls, path):
         return os.path.exists(os.path.join(path, cls.dirname))
 
     @classmethod
-    def controls_location(cls, location):
-        # type: (str) -> bool
+    def get_repository_root(cls, location):
+        # type: (str) -> Optional[str]
         """
-        Check if a location is controlled by the vcs.
+        Return the "root" (top-level) directory controlled by the vcs,
+        or `None` if the directory is not in any.
+
         It is meant to be overridden to implement smarter detection
         mechanisms for specific vcs.
 
-        This can do more than is_repository_directory() alone.  For example,
-        the Git override checks that Git is actually available.
+        This can do more than is_repository_directory() alone. For
+        example, the Git override checks that Git is actually available.
         """
-        return cls.is_repository_directory(location)
+        if cls.is_repository_directory(location):
+            return location
+        return None

tests/functional/test_freeze.py

@@ -10,6 +10,7 @@
     _create_test_package,
     _create_test_package_with_srcdir,
     _git_commit,
+    _vcs_add,
     need_bzr,
     need_mercurial,
     need_svn,
@@ -495,6 +496,39 @@ def test_freeze_bazaar_clone(script, tmpdir):
     _check_output(result.stdout, expected)
 
 
+@need_mercurial
+@pytest.mark.git
+@pytest.mark.parametrize(
+    "outer_vcs, inner_vcs",
+    [("hg", "git"), ("git", "hg")],
+)
+def test_freeze_nested_vcs(script, outer_vcs, inner_vcs):
+    """Test VCS can be correctly freezed when resides inside another VCS repo.
+    """
+    # Create Python package.
+    pkg_path = _create_test_package(script, vcs=inner_vcs)
+
+    # Create outer repo to clone into.
+    root_path = script.scratch_path.joinpath("test_freeze_nested_vcs")
+    root_path.mkdir()
+    root_path.joinpath(".hgignore").write_text("src")
+    root_path.joinpath(".gitignore").write_text("src")
+    _vcs_add(script, root_path, outer_vcs)
+
+    # Clone Python package into inner directory and install it.
+    src_path = root_path.joinpath("src")
+    src_path.mkdir()
+    script.run(inner_vcs, "clone", pkg_path, src_path, expect_stderr=True)
+    script.pip("install", "-e", src_path, expect_stderr=True)
+
+    # Check the freeze output recognizes the inner VCS.
+    result = script.pip("freeze", expect_stderr=True)
+    _check_output(
+        result.stdout,
+        "...-e {}+...#egg=version_pkg\n...".format(inner_vcs),
+    )
+
+
 # used by the test_freeze_with_requirement_* tests below
 _freeze_req_opts = textwrap.dedent("""\
     # Unchanged requirements below this line

tests/functional/test_vcs_git.py

@@ -238,3 +238,15 @@ def test_is_immutable_rev_checkout(script):
     assert not Git().is_immutable_rev_checkout(
         "git+https://g.c/o/r@master", version_pkg_path
     )
+
+
+def test_get_repository_root(script):
+    version_pkg_path = _create_test_package(script)
+    tests_path = version_pkg_path.joinpath("tests")
+    tests_path.mkdir()
+
+    root1 = Git.get_repository_root(version_pkg_path)
+    assert os.path.normcase(root1) == os.path.normcase(version_pkg_path)
+
+    root2 = Git.get_repository_root(version_pkg_path.joinpath("tests"))
+    assert os.path.normcase(root2) == os.path.normcase(version_pkg_path)

tests/functional/test_vcs_mercurial.py

@@ -0,0 +1,17 @@
+import os
+
+from pip._internal.vcs.mercurial import Mercurial
+from tests.lib import _create_test_package, need_mercurial
+
+
+@need_mercurial
+def test_get_repository_root(script):
+    version_pkg_path = _create_test_package(script, vcs="hg")
+    tests_path = version_pkg_path.joinpath("tests")
+    tests_path.mkdir()
+
+    root1 = Mercurial.get_repository_root(version_pkg_path)
+    assert os.path.normcase(root1) == os.path.normcase(version_pkg_path)
+
+    root2 = Mercurial.get_repository_root(version_pkg_path.joinpath("tests"))
+    assert os.path.normcase(root2) == os.path.normcase(version_pkg_path)