From 911722173ec3b7ed3b1c3ff5a2637a7340c904e4 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 3 Nov 2019 15:25:49 +0530 Subject: [PATCH 1/6] Move distributions.{source.legacy -> source} Why: Based on some more experience from refactoring metadata generation, it became clear to me that the separation of legacy vs modern codepaths should happen at lower level than this abstraction. --- src/pip/_internal/distributions/__init__.py | 2 +- .../_internal/distributions/{source/legacy.py => source.py} | 5 ----- src/pip/_internal/distributions/source/__init__.py | 0 3 files changed, 1 insertion(+), 6 deletions(-) rename src/pip/_internal/distributions/{source/legacy.py => source.py} (93%) delete mode 100644 src/pip/_internal/distributions/source/__init__.py diff --git a/src/pip/_internal/distributions/__init__.py b/src/pip/_internal/distributions/__init__.py index bba02f26cd6..20ffe2b32b5 100644 --- a/src/pip/_internal/distributions/__init__.py +++ b/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 diff --git a/src/pip/_internal/distributions/source/legacy.py b/src/pip/_internal/distributions/source.py similarity index 93% rename from src/pip/_internal/distributions/source/legacy.py rename to src/pip/_internal/distributions/source.py index 6d2f53ebc65..a4bbe838d19 100644 --- a/src/pip/_internal/distributions/source/legacy.py +++ b/src/pip/_internal/distributions/source.py @@ -16,11 +16,6 @@ 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): diff --git a/src/pip/_internal/distributions/source/__init__.py b/src/pip/_internal/distributions/source/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 From 33ccea2e0c4a7e0a07d41fc056dd06b33896dfdb Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 3 Nov 2019 16:25:35 +0530 Subject: [PATCH 2/6] Reduce operations.build.metadata's API to 1 function Why: There isn't any state being maintained, or multiple uses of the metadata generator for a single requirement. The additional complexity does not help in any significant manner. --- src/pip/_internal/operations/build/metadata.py | 17 ++++++----------- src/pip/_internal/req/req_install.py | 5 ++--- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/pip/_internal/operations/build/metadata.py b/src/pip/_internal/operations/build/metadata.py index 66b59eb5129..43c47813884 100644 --- a/src/pip/_internal/operations/build/metadata.py +++ b/src/pip/_internal/operations/build/metadata.py @@ -12,25 +12,20 @@ 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. - - 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. +def generate_metadata(install_req): + # type: (InstallRequirement) -> str + """Generate metadata and return the metadata directory. """ + func = _generate_metadata if not install_req.use_pep517: - return _generate_metadata_legacy + func = _generate_metadata_legacy - return _generate_metadata + return func(install_req) def _generate_metadata(install_req): diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index fe0f8b4c76d..20c29685c43 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -24,7 +24,7 @@ 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.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,9 +615,8 @@ def prepare_metadata(self): """ assert self.source_dir - metadata_generator = get_metadata_generator(self) with indent_log(): - self.metadata_directory = metadata_generator(self) + self.metadata_directory = generate_metadata(self) if not self.name: self.move_to_correct_build_directory() From 54fc70d212d445618c6e2ba94ddce63add7461d7 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 3 Nov 2019 19:35:44 +0530 Subject: [PATCH 3/6] Stop generating metadata as part of a unit test --- tests/unit/test_req.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index fcee96509aa..a7477b33c76 100644 --- a/tests/unit/test_req.py +++ b/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' ) From 67ae8fdc28bfc54ff2f67a852b889d460ac78861 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 3 Nov 2019 16:26:52 +0530 Subject: [PATCH 4/6] Move call to assert_source_matches_version --- src/pip/_internal/distributions/source.py | 1 - src/pip/_internal/req/req_install.py | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/distributions/source.py b/src/pip/_internal/distributions/source.py index a4bbe838d19..a4fad1f5472 100644 --- a/src/pip/_internal/distributions/source.py +++ b/src/pip/_internal/distributions/source.py @@ -32,7 +32,6 @@ def prepare_distribution_metadata(self, finder, build_isolation): 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): diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 20c29685c43..9e7fe83b3f2 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -623,6 +623,8 @@ def prepare_metadata(self): else: self.warn_on_mismatching_name() + self.assert_source_matches_version() + @property def metadata(self): # type: () -> Any From 528d27a2fe82b77ad88a98cd5112e2890dcb1494 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 3 Nov 2019 16:35:31 +0530 Subject: [PATCH 5/6] Nicer comments in prepare_distribution_metadata --- src/pip/_internal/distributions/source.py | 7 +++---- src/pip/_internal/req/req_install.py | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/distributions/source.py b/src/pip/_internal/distributions/source.py index a4fad1f5472..f74ec5acc82 100644 --- a/src/pip/_internal/distributions/source.py +++ b/src/pip/_internal/distributions/source.py @@ -22,11 +22,10 @@ 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) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 9e7fe83b3f2..aa8599d618c 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -618,6 +618,7 @@ def prepare_metadata(self): with indent_log(): self.metadata_directory = generate_metadata(self) + # Act on the newly generated metadata, based on the name and version. if not self.name: self.move_to_correct_build_directory() else: From f137aef12ed56279de4d6754164af883f9417ae0 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Mon, 4 Nov 2019 15:24:09 +0530 Subject: [PATCH 6/6] Choose metadata generator in prepare_metadata --- src/pip/_internal/operations/build/metadata.py | 15 +++------------ .../_internal/operations/build/metadata_legacy.py | 4 ++++ src/pip/_internal/req/req_install.py | 8 +++++++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/pip/_internal/operations/build/metadata.py b/src/pip/_internal/operations/build/metadata.py index 43c47813884..43c3590c0ef 100644 --- a/src/pip/_internal/operations/build/metadata.py +++ b/src/pip/_internal/operations/build/metadata.py @@ -5,8 +5,6 @@ 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 @@ -19,17 +17,10 @@ def generate_metadata(install_req): # type: (InstallRequirement) -> str - """Generate metadata and return the metadata directory. - """ - func = _generate_metadata - if not install_req.use_pep517: - func = _generate_metadata_legacy - - return func(install_req) - + """Generate metadata using mechanisms described in PEP 517. -def _generate_metadata(install_req): - # type: (InstallRequirement) -> str + Returns the generated metadata directory. + """ assert install_req.pep517_backend is not None build_env = install_req.build_env backend = install_req.pep517_backend diff --git a/src/pip/_internal/operations/build/metadata_legacy.py b/src/pip/_internal/operations/build/metadata_legacy.py index ba6265db791..d817504764b 100644 --- a/src/pip/_internal/operations/build/metadata_legacy.py +++ b/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) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index aa8599d618c..4bc341fb97d 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -25,6 +25,8 @@ from pip._internal.locations import distutils_scheme from pip._internal.models.link import Link 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,8 +617,12 @@ def prepare_metadata(self): """ assert self.source_dir + metadata_generator = generate_metadata + if not self.use_pep517: + metadata_generator = generate_metadata_legacy + with indent_log(): - self.metadata_directory = generate_metadata(self) + self.metadata_directory = metadata_generator(self) # Act on the newly generated metadata, based on the name and version. if not self.name: