Merge pull request #7287 from pradyunsg/refactor/simplify-metadata-ge…

…neration-flow

Simplify metadata generation flow, to be a lot more "linear"

src/pip/_internal/distributions/__init__.py

@@ -1,4 +1,4 @@
-from pip._internal.distributions.source.legacy import SourceDistribution
+from pip._internal.distributions.source import SourceDistribution
 from pip._internal.distributions.wheel import WheelDistribution
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 

src/pip/_internal/distributions/source/legacy.py → src/pip/_internal/distributions/source.py

@@ -16,28 +16,21 @@ class SourceDistribution(AbstractDistribution):
 
     The preparation step for these needs metadata for the packages to be
     generated, either using PEP 517 or using the legacy `setup.py egg_info`.
-
-    NOTE from @pradyunsg (14 June 2019)
-    I expect SourceDistribution class will need to be split into
-    `legacy_source` (setup.py based) and `source` (PEP 517 based) when we start
-    bringing logic for preparation out of InstallRequirement into this class.
     """
 
     def get_pkg_resources_distribution(self):
         return self.req.get_dist()
 
     def prepare_distribution_metadata(self, finder, build_isolation):
-        # Prepare for building. We need to:
-        #   1. Load pyproject.toml (if it exists)
-        #   2. Set up the build environment
-
+        # Load pyproject.toml, to determine whether PEP 517 is to be used
         self.req.load_pyproject_toml()
+
+        # Set up the build isolation, if this requirement should be isolated
         should_isolate = self.req.use_pep517 and build_isolation
         if should_isolate:
             self._setup_isolation(finder)
 
         self.req.prepare_metadata()
-        self.req.assert_source_matches_version()
 
     def _setup_isolation(self, finder):
         def _raise_conflicts(conflicting_with, conflicting_reqs):

src/pip/_internal/operations/build/metadata.py

@@ -5,36 +5,22 @@
 import logging
 import os
 
-from pip._internal.operations.build.metadata_legacy import \
-    generate_metadata as _generate_metadata_legacy
 from pip._internal.utils.subprocess import runner_with_spinner_message
 from pip._internal.utils.temp_dir import TempDirectory
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 
 if MYPY_CHECK_RUNNING:
-    from typing import Callable
-
     from pip._internal.req.req_install import InstallRequirement
 
 logger = logging.getLogger(__name__)
 
 
-def get_metadata_generator(install_req):
-    # type: (InstallRequirement) -> Callable[[InstallRequirement], str]
-    """Return a callable metadata generator for this InstallRequirement.
+def generate_metadata(install_req):
+    # type: (InstallRequirement) -> str
+    """Generate metadata using mechanisms described in PEP 517.
 
-    A metadata generator takes an InstallRequirement (install_req) as an input,
-    generates metadata via the appropriate process for that install_req and
-    returns the generated metadata directory.
+    Returns the generated metadata directory.
     """
-    if not install_req.use_pep517:
-        return _generate_metadata_legacy
-
-    return _generate_metadata
-
-
-def _generate_metadata(install_req):
-    # type: (InstallRequirement) -> str
     assert install_req.pep517_backend is not None
     build_env = install_req.build_env
     backend = install_req.pep517_backend

src/pip/_internal/operations/build/metadata_legacy.py

@@ -80,6 +80,10 @@ def depth_of_directory(dir_):
 
 def generate_metadata(install_req):
     # type: (InstallRequirement) -> str
+    """Generate metadata using setup.py-based defacto mechanisms.ArithmeticError
+
+    Returns the generated metadata directory.
+    """
     assert install_req.unpacked_source_directory
 
     req_details_str = install_req.name or "from {}".format(install_req.link)

src/pip/_internal/req/req_install.py

@@ -24,7 +24,9 @@
 from pip._internal.exceptions import InstallationError
 from pip._internal.locations import distutils_scheme
 from pip._internal.models.link import Link
-from pip._internal.operations.build.metadata import get_metadata_generator
+from pip._internal.operations.build.metadata import generate_metadata
+from pip._internal.operations.build.metadata_legacy import \
+    generate_metadata as generate_metadata_legacy
 from pip._internal.pyproject import load_pyproject_toml, make_pyproject_path
 from pip._internal.req.req_uninstall import UninstallPathSet
 from pip._internal.utils.compat import native_str
@@ -615,15 +617,21 @@ def prepare_metadata(self):
         """
         assert self.source_dir
 
-        metadata_generator = get_metadata_generator(self)
+        metadata_generator = generate_metadata
+        if not self.use_pep517:
+            metadata_generator = generate_metadata_legacy
+
         with indent_log():
             self.metadata_directory = metadata_generator(self)
 
+        # Act on the newly generated metadata, based on the name and version.
         if not self.name:
             self.move_to_correct_build_directory()
         else:
             self.warn_on_mismatching_name()
 
+        self.assert_source_matches_version()
+
     @property
     def metadata(self):
         # type: () -> Any

tests/unit/test_req.py

@@ -32,12 +32,7 @@
 from pip._internal.req.req_file import ParsedLine, get_line_parser, handle_line
 from pip._internal.req.req_tracker import RequirementTracker
 from pip._internal.utils.urls import path_to_url
-from tests.lib import (
-    DATA_DIR,
-    assert_raises_regexp,
-    make_test_finder,
-    requirements_file,
-)
+from tests.lib import assert_raises_regexp, make_test_finder, requirements_file
 
 
 def get_processed_req_from_line(line, fname='file', lineno=1):
@@ -664,17 +659,17 @@ def test_exclusive_environment_markers():
     assert req_set.has_requirement('Django')
 
 
-def test_mismatched_versions(caplog, tmpdir):
-    original_source = os.path.join(DATA_DIR, 'src', 'simplewheel-1.0')
-    source_dir = os.path.join(tmpdir, 'simplewheel')
-    shutil.copytree(original_source, source_dir)
-    req = InstallRequirement(req=Requirement('simplewheel==2.0'),
-                             comes_from=None, source_dir=source_dir)
-    req.prepare_metadata()
+def test_mismatched_versions(caplog):
+    req = InstallRequirement(
+        req=Requirement('simplewheel==2.0'),
+        comes_from=None,
+        source_dir="/tmp/somewhere",
+    )
+    # Monkeypatch!
+    req._metadata = {"name": "simplewheel", "version": "1.0"}
     req.assert_source_matches_version()
     assert caplog.records[-1].message == (
-        'Requested simplewheel==2.0, '
-        'but installing version 1.0'
+        'Requested simplewheel==2.0, but installing version 1.0'
     )