Merge pull request pypa#7072 from TonyBeswick/master

Fix pip freeze not showing correct entry for mercurial packages that use subdirectories.

Author: pradyunsg

news/7071.bugfix

@@ -0,0 +1 @@
+Fix ``pip freeze`` not showing correct entry for mercurial packages that use subdirectories.

src/pip/_internal/utils/subprocess.py

@@ -123,7 +123,8 @@ def call_subprocess(
     command_desc=None,  # type: Optional[str]
     extra_environ=None,  # type: Optional[Mapping[str, Any]]
     unset_environ=None,  # type: Optional[Iterable[str]]
-    spinner=None  # type: Optional[SpinnerInterface]
+    spinner=None,  # type: Optional[SpinnerInterface]
+    log_failed_cmd=True  # type: Optional[bool]
 ):
     # type: (...) -> Text
     """
@@ -134,6 +135,7 @@ def call_subprocess(
         acceptable, in addition to 0. Defaults to None, which means [].
       unset_environ: an iterable of environment variable names to unset
         prior to calling subprocess.Popen().
+      log_failed_cmd: if false, failed commands are not logged, only raised.
     """
     if extra_ok_returncodes is None:
         extra_ok_returncodes = []
@@ -189,9 +191,10 @@ def call_subprocess(
         )
         proc.stdin.close()
     except Exception as exc:
-        subprocess_logger.critical(
-            "Error %s while executing command %s", exc, command_desc,
-        )
+        if log_failed_cmd:
+            subprocess_logger.critical(
+                "Error %s while executing command %s", exc, command_desc,
+            )
         raise
     all_output = []
     while True:
