diff --git a/src/python/pants/backend/python/macros/poetry_requirements.py b/src/python/pants/backend/python/macros/poetry_requirements.py index 25c432c6bbf..cb0ee6c5235 100644 --- a/src/python/pants/backend/python/macros/poetry_requirements.py +++ b/src/python/pants/backend/python/macros/poetry_requirements.py @@ -6,14 +6,15 @@ import itertools import logging import os -from pathlib import Path -from typing import Any, Iterable, Mapping, Optional +from dataclasses import dataclass +from pathlib import Path, PurePath +from typing import Any, Iterable, Iterator, Mapping, Optional import toml from packaging.version import InvalidVersion, Version from pkg_resources import Requirement -from pants.base.build_environment import get_buildroot +from pants.base.parse_context import ParseContext logger = logging.getLogger(__name__) @@ -67,12 +68,10 @@ def parse_str_version(proj_name: str, attributes: str, fp: str) -> str: parsed_version = Version(req[1:]) except InvalidVersion: raise InvalidVersion( - ( - f'Failed to parse requirement {proj_name} = "{req}" in {fp}' - "loaded by the poetry_requirements macro.\n\nIf you believe this requirement is " - "valid, consider opening an issue at https://github.com/pantsbuild/pants/issues" - "so that we can update Pants's Poetry macro to support this." - ) + f'Failed to parse requirement {proj_name} = "{req}" in {fp} loaded by the ' + "poetry_requirements macro.\n\nIf you believe this requirement is valid, " + "consider opening an issue at https://github.com/pantsbuild/pants/issues so " + "that we can update Pants' Poetry macro to support this." ) max_ver = get_max_caret(parsed_version) if is_caret else get_max_tilde(parsed_version) @@ -109,7 +108,68 @@ def prepend(version: str) -> str: ) -def handle_dict_attr(proj_name: str, attributes: dict[str, str], fp: str) -> str: +@dataclass(frozen=True) +class PyProjectToml: + build_root: PurePath + toml_relpath: PurePath + toml_contents: str + + @classmethod + def create(cls, parse_context: ParseContext, pyproject_toml_relpath: str) -> PyProjectToml: + build_root = Path(parse_context.build_root) + toml_relpath = PurePath(parse_context.rel_path, pyproject_toml_relpath) + return cls( + build_root=build_root, + toml_relpath=toml_relpath, + toml_contents=(build_root / toml_relpath).read_text(), + ) + + def parse(self) -> Mapping[str, Any]: + return toml.loads(self.toml_contents) + + def _non_pants_project_abs_path(self, path: Path) -> Path | None: + resolved = path.resolve() + if resolved.is_file(): + return resolved + + try: + resolved.relative_to(self.build_root) + except ValueError: + return resolved + + return None + + def non_pants_project_abs_path(self, path: str) -> Path | None: + """Determine if the given path represents a non-Pants controlled project. + + If the path points to a file, it's assumed the file is a distribution ( a wheel or sdist) + and the absolute path of that file is returned. + + If the path points to a directory and that directory is outside of the build root, it's + assumed the directory is the root of a buildable Python project (i.e.: it contains a + pyproject.toml or setup.py) and the absolute path of the project is returned. + + Otherwise, `None` is returned since the directory lies inside the build root and is assumed + to be a Pants controlled project. + """ + # TODO(John Sirois): This leaves the case where the path is a Python project directory + # inside the build root that the user actually wants Pex / Pip to build. A concrete case + # for this would be a repo where third party is partially handled with vendored exploded + # source distributions. If someone in the wild needs the described case, plumb a + # PoetryRequirements parameter that can list paths to treat as Pants controlled or + # vice-versa. + given_path = Path(path) + if given_path.is_absolute(): + return self._non_pants_project_abs_path(given_path) + else: + return self._non_pants_project_abs_path( + Path(self.build_root / self.toml_relpath).parent / given_path + ) + + +def handle_dict_attr( + proj_name: str, attributes: dict[str, str], pyproject_toml: PyProjectToml +) -> str | None: def produce_match(sep: str, feat: Optional[str]) -> str: return f"{sep}{feat}" if feat else "" @@ -122,14 +182,23 @@ def produce_match(sep: str, feat: Optional[str]) -> str: return f"{proj_name} @ git+{git_lookup}{tag_lookup}{branch_lookup}{rev_lookup}" version_lookup = attributes.get("version") + path_lookup = attributes.get("path") if path_lookup is not None: - return f"{proj_name} @ file://{path_lookup}" + non_pants_project_abs_path = pyproject_toml.non_pants_project_abs_path(path_lookup) + if non_pants_project_abs_path: + return f"{proj_name} @ file://{non_pants_project_abs_path}" + # An internal path will be handled by normal Pants dependencies and dependency inference; + # i.e.: it never represents a third party requirement. + return None + url_lookup = attributes.get("url") if url_lookup is not None: return f"{proj_name} @ {url_lookup}" + if version_lookup is not None: markers_lookup = produce_match(";", attributes.get("markers")) + fp = str(pyproject_toml.toml_relpath) python_lookup = parse_python_constraint(attributes.get("python"), fp) version_parsed = parse_str_version(proj_name, version_lookup, fp) return ( @@ -138,51 +207,50 @@ def produce_match(sep: str, feat: Optional[str]) -> str: f"{' and ' if python_lookup and markers_lookup else (';' if python_lookup else '')}" f"{python_lookup}" ) - else: - raise AssertionError( - ( - f"{proj_name} is not formatted correctly; at" - " minimum provide either a version, url, path or git location for" - " your dependency. " - ) - ) + + raise AssertionError( + f"{proj_name} is not formatted correctly; at minimum provide either a version, url, path " + "or git location for your dependency. " + ) def parse_single_dependency( - proj_name: str, attributes: str | dict[str, Any] | list[dict[str, Any]], fp: str -) -> tuple[Requirement, ...]: + proj_name: str, + attributes: str | dict[str, Any] | list[dict[str, Any]], + pyproject_toml: PyProjectToml, +) -> Iterator[Requirement]: if isinstance(attributes, str): # E.g. `foo = "~1.1~'. - return (Requirement.parse(parse_str_version(proj_name, attributes, fp)),) + yield Requirement.parse( + parse_str_version(proj_name, attributes, str(pyproject_toml.toml_relpath)) + ) elif isinstance(attributes, dict): # E.g. `foo = {version = "~1.1"}`. - return (Requirement.parse(handle_dict_attr(proj_name, attributes, fp)),) + req_str = handle_dict_attr(proj_name, attributes, pyproject_toml) + if req_str: + yield Requirement.parse(req_str) elif isinstance(attributes, list): # E.g. ` foo = [{version = "1.1","python" = "2.7"}, {version = "1.1","python" = "2.7"}] - return tuple( - Requirement.parse(handle_dict_attr(proj_name, attr, fp)) for attr in attributes - ) + for attr in attributes: + req_str = handle_dict_attr(proj_name, attr, pyproject_toml) + if req_str: + yield Requirement.parse(req_str) else: raise AssertionError( - ( - "Error: invalid poetry requirement format. Expected " - " type of requirement attributes to be string," - f"dict, or list, but was of type {type(attributes).__name__}." - ) + "Error: invalid poetry requirement format. Expected type of requirement attributes to " + f"be string, dict, or list, but was of type {type(attributes).__name__}." ) -def parse_pyproject_toml(toml_contents: str, file_path: str) -> set[Requirement]: - parsed = toml.loads(toml_contents) +def parse_pyproject_toml(pyproject_toml: PyProjectToml) -> set[Requirement]: + parsed = pyproject_toml.parse() try: poetry_vals = parsed["tool"]["poetry"] except KeyError: raise KeyError( - ( - f"No section `tool.poetry` found in {file_path}, which" - "is loaded by Pants from a `poetry_requirements` macro. " - "Did you mean to set up Poetry?" - ) + f"No section `tool.poetry` found in {pyproject_toml.toml_relpath}, which " + "is loaded by Pants from a `poetry_requirements` macro. " + "Did you mean to set up Poetry?" ) dependencies = poetry_vals.get("dependencies", {}) # N.B.: The "python" dependency is a special dependency required by Poetry that only serves to @@ -193,17 +261,15 @@ def parse_pyproject_toml(toml_contents: str, file_path: str) -> set[Requirement] dev_dependencies = poetry_vals.get("dev-dependencies", {}) if not dependencies and not dev_dependencies: logger.warning( - ( - "No requirements defined in poetry.tools.dependencies and" - f" poetry.tools.dev-dependencies in {file_path}, which is loaded by Pants" - " from a poetry_requirements macro. Did you mean to populate these" - " with requirements?" - ) + "No requirements defined in poetry.tools.dependencies and " + f"poetry.tools.dev-dependencies in {pyproject_toml.toml_relpath}, which is loaded " + "by Pants from a poetry_requirements macro. Did you mean to populate these " + "with requirements?" ) return set( itertools.chain.from_iterable( - parse_single_dependency(proj, attr, file_path) + parse_single_dependency(proj, attr, pyproject_toml) for proj, attr in {**dependencies, **dev_dependencies}.items() ) ) @@ -267,9 +333,8 @@ def __call__( ) requirements_dep = f":{req_file_tgt.name}" - req_file = Path(get_buildroot(), self._parse_context.rel_path, pyproject_toml_relpath) requirements = parse_pyproject_toml( - req_file.read_text(), str(req_file.relative_to(get_buildroot())) + PyProjectToml.create(self._parse_context, pyproject_toml_relpath) ) for parsed_req in requirements: proj_name = parsed_req.project_name diff --git a/src/python/pants/backend/python/macros/poetry_requirements_test.py b/src/python/pants/backend/python/macros/poetry_requirements_test.py index b31cd234bae..28e86c606e9 100644 --- a/src/python/pants/backend/python/macros/poetry_requirements_test.py +++ b/src/python/pants/backend/python/macros/poetry_requirements_test.py @@ -1,6 +1,9 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +from __future__ import annotations + +from pathlib import Path, PurePath from textwrap import dedent from typing import Any, Dict, Iterable @@ -10,6 +13,7 @@ from pants.backend.python.macros.poetry_requirements import ( PoetryRequirements, + PyProjectToml, get_max_caret, get_max_tilde, handle_dict_attr, @@ -80,11 +84,28 @@ def test_handle_str(test, exp) -> None: assert parse_str_version("foo", test, "") == f"foo {exp}" -def test_handle_git() -> None: +def create_pyproject_toml( + build_root: PurePath | str = ".", + toml_relpath: PurePath | str = "pyproject.toml", + toml_contents: str = "", +) -> PyProjectToml: + return PyProjectToml( + build_root=PurePath(build_root), + toml_relpath=PurePath(toml_relpath), + toml_contents=toml_contents, + ) + + +@pytest.fixture +def empty_pyproject_toml() -> PyProjectToml: + return create_pyproject_toml(toml_contents="") + + +def test_handle_git(empty_pyproject_toml: PyProjectToml) -> None: def assert_git(extra_opts: Dict[str, str], suffix: str) -> None: attr = {"git": "https://github.com/requests/requests.git", **extra_opts} assert ( - handle_dict_attr("requests", attr, "") + handle_dict_attr("requests", attr, empty_pyproject_toml) == f"requests @ git+https://github.com/requests/requests.git{suffix}" ) @@ -94,25 +115,71 @@ def assert_git(extra_opts: Dict[str, str], suffix: str) -> None: assert_git({"rev": "1a2b3c4d"}, "#1a2b3c4d") -def test_handle_path_arg() -> None: - attr = {"path": "../../my_py_proj.whl"} - assert handle_dict_attr("my_py_proj", attr, "") == "my_py_proj @ file://../../my_py_proj.whl" +def test_handle_path_arg(tmp_path: Path) -> None: + build_root = tmp_path / "build_root" + + one_level = Path("one") + one_pyproject_toml = create_pyproject_toml( + build_root=build_root, toml_relpath=one_level / "pyproject.toml" + ) + + two_level = one_level / "two" + two_pyproject_toml = create_pyproject_toml( + build_root=build_root, toml_relpath=two_level / "pyproject.toml" + ) + + (build_root / two_level).mkdir(parents=True) + + external_file = tmp_path / "my_py_proj.whl" + external_file.touch() + external_project = tmp_path / "my_py_proj" + external_project.mkdir() -def test_handle_url_arg() -> None: + internal_file = build_root / "my_py_proj.whl" + internal_file.touch() + + internal_project = build_root / "my_py_proj" + internal_project.mkdir() + + file_attr = {"path": "../../my_py_proj.whl"} + dir_attr = {"path": "../../my_py_proj"} + + assert ( + handle_dict_attr("my_py_proj", file_attr, one_pyproject_toml) + == f"my_py_proj @ file://{external_file}" + ) + + assert ( + handle_dict_attr("my_py_proj", file_attr, two_pyproject_toml) + == f"my_py_proj @ file://{internal_file}" + ) + + assert ( + handle_dict_attr("my_py_proj", dir_attr, one_pyproject_toml) + == f"my_py_proj @ file://{external_project}" + ) + + assert handle_dict_attr("my_py_proj", dir_attr, two_pyproject_toml) is None + + +def test_handle_url_arg(empty_pyproject_toml: PyProjectToml) -> None: attr = {"url": "https://my-site.com/mydep.whl"} - assert handle_dict_attr("my_py_proj", attr, "") == "my_py_proj @ https://my-site.com/mydep.whl" + assert ( + handle_dict_attr("my_py_proj", attr, empty_pyproject_toml) + == "my_py_proj @ https://my-site.com/mydep.whl" + ) -def test_version_only() -> None: +def test_version_only(empty_pyproject_toml: PyProjectToml) -> None: attr = {"version": "1.2.3"} - assert handle_dict_attr("foo", attr, "") == "foo ==1.2.3" + assert handle_dict_attr("foo", attr, empty_pyproject_toml) == "foo ==1.2.3" -def test_py_constraints() -> None: +def test_py_constraints(empty_pyproject_toml: PyProjectToml) -> None: def assert_py_constraints(py_req: str, suffix: str) -> None: attr = {"version": "1.2.3", "python": py_req} - assert handle_dict_attr("foo", attr, "") == f"foo ==1.2.3;{suffix}" + assert handle_dict_attr("foo", attr, empty_pyproject_toml) == f"foo ==1.2.3;{suffix}" assert_py_constraints("3.6", "python_version == '3.6'") assert_py_constraints("3.6 || 3.7", "(python_version == '3.6') or (python_version == '3.7')") @@ -123,13 +190,16 @@ def assert_py_constraints(py_req: str, suffix: str) -> None: ) assert_py_constraints( "~3.6 || ^3.7", - "(python_version >= '3.6' and python_version< '3.7') or (python_version >= '3.7' and python_version< '4.0')", + ( + "(python_version >= '3.6' and python_version< '3.7') or " + "(python_version >= '3.7' and python_version< '4.0')" + ), ) -def test_multi_version_const() -> None: +def test_multi_version_const(empty_pyproject_toml: PyProjectToml) -> None: lst_attr = [{"version": "1.2.3", "python": "3.6"}, {"version": "1.2.4", "python": "3.7"}] - retval = parse_single_dependency("foo", lst_attr, "") + retval = tuple(parse_single_dependency("foo", lst_attr, empty_pyproject_toml)) actual_reqs = ( Requirement.parse("foo ==1.2.3; python_version == '3.6'"), Requirement.parse("foo ==1.2.4; python_version == '3.7'"), @@ -146,10 +216,12 @@ def test_extended_form() -> None: markers = "platform_python_implementation == 'CPython'" [tool.poetry.dev-dependencies] """ - retval = parse_pyproject_toml(toml_black_str, "/path/to/file") + pyproject_toml_black = create_pyproject_toml(toml_contents=toml_black_str) + retval = parse_pyproject_toml(pyproject_toml_black) actual_req = { Requirement.parse( - 'black==19.10b0; platform_python_implementation == "CPython" and python_version == "3.6"' + "black==19.10b0; " + 'platform_python_implementation == "CPython" and python_version == "3.6"' ) } assert retval == actual_req @@ -181,7 +253,8 @@ def test_parse_multi_reqs() -> None: requires = ["poetry-core>=1.0.0"] build-backend = "poetry.core.masonry.api" """ - retval = parse_pyproject_toml(toml_str, "/path/to/file") + pyproject_toml = create_pyproject_toml(toml_contents=toml_str) + retval = parse_pyproject_toml(pyproject_toml) actual_reqs = { Requirement.parse("junk@ https://github.com/myrepo/junk.whl"), Requirement.parse("poetry@ git+https://github.com/python-poetry/poetry.git@v1.1.1"), @@ -189,7 +262,8 @@ def test_parse_multi_reqs() -> None: Requirement.parse('foo>=1.9; python_version >= "2.7" and python_version < "3.0"'), Requirement.parse('foo<3.0.0,>=2.0; python_version == "3.4" or python_version == "3.5"'), Requirement.parse( - 'black==19.10b0; platform_python_implementation == "CPython" and python_version == "3.6"' + "black==19.10b0; " + 'platform_python_implementation == "CPython" and python_version == "3.6"' ), Requirement.parse("isort<5.6,>=5.5.1"), } diff --git a/src/python/pants/base/parse_context.py b/src/python/pants/base/parse_context.py index eb4711411ac..17348200139 100644 --- a/src/python/pants/base/parse_context.py +++ b/src/python/pants/base/parse_context.py @@ -19,13 +19,16 @@ class ParseContext: in its `__init__`). """ - def __init__(self, type_aliases: Mapping[str, Any], rel_path_oracle: RelPathOracle) -> None: + def __init__( + self, build_root: str, type_aliases: Mapping[str, Any], rel_path_oracle: RelPathOracle + ) -> None: """Create a ParseContext. + :param build_root: The absolute path to the build root. :param type_aliases: A dictionary of BUILD file symbols. :param rel_path_oracle: An oracle than can be queried for the current BUILD file path. """ - + self._build_root = build_root self._type_aliases = type_aliases self._rel_path_oracle = rel_path_oracle @@ -56,3 +59,8 @@ def rel_path(self) -> str: :API: public """ return self._rel_path_oracle.rel_path() + + @property + def build_root(self) -> str: + """Absolute path of the build root.""" + return self._build_root diff --git a/src/python/pants/engine/internals/build_files_test.py b/src/python/pants/engine/internals/build_files_test.py index 88130825211..a32318a78f0 100644 --- a/src/python/pants/engine/internals/build_files_test.py +++ b/src/python/pants/engine/internals/build_files_test.py @@ -39,7 +39,7 @@ def test_parse_address_family_empty() -> None: af = run_rule_with_mocks( parse_address_family, rule_args=[ - Parser(target_type_aliases=[], object_aliases=BuildFileAliases()), + Parser(build_root="", target_type_aliases=[], object_aliases=BuildFileAliases()), create_subsystem(GlobalOptions, build_patterns=["BUILD"], build_ignore=[]), BuildFilePreludeSymbols(FrozenDict()), AddressFamilyDir("/dev/null"), diff --git a/src/python/pants/engine/internals/mapper_test.py b/src/python/pants/engine/internals/mapper_test.py index 6c3960a1af3..a9b7cbc1e9e 100644 --- a/src/python/pants/engine/internals/mapper_test.py +++ b/src/python/pants/engine/internals/mapper_test.py @@ -21,7 +21,7 @@ def parse_address_map(build_file: str) -> AddressMap: path = "/dev/null" - parser = Parser(target_type_aliases=["thing"], object_aliases=BuildFileAliases()) + parser = Parser(build_root="", target_type_aliases=["thing"], object_aliases=BuildFileAliases()) address_map = AddressMap.parse(path, build_file, parser, BuildFilePreludeSymbols(FrozenDict())) assert path == address_map.path return address_map diff --git a/src/python/pants/engine/internals/parser.py b/src/python/pants/engine/internals/parser.py index 97df7f46293..31deef4e173 100644 --- a/src/python/pants/engine/internals/parser.py +++ b/src/python/pants/engine/internals/parser.py @@ -59,14 +59,19 @@ def parsed_targets(self) -> list[TargetAdaptor]: class Parser: def __init__( - self, *, target_type_aliases: Iterable[str], object_aliases: BuildFileAliases + self, + *, + build_root: str, + target_type_aliases: Iterable[str], + object_aliases: BuildFileAliases, ) -> None: self._symbols, self._parse_state = self._generate_symbols( - target_type_aliases, object_aliases + build_root, target_type_aliases, object_aliases ) @staticmethod def _generate_symbols( + build_root: str, target_type_aliases: Iterable[str], object_aliases: BuildFileAliases, ) -> tuple[dict[str, Any], ParseState]: @@ -96,7 +101,9 @@ def __call__(self, **kwargs: Any) -> TargetAdaptor: symbols: dict[str, Any] = dict(object_aliases.objects) symbols.update((alias, Registrar(alias)) for alias in target_type_aliases) - parse_context = ParseContext(type_aliases=symbols, rel_path_oracle=parse_state) + parse_context = ParseContext( + build_root=build_root, type_aliases=symbols, rel_path_oracle=parse_state + ) for alias, object_factory in object_aliases.context_aware_object_factories.items(): symbols[alias] = object_factory(parse_context) diff --git a/src/python/pants/engine/internals/parser_test.py b/src/python/pants/engine/internals/parser_test.py index 06cecbc6da9..70a5d93b670 100644 --- a/src/python/pants/engine/internals/parser_test.py +++ b/src/python/pants/engine/internals/parser_test.py @@ -12,7 +12,7 @@ def test_imports_banned() -> None: - parser = Parser(target_type_aliases=[], object_aliases=BuildFileAliases()) + parser = Parser(build_root="", target_type_aliases=[], object_aliases=BuildFileAliases()) with pytest.raises(ParseError) as exc: parser.parse( "dir/BUILD", "\nx = 'hello'\n\nimport os\n", BuildFilePreludeSymbols(FrozenDict()) @@ -24,6 +24,7 @@ def test_unrecogonized_symbol() -> None: def perform_test(extra_targets: list[str], dym: str) -> None: parser = Parser( + build_root="", target_type_aliases=["tgt", *extra_targets], object_aliases=BuildFileAliases( objects={"obj": 0}, diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index febc287ac00..2a5e4d37b6c 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -210,7 +210,7 @@ def setup_graph_extended( native_engine_visualize_to: Optional[str] = None, watch_filesystem: bool = True, ) -> GraphScheduler: - build_root = build_root or get_buildroot() + build_root_path = build_root or get_buildroot() rules = build_configuration.rules union_membership = UnionMembership.from_rules(build_configuration.union_rules) @@ -221,6 +221,7 @@ def setup_graph_extended( @rule def parser_singleton() -> Parser: return Parser( + build_root=build_root_path, target_type_aliases=registered_target_types.aliases, object_aliases=build_configuration.registered_aliases, ) @@ -283,7 +284,7 @@ def ensure_optional_absolute_path(v: Optional[str]) -> Optional[str]: scheduler = Scheduler( ignore_patterns=pants_ignore_patterns, use_gitignore=use_gitignore, - build_root=build_root, + build_root=build_root_path, local_execution_root_dir=ensure_absolute_path(local_execution_root_dir), named_caches_dir=ensure_absolute_path(named_caches_dir), ca_certs_path=ensure_optional_absolute_path(ca_certs_path),