diff --git a/news/3988.bugfix b/news/3988.bugfix new file mode 100644 index 00000000000..314bd31fcbb --- /dev/null +++ b/news/3988.bugfix @@ -0,0 +1 @@ +Correctly freeze a VCS editable package when it is nested inside another VCS repository. diff --git a/src/pip/_internal/vcs/git.py b/src/pip/_internal/vcs/git.py index 7483303a94b..e173ec894ca 100644 --- a/src/pip/_internal/vcs/git.py +++ b/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) diff --git a/src/pip/_internal/vcs/mercurial.py b/src/pip/_internal/vcs/mercurial.py index d9b58cfe9a4..75e903cc8a6 100644 --- a/src/pip/_internal/vcs/mercurial.py +++ b/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) diff --git a/src/pip/_internal/vcs/versioncontrol.py b/src/pip/_internal/vcs/versioncontrol.py index 7cfd568829f..211c6f0ec78 100644 --- a/src/pip/_internal/vcs/versioncontrol.py +++ b/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] @@ -687,14 +699,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 diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py index 2e35de3e686..349c4af0805 100644 --- a/tests/functional/test_freeze.py +++ b/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 diff --git a/tests/functional/test_vcs_git.py b/tests/functional/test_vcs_git.py index de8b4c14b58..cee0a3cfeeb 100644 --- a/tests/functional/test_vcs_git.py +++ b/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) diff --git a/tests/functional/test_vcs_mercurial.py b/tests/functional/test_vcs_mercurial.py new file mode 100644 index 00000000000..841c4d8218e --- /dev/null +++ b/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)