@@ -222,7 +225,7 @@ def call_subprocess(
             spinner.finish("done")
     if proc_had_error:
         if on_returncode == 'raise':
-            if not showing_subprocess:
+            if not showing_subprocess and log_failed_cmd:
                 # Then the subprocess streams haven't been logged to the
                 # console yet.
                 msg = make_subprocess_output_error(

src/pip/_internal/vcs/git.py

@@ -12,14 +12,14 @@
 from pip._vendor.six.moves.urllib import request as urllib_request
 
 from pip._internal.exceptions import BadCommand
-from pip._internal.utils.compat import samefile
 from pip._internal.utils.misc import display_path
 from pip._internal.utils.subprocess import make_command
 from pip._internal.utils.temp_dir import TempDirectory
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 from pip._internal.vcs.versioncontrol import (
     RemoteNotFoundError,
     VersionControl,
+    find_path_to_setup_from_repo_root,
     vcs,
 )
 
@@ -295,30 +295,18 @@ def get_revision(cls, location, rev=None):
 
     @classmethod
     def get_subdirectory(cls, location):
+        """
+        Return the path to setup.py, relative to the repo root.
+        Return None if setup.py is in the repo root.
+        """
         # find the repo root
-        git_dir = cls.run_command(['rev-parse', '--git-dir'],
-                                  show_stdout=False, cwd=location).strip()
+        git_dir = cls.run_command(
+            ['rev-parse', '--git-dir'],
+            show_stdout=False, cwd=location).strip()
         if not os.path.isabs(git_dir):
             git_dir = os.path.join(location, git_dir)
-        root_dir = os.path.join(git_dir, '..')
-        # find setup.py
-        orig_location = location
-        while not os.path.exists(os.path.join(location, 'setup.py')):
-            last_location = location
-            location = os.path.dirname(location)
-            if location == last_location:
-                # We've traversed up to the root of the filesystem without
-                # finding setup.py
-                logger.warning(
-                    "Could not find setup.py for directory %s (tried all "
-                    "parent directories)",
-                    orig_location,
-                )
-                return None
-        # relative path of setup.py to repo root
-        if samefile(root_dir, location):
-            return None
-        return os.path.relpath(location, root_dir)
+        repo_root = os.path.abspath(os.path.join(git_dir, '..'))
+        return find_path_to_setup_from_repo_root(location, repo_root)
 
     @classmethod
     def get_url_rev_and_auth(cls, url):
@@ -372,7 +360,8 @@ def controls_location(cls, location):
             r = cls.run_command(['rev-parse'],
                                 cwd=location,
                                 show_stdout=False,
-                                on_returncode='ignore')
+                                on_returncode='ignore',
+                                log_failed_cmd=False)
             return not r
         except BadCommand:
             logger.debug("could not determine if %s is under git control "

src/pip/_internal/vcs/mercurial.py

@@ -8,12 +8,17 @@
 
 from pip._vendor.six.moves import configparser
 
+from pip._internal.exceptions import BadCommand, InstallationError
 from pip._internal.utils.misc import display_path
 from pip._internal.utils.subprocess import make_command
 from pip._internal.utils.temp_dir import TempDirectory
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 from pip._internal.utils.urls import path_to_url
-from pip._internal.vcs.versioncontrol import VersionControl, vcs
+from pip._internal.vcs.versioncontrol import (
+    VersionControl,
+    find_path_to_setup_from_repo_root,
+    vcs,
+)
 
 if MYPY_CHECK_RUNNING:
     from pip._internal.utils.misc import HiddenText
@@ -118,5 +123,33 @@ def is_commit_id_equal(cls, dest, name):
         """Always assume the versions don't match"""
         return False
 
+    @classmethod
+    def get_subdirectory(cls, location):
+        """
+        Return the path to setup.py, relative to the repo root.
+        Return None if setup.py is in the repo root.
+        """
+        # find the repo root
+        repo_root = cls.run_command(
+            ['root'], show_stdout=False, cwd=location).strip()
+        if not os.path.isabs(repo_root):
+            repo_root = os.path.abspath(os.path.join(location, repo_root))
+        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
+        try:
+            cls.run_command(
+                ['identify'],
+                cwd=location,
+                show_stdout=False,
+                on_returncode='raise',
+                log_failed_cmd=False)
+            return True
+        except (BadCommand, InstallationError):
+            return False
+
 
 vcs.register(Mercurial)

src/pip/_internal/vcs/versioncontrol.py

@@ -15,6 +15,7 @@
 from pip._vendor.six.moves.urllib import parse as urllib_parse
 
 from pip._internal.exceptions import BadCommand
+from pip._internal.utils.compat import samefile
 from pip._internal.utils.misc import (
     ask_path_exists,
     backup_dir,
@@ -71,6 +72,33 @@ def make_vcs_requirement_url(repo_url, rev, project_name, subdir=None):
     return req
 
 
+def find_path_to_setup_from_repo_root(location, repo_root):
+    """
+    Find the path to `setup.py` by searching up the filesystem from `location`.
+    Return the path to `setup.py` relative to `repo_root`.
+    Return None if `setup.py` is in `repo_root` or cannot be found.
+    """
+    # find setup.py
+    orig_location = location
+    while not os.path.exists(os.path.join(location, 'setup.py')):
+        last_location = location
+        location = os.path.dirname(location)
+        if location == last_location:
+            # We've traversed up to the root of the filesystem without
+            # finding setup.py
+            logger.warning(
+                "Could not find setup.py for directory %s (tried all "
+                "parent directories)",
+                orig_location,
+            )
+            return None
+
+    if samefile(repo_root, location):
+        return None
+
+    return os.path.relpath(location, repo_root)
+
+
 class RemoteNotFoundError(Exception):
     pass
 
@@ -240,9 +268,10 @@ def should_add_vcs_url_prefix(cls, remote_url):
         return not remote_url.lower().startswith('{}:'.format(cls.name))
 
     @classmethod
-    def get_subdirectory(cls, repo_dir):
+    def get_subdirectory(cls, location):
         """
         Return the path to setup.py, relative to the repo root.
+        Return None if setup.py is in the repo root.
         """
         return None
 
@@ -582,7 +611,8 @@ def run_command(
         extra_ok_returncodes=None,  # type: Optional[Iterable[int]]
         command_desc=None,  # type: Optional[str]
         extra_environ=None,  # type: Optional[Mapping[str, Any]]
-        spinner=None  # type: Optional[SpinnerInterface]
+        spinner=None,  # type: Optional[SpinnerInterface]
+        log_failed_cmd=True
     ):
         # type: (...) -> Text
         """
@@ -598,7 +628,8 @@ def run_command(
                                    command_desc=command_desc,
                                    extra_environ=extra_environ,
                                    unset_environ=cls.unset_environ,
-                                   spinner=spinner)
+                                   spinner=spinner,
+                                   log_failed_cmd=log_failed_cmd)
         except OSError as e:
             # errno.ENOENT = no such file or directory
             # In other words, the VCS executable isn't available

tests/functional/test_freeze.py

@@ -322,6 +322,46 @@ def test_freeze_git_clone_srcdir(script, tmpdir):
     _check_output(result.stdout, expected)
 
 
+@need_mercurial
+def test_freeze_mercurial_clone_srcdir(script, tmpdir):
+    """
+    Test freezing a Mercurial clone where setup.py is in a subdirectory
+    relative to the repo root and the source code is in a subdirectory
+    relative to setup.py.
+    """
+    # Returns path to a generated package called "version_pkg"
+    pkg_version = _create_test_package_with_srcdir(script, vcs='hg')
+
+    result = script.run(
+        'hg', 'clone', pkg_version, 'pip-test-package'
+    )
+    repo_dir = script.scratch_path / 'pip-test-package'
+    result = script.run(
+        'python', 'setup.py', 'develop',
+        cwd=repo_dir / 'subdir'
+    )
+    result = script.pip('freeze')
+    expected = textwrap.dedent(
+        """
+            ...-e hg+...#egg=version_pkg&subdirectory=subdir
+            ...
+        """
+    ).strip()
+    _check_output(result.stdout, expected)
+
+    result = script.pip(
+        'freeze', '-f', '%s#egg=pip_test_package' % repo_dir
+    )
+    expected = textwrap.dedent(
+        """
+            -f %(repo)s#egg=pip_test_package...
+            -e hg+...#egg=version_pkg&subdirectory=subdir
+            ...
+        """ % {'repo': repo_dir},
+    ).strip()
+    _check_output(result.stdout, expected)
+
+
 @pytest.mark.git
 def test_freeze_git_remote(script, tmpdir):
     """