From dab9d4c23976a520a45cc739f3a3acf70e226b8b Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 12 May 2021 12:51:59 -0700 Subject: [PATCH 1/6] Fix `module_mapping` to work regardless of capitalization and `-` vs `_` # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../default_module_mapping.py | 20 +++++++------------ .../dependency_inference/module_mapper.py | 8 +++----- .../module_mapper_test.py | 4 +++- .../pants/backend/python/target_types.py | 13 ++++++++++-- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/default_module_mapping.py b/src/python/pants/backend/python/dependency_inference/default_module_mapping.py index 43c08d0e385..04bd64c1179 100644 --- a/src/python/pants/backend/python/dependency_inference/default_module_mapping.py +++ b/src/python/pants/backend/python/dependency_inference/default_module_mapping.py @@ -1,9 +1,10 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -# notice: using sets here to ensure the mapping is hashable +# NB: You must use lowercase and replace all `-` with `_` for the requirement's name. DEFAULT_MODULE_MAPPING = { "ansicolors": ("colors",), + "apache_airflow": ("airflow",), "attrs": ("attr",), "beautifulsoup4": ("bs4",), "djangorestframework": ("rest_framework",), @@ -12,17 +13,10 @@ "protobuf": ("google.protobuf",), "pycrypto": ("Crypto",), "pyopenssl": ("OpenSSL",), - "python-dateutil": ("dateutil",), - "python-jose": ("jose",), - "PyYAML": ("yaml",), - "pymongo": ( - "bson", - "gridfs", - ), - "pytest_runner": ("ptr",), "python_dateutil": ("dateutil",), - "setuptools": ( - "easy_install", - "pkg_resources", - ), + "python_jose": ("jose",), + "pyyaml": ("yaml",), + "pymongo": ("bson", "gridfs"), + "pytest_runner": ("ptr",), + "setuptools": ("easy_install", "pkg_resources"), } diff --git a/src/python/pants/backend/python/dependency_inference/module_mapper.py b/src/python/pants/backend/python/dependency_inference/module_mapper.py index 6e138e48ccb..868cfb2d512 100644 --- a/src/python/pants/backend/python/dependency_inference/module_mapper.py +++ b/src/python/pants/backend/python/dependency_inference/module_mapper.py @@ -238,11 +238,9 @@ async def map_third_party_modules_to_addresses() -> ThirdPartyPythonModuleMappin if not tgt.has_field(PythonRequirementsField): continue module_map = tgt.get(ModuleMappingField).value - for python_req in tgt[PythonRequirementsField].value: - modules = module_map.get( - python_req.project_name, - [python_req.project_name.lower().replace("-", "_")], - ) + for req in tgt[PythonRequirementsField].value: + normalized_project_name = ModuleMappingField.normalize_project_name(req.project_name) + modules = module_map.get(normalized_project_name, [normalized_project_name]) for module in modules: if module in modules_to_addresses: modules_with_multiple_owners[module].update( diff --git a/src/python/pants/backend/python/dependency_inference/module_mapper_test.py b/src/python/pants/backend/python/dependency_inference/module_mapper_test.py index 46d0a86aab5..86faa565f8e 100644 --- a/src/python/pants/backend/python/dependency_inference/module_mapper_test.py +++ b/src/python/pants/backend/python/dependency_inference/module_mapper_test.py @@ -255,7 +255,8 @@ def test_map_third_party_modules_to_addresses(rule_runner: RuleRunner) -> None: python_requirement_library( name='un_normalized', - requirements=['Un-Normalized-Project>3', 'two_owners'], + requirements=['Un-Normalized-Project>3', 'two_owners', 'DiFFerent-than_Mapping'], + module_mapping={"different_THAN-mapping": ["different_than_mapping"]}, ) python_requirement_library( @@ -272,6 +273,7 @@ def test_map_third_party_modules_to_addresses(rule_runner: RuleRunner) -> None: mapping=FrozenDict( { "colors": Address("3rdparty/python", target_name="ansicolors"), + "different_than_mapping": Address("3rdparty/python", target_name="un_normalized"), "local_dist": Address("3rdparty/python", target_name="direct_references"), "pip": Address("3rdparty/python", target_name="direct_references"), "req1": Address("3rdparty/python", target_name="req1"), diff --git a/src/python/pants/backend/python/target_types.py b/src/python/pants/backend/python/target_types.py index a918c36f681..7c116240b4b 100644 --- a/src/python/pants/backend/python/target_types.py +++ b/src/python/pants/backend/python/target_types.py @@ -587,16 +587,25 @@ class ModuleMappingField(DictStringToStringSequenceField): "A mapping of requirement names to a list of the modules they provide.\n\nFor example, " '`{"ansicolors": ["colors"]}`. Any unspecified requirements will use the requirement ' 'name as the default module, e.g. "Django" will default to `["django"]`.\n\nThis is ' - "used for Pants to be able to infer dependencies in BUILD files." + "used to infer dependencies." ) value: FrozenDict[str, Tuple[str, ...]] + @staticmethod + def normalize_project_name(s: str) -> str: + return s.lower().replace("-", "_") + @classmethod def compute_value( cls, raw_value: Optional[Dict[str, Iterable[str]]], address: Address ) -> FrozenDict[str, Tuple[str, ...]]: provided_mapping = super().compute_value(raw_value, address) - return FrozenDict({**DEFAULT_MODULE_MAPPING, **(provided_mapping or {})}) + return FrozenDict( + { + **DEFAULT_MODULE_MAPPING, + **{cls.normalize_project_name(k): v for k, v in (provided_mapping or {}).items()}, + } + ) class PythonRequirementLibrary(Target): From 285490970f61457bc61c5169b606db8e3c3bc9b7 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 12 May 2021 13:55:12 -0700 Subject: [PATCH 2/6] Use packaging.utils.canonicalize_name # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../dependency_inference/default_module_mapping.py | 13 +++++++------ .../python/dependency_inference/module_mapper.py | 8 ++++++-- .../dependency_inference/module_mapper_test.py | 9 +++++++++ src/python/pants/backend/python/target_types.py | 8 +++----- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/default_module_mapping.py b/src/python/pants/backend/python/dependency_inference/default_module_mapping.py index 04bd64c1179..877d501f739 100644 --- a/src/python/pants/backend/python/dependency_inference/default_module_mapping.py +++ b/src/python/pants/backend/python/dependency_inference/default_module_mapping.py @@ -1,22 +1,23 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -# NB: You must use lowercase and replace all `-` with `_` for the requirement's name. +# NB: You must use lowercase and replace all `_` and `.` with `-` for the requirement's name. +# See https://www.python.org/dev/peps/pep-0503/#normalized-names. DEFAULT_MODULE_MAPPING = { "ansicolors": ("colors",), - "apache_airflow": ("airflow",), + "apache-airflow": ("airflow",), "attrs": ("attr",), "beautifulsoup4": ("bs4",), "djangorestframework": ("rest_framework",), "enum34": ("enum",), - "paho_mqtt": ("paho",), + "paho-mqtt": ("paho",), "protobuf": ("google.protobuf",), "pycrypto": ("Crypto",), "pyopenssl": ("OpenSSL",), - "python_dateutil": ("dateutil",), - "python_jose": ("jose",), + "python-dateutil": ("dateutil",), + "python-jose": ("jose",), "pyyaml": ("yaml",), "pymongo": ("bson", "gridfs"), - "pytest_runner": ("ptr",), + "pytest-runner": ("ptr",), "setuptools": ("easy_install", "pkg_resources"), } diff --git a/src/python/pants/backend/python/dependency_inference/module_mapper.py b/src/python/pants/backend/python/dependency_inference/module_mapper.py index 868cfb2d512..c2b03e72cf5 100644 --- a/src/python/pants/backend/python/dependency_inference/module_mapper.py +++ b/src/python/pants/backend/python/dependency_inference/module_mapper.py @@ -9,6 +9,8 @@ from pathlib import PurePath from typing import DefaultDict +from packaging.utils import canonicalize_name as canonicalize_project_name + from pants.backend.python.target_types import ( ModuleMappingField, PythonRequirementsField, @@ -239,8 +241,10 @@ async def map_third_party_modules_to_addresses() -> ThirdPartyPythonModuleMappin continue module_map = tgt.get(ModuleMappingField).value for req in tgt[PythonRequirementsField].value: - normalized_project_name = ModuleMappingField.normalize_project_name(req.project_name) - modules = module_map.get(normalized_project_name, [normalized_project_name]) + normalized_project_name = canonicalize_project_name(req.project_name) + modules = module_map.get( + normalized_project_name, [normalized_project_name.replace("-", "_")] + ) for module in modules: if module in modules_to_addresses: modules_with_multiple_owners[module].update( diff --git a/src/python/pants/backend/python/dependency_inference/module_mapper_test.py b/src/python/pants/backend/python/dependency_inference/module_mapper_test.py index 86faa565f8e..614c512faae 100644 --- a/src/python/pants/backend/python/dependency_inference/module_mapper_test.py +++ b/src/python/pants/backend/python/dependency_inference/module_mapper_test.py @@ -7,9 +7,11 @@ from textwrap import dedent import pytest +from packaging.utils import canonicalize_name as canonicalize_project_name from pants.backend.codegen.protobuf.python import python_protobuf_module_mapper from pants.backend.codegen.protobuf.target_types import ProtobufLibrary +from pants.backend.python.dependency_inference.default_module_mapping import DEFAULT_MODULE_MAPPING from pants.backend.python.dependency_inference.module_mapper import ( FirstPartyPythonModuleMapping, PythonModule, @@ -24,6 +26,13 @@ from pants.util.frozendict import FrozenDict +def test_default_module_mapping_is_normalized() -> None: + for k in DEFAULT_MODULE_MAPPING: + assert k == canonicalize_project_name( + k + ), "Please update `DEFAULT_MODULE_MAPPING` to use canonical project names" + + @pytest.mark.parametrize( "stripped_path,expected", [ diff --git a/src/python/pants/backend/python/target_types.py b/src/python/pants/backend/python/target_types.py index 7c116240b4b..64165bb8814 100644 --- a/src/python/pants/backend/python/target_types.py +++ b/src/python/pants/backend/python/target_types.py @@ -6,12 +6,14 @@ import collections.abc import logging import os.path +import re from abc import ABC, abstractmethod from dataclasses import dataclass from enum import Enum from textwrap import dedent from typing import Dict, Iterable, Iterator, Optional, Tuple, Union, cast +from packaging.utils import canonicalize_name as canonicalize_project_name from pkg_resources import Requirement from pants.backend.python.dependency_inference.default_module_mapping import DEFAULT_MODULE_MAPPING @@ -591,10 +593,6 @@ class ModuleMappingField(DictStringToStringSequenceField): ) value: FrozenDict[str, Tuple[str, ...]] - @staticmethod - def normalize_project_name(s: str) -> str: - return s.lower().replace("-", "_") - @classmethod def compute_value( cls, raw_value: Optional[Dict[str, Iterable[str]]], address: Address @@ -603,7 +601,7 @@ def compute_value( return FrozenDict( { **DEFAULT_MODULE_MAPPING, - **{cls.normalize_project_name(k): v for k, v in (provided_mapping or {}).items()}, + **{canonicalize_project_name(k): v for k, v in (provided_mapping or {}).items()}, } ) From 1cf5ae91db17574619e92a750babf735aac1c18a Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 12 May 2021 14:09:42 -0700 Subject: [PATCH 3/6] Less chatty comment # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../python/dependency_inference/default_module_mapping.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/default_module_mapping.py b/src/python/pants/backend/python/dependency_inference/default_module_mapping.py index 877d501f739..02b98998a54 100644 --- a/src/python/pants/backend/python/dependency_inference/default_module_mapping.py +++ b/src/python/pants/backend/python/dependency_inference/default_module_mapping.py @@ -1,8 +1,8 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -# NB: You must use lowercase and replace all `_` and `.` with `-` for the requirement's name. -# See https://www.python.org/dev/peps/pep-0503/#normalized-names. +# NB: The project names must follow the naming scheme at +# https://www.python.org/dev/peps/pep-0503/#normalized-names. DEFAULT_MODULE_MAPPING = { "ansicolors": ("colors",), "apache-airflow": ("airflow",), From 7e9c9b466f23e0e71a18b9b56d868943b8f1a0de Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 12 May 2021 15:29:25 -0700 Subject: [PATCH 4/6] See if CI still fails [ci skip-rust] [ci skip-build-wheels] From c2dc383fc91b686bdfa052d97ca3bd82c5cef3ec Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 12 May 2021 16:35:01 -0700 Subject: [PATCH 5/6] Fix legit lint and test failures # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/backend/python/target_types.py | 1 - src/python/pants/backend/python/target_types_test.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/python/target_types.py b/src/python/pants/backend/python/target_types.py index 64165bb8814..3d9d32222b3 100644 --- a/src/python/pants/backend/python/target_types.py +++ b/src/python/pants/backend/python/target_types.py @@ -6,7 +6,6 @@ import collections.abc import logging import os.path -import re from abc import ABC, abstractmethod from dataclasses import dataclass from enum import Enum diff --git a/src/python/pants/backend/python/target_types_test.py b/src/python/pants/backend/python/target_types_test.py index 15a2cea9435..3a7e59033e0 100644 --- a/src/python/pants/backend/python/target_types_test.py +++ b/src/python/pants/backend/python/target_types_test.py @@ -370,12 +370,12 @@ def test_python_distribution_dependency_injection() -> None: ["raw_value", "expected"], ( (None, {}), - ({"new_dist": ["new_module"]}, {"new_dist": ("new_module",)}), - ({"PyYAML": ["custom_yaml"]}, {"PyYAML": ("custom_yaml",)}), + ({"new-dist": ["new_module"]}, {"new-dist": ("new_module",)}), + ({"PyYAML": ["custom_yaml"]}, {"pyyaml": ("custom_yaml",)}), ), ) def test_module_mapping_field( - raw_value: Optional[Dict[str, Iterable[str]]], expected: Dict[str, Tuple[str]] + raw_value: Optional[Dict[str, Iterable[str]]], expected: Dict[str, Tuple[str, ...]] ) -> None: merged = dict(DEFAULT_MODULE_MAPPING) merged.update(expected) From 449dc27eafa775bf8b0fbd3c0168330d1380e6ec Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 12 May 2021 17:49:53 -0700 Subject: [PATCH 6/6] Fix the default module to not remove '.' and add a test # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../python/dependency_inference/module_mapper.py | 7 +++++-- .../python/dependency_inference/module_mapper_test.py | 11 ++++++++++- .../backend/python/macros/pants_requirement_test.py | 7 ++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/module_mapper.py b/src/python/pants/backend/python/dependency_inference/module_mapper.py index c2b03e72cf5..53120ebe8ac 100644 --- a/src/python/pants/backend/python/dependency_inference/module_mapper.py +++ b/src/python/pants/backend/python/dependency_inference/module_mapper.py @@ -241,9 +241,12 @@ async def map_third_party_modules_to_addresses() -> ThirdPartyPythonModuleMappin continue module_map = tgt.get(ModuleMappingField).value for req in tgt[PythonRequirementsField].value: - normalized_project_name = canonicalize_project_name(req.project_name) modules = module_map.get( - normalized_project_name, [normalized_project_name.replace("-", "_")] + # NB: We only use `canonicalize_project_name()` for the key, but not the fallback + # value, because we want to preserve `.` in the module name. See + # https://www.python.org/dev/peps/pep-0503/#normalized-names. + canonicalize_project_name(req.project_name), + [req.project_name.lower().replace("-", "_")], ) for module in modules: if module in modules_to_addresses: diff --git a/src/python/pants/backend/python/dependency_inference/module_mapper_test.py b/src/python/pants/backend/python/dependency_inference/module_mapper_test.py index 614c512faae..02244a4fb23 100644 --- a/src/python/pants/backend/python/dependency_inference/module_mapper_test.py +++ b/src/python/pants/backend/python/dependency_inference/module_mapper_test.py @@ -88,10 +88,16 @@ def test_first_party_modules_mapping() -> None: def test_third_party_modules_mapping() -> None: colors_addr = Address("", target_name="ansicolors") pants_addr = Address("", target_name="pantsbuild") + pants_testutil_addr = Address("", target_name="pantsbuild.testutil") submodule_addr = Address("", target_name="submodule") mapping = ThirdPartyPythonModuleMapping( mapping=FrozenDict( - {"colors": colors_addr, "pants": pants_addr, "req.submodule": submodule_addr} + { + "colors": colors_addr, + "pants": pants_addr, + "req.submodule": submodule_addr, + "pants.testutil": pants_testutil_addr, + } ), ambiguous_modules=FrozenDict({"ambiguous": (colors_addr, pants_addr)}), ) @@ -103,6 +109,9 @@ def test_third_party_modules_mapping() -> None: assert mapping.address_for_module("pants.task.task") == (pants_addr, ()) assert mapping.address_for_module("pants.task.task.Task") == (pants_addr, ()) + assert mapping.address_for_module("pants.testutil") == (pants_testutil_addr, ()) + assert mapping.address_for_module("pants.testutil.foo") == (pants_testutil_addr, ()) + assert mapping.address_for_module("req.submodule") == (submodule_addr, ()) assert mapping.address_for_module("req.submodule.foo") == (submodule_addr, ()) assert mapping.address_for_module("req.another") == (None, ()) diff --git a/src/python/pants/backend/python/macros/pants_requirement_test.py b/src/python/pants/backend/python/macros/pants_requirement_test.py index 40e142e5fd7..16da650fd98 100644 --- a/src/python/pants/backend/python/macros/pants_requirement_test.py +++ b/src/python/pants/backend/python/macros/pants_requirement_test.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import pytest +from packaging.utils import canonicalize_name as canonicalize_project_name from pkg_resources import Requirement from pants.backend.python.macros.pants_requirement import PantsRequirement @@ -39,9 +40,9 @@ def assert_pants_requirement( assert target[PythonRequirementsField].value == ( Requirement.parse(f"{expected_dist}=={pants_version()}"), ) - actual_value = target[ModuleMappingField].value - assert isinstance(actual_value, FrozenDict) - assert actual_value.get(expected_dist) == (expected_module,) + module_mapping = target[ModuleMappingField].value + assert isinstance(module_mapping, FrozenDict) + assert module_mapping.get(canonicalize_project_name(expected_dist)) == (expected_module,) def test_target_name(rule_runner: RuleRunner) -> None: