From 4f6584d9f7b9b1e63a000b3d0869104938b2bd5b Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sun, 4 Jul 2021 16:39:49 -0700 Subject: [PATCH 1/4] Refactor BUILD file parsing. Move to parse state storage from ParseContext to parser where it belongs. This cleans up the conceptual model a bit and allows for typing without untoward dependencies. # 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/base/parse_context.py | 51 ++++++-------- src/python/pants/engine/internals/parser.py | 73 ++++++++++++++------- 2 files changed, 68 insertions(+), 56 deletions(-) diff --git a/src/python/pants/base/parse_context.py b/src/python/pants/base/parse_context.py index eed5edda4f8..eb4711411ac 100644 --- a/src/python/pants/base/parse_context.py +++ b/src/python/pants/base/parse_context.py @@ -1,23 +1,14 @@ # Copyright 2014 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -import threading +from typing import Any, Mapping +from typing_extensions import Protocol -class Storage(threading.local): - def __init__(self, rel_path): - self.clear(rel_path) - def clear(self, rel_path): - self.rel_path = rel_path - self.objects_by_name = dict() - self.objects = [] - - def add(self, obj, name=None): - if name is not None: - # NB: `src/python/pants/engine/mapper.py` will detect an overwritten object later. - self.objects_by_name[name] = obj - self.objects.append(obj) +class RelPathOracle(Protocol): + def rel_path(self) -> str: + ... class ParseContext: @@ -28,42 +19,40 @@ class ParseContext: in its `__init__`). """ - def __init__(self, rel_path, type_aliases): + def __init__(self, type_aliases: Mapping[str, Any], rel_path_oracle: RelPathOracle) -> None: """Create a ParseContext. - :param rel_path: The (build file) path that the parse is currently operating on: initially None. - :param type_aliases: A dictionary of alias name strings or alias classes to a callable - constructor for the alias. + :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._type_aliases = type_aliases - self._storage = Storage(rel_path) + self._rel_path_oracle = rel_path_oracle - def create_object(self, alias, *args, **kwargs): + def create_object(self, alias: str, *args: Any, **kwargs: Any) -> Any: """Constructs the type with the given alias using the given args and kwargs. - NB: aliases may be the alias' object type itself if that type is known. - :API: public - :param alias: Either the type alias or the type itself. - :type alias: string|type + :param alias: The type alias. :param args: These pass through to the underlying callable object. :param kwargs: These pass through to the underlying callable object. :returns: The created object. """ object_type = self._type_aliases.get(alias) if object_type is None: - raise KeyError("There is no type registered for alias {0}".format(alias)) + raise KeyError(f"There is no type registered for alias {alias}") + if not callable(object_type): + raise TypeError( + f"Asked to call {alias} with args {args} and kwargs {kwargs} but it is not " + f"callable, its a {type(alias).__name__}." + ) return object_type(*args, **kwargs) @property - def rel_path(self): - """Relative path from the build root to the BUILD file the context aware object is called - in. + def rel_path(self) -> str: + """Relative path from the build root to the BUILD file being parsed. :API: public - - :rtype string """ - return self._storage.rel_path + return self._rel_path_oracle.rel_path() diff --git a/src/python/pants/engine/internals/parser.py b/src/python/pants/engine/internals/parser.py index 37bec51c8f5..97df7f46293 100644 --- a/src/python/pants/engine/internals/parser.py +++ b/src/python/pants/engine/internals/parser.py @@ -1,12 +1,15 @@ # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +from __future__ import annotations + import os.path +import threading import tokenize from dataclasses import dataclass from difflib import get_close_matches from io import StringIO -from typing import Any, Dict, Iterable, List, Tuple, cast +from typing import Any, Iterable from pants.base.exceptions import MappingError from pants.base.parse_context import ParseContext @@ -29,11 +32,36 @@ class UnaddressableObjectError(MappingError): """Indicates an un-addressable object was found at the top level.""" +class ParseState(threading.local): + def __init__(self): + self._rel_path: str | None = None + self._target_adapters: list[TargetAdaptor] = [] + + def reset(self, rel_path: str) -> None: + self._rel_path = rel_path + self._target_adapters.clear() + + def add(self, target_adapter: TargetAdaptor) -> None: + self._target_adapters.append(target_adapter) + + def rel_path(self) -> str: + if self._rel_path is None: + raise AssertionError( + "The BUILD file rel_path was accessed before being set. This indicates a " + "programming error in Pants. Please file a bug report at " + "https://github.com/pantsbuild/pants/issues/new." + ) + return self._rel_path + + def parsed_targets(self) -> list[TargetAdaptor]: + return list(self._target_adapters) + + class Parser: def __init__( self, *, target_type_aliases: Iterable[str], object_aliases: BuildFileAliases ) -> None: - self._symbols, self._parse_context = self._generate_symbols( + self._symbols, self._parse_state = self._generate_symbols( target_type_aliases, object_aliases ) @@ -41,48 +69,43 @@ def __init__( def _generate_symbols( target_type_aliases: Iterable[str], object_aliases: BuildFileAliases, - ) -> Tuple[Dict[str, Any], ParseContext]: - symbols: Dict[str, Any] = {} - - # Compute "per path" symbols. For performance, we use the same ParseContext, which we - # mutate to set the rel_path appropriately before it's actually used. This allows this - # method to reuse the same symbols for all parses. Meanwhile, we set the rel_path to None, - # so that we get a loud error if anything tries to use it before it's set. - # TODO: See https://github.com/pantsbuild/pants/issues/3561 - parse_context = ParseContext(rel_path=None, type_aliases=symbols) + ) -> tuple[dict[str, Any], ParseState]: + # N.B.: We re-use the thread local ParseState across symbols for performance reasons. + # This allows a single construction of all symbols here that can be re-used for each BUILD + # file parse with a reset of the ParseState for the calling thread. + parse_state = ParseState() class Registrar: - def __init__(self, parse_context: ParseContext, type_alias: str): - self._parse_context = parse_context + def __init__(self, type_alias: str) -> None: self._type_alias = type_alias - def __call__(self, *args, **kwargs): + def __call__(self, **kwargs: Any) -> TargetAdaptor: # Target names default to the name of the directory their BUILD file is in # (as long as it's not the root directory). if "name" not in kwargs: - dirname = os.path.basename(self._parse_context.rel_path) + dirname = os.path.basename(parse_state.rel_path()) if not dirname: raise UnaddressableObjectError( "Targets in root-level BUILD files must be named explicitly." ) kwargs["name"] = dirname - kwargs.setdefault("type_alias", self._type_alias) - target_adaptor = TargetAdaptor(**kwargs) - self._parse_context._storage.add(target_adaptor) + target_adaptor = TargetAdaptor(self._type_alias, **kwargs) + parse_state.add(target_adaptor) return target_adaptor - symbols.update({alias: Registrar(parse_context, alias) for alias in target_type_aliases}) - symbols.update(object_aliases.objects) + 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) for alias, object_factory in object_aliases.context_aware_object_factories.items(): symbols[alias] = object_factory(parse_context) - return symbols, parse_context + return symbols, parse_state def parse( self, filepath: str, build_file_content: str, extra_symbols: BuildFilePreludeSymbols - ) -> List[TargetAdaptor]: - # Mutate the parse context with the new path. - self._parse_context._storage.clear(os.path.dirname(filepath)) + ) -> list[TargetAdaptor]: + self._parse_state.reset(rel_path=os.path.dirname(filepath)) # We update the known symbols with Build File Preludes. This is subtle code; functions have # their own globals set on __globals__ which they derive from the environment where they @@ -124,7 +147,7 @@ def parse( error_on_imports(build_file_content, filepath) - return cast(List[TargetAdaptor], list(self._parse_context._storage.objects)) + return self._parse_state.parsed_targets() def error_on_imports(build_file_content: str, filepath: str) -> None: From b7deacb0f61abaae78d18e4d2973fed24d08fb4b Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sun, 4 Jul 2021 19:29:49 -0700 Subject: [PATCH 2/4] Fix poetry_requirements: ignore internal projects. Previously dependencies with paths were assumed to be non-Pants controlled projects and the resulting requirements were not generate with absolute file URLs. Fix both issues by assuming all paths that point to directories inside the build root are Pants controlled projects and treating the rest as absolute file URL requirements. Fixes #12272 [ci skip-rust] [ci skip-build-wheels] --- .../python/macros/poetry_requirements.py | 157 ++++++++++++------ .../python/macros/poetry_requirements_test.py | 110 ++++++++++-- src/python/pants/base/parse_context.py | 12 +- .../engine/internals/build_files_test.py | 2 +- .../pants/engine/internals/mapper_test.py | 2 +- src/python/pants/engine/internals/parser.py | 13 +- .../pants/engine/internals/parser_test.py | 3 +- src/python/pants/init/engine_initializer.py | 5 +- 8 files changed, 229 insertions(+), 75 deletions(-) diff --git a/src/python/pants/backend/python/macros/poetry_requirements.py b/src/python/pants/backend/python/macros/poetry_requirements.py index 25c432c6bbf..e32bed27ffc 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,66 @@ 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(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. + 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 +180,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}" + external_path = pyproject_toml.non_pants_project_abs_path(path_lookup) + if external_path: + return f"{proj_name} @ file://{external_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 +205,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 +259,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 +331,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), From 256e4daa778be3cf1a61ddd2710ac29f7972a311 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Sun, 4 Jul 2021 20:57:18 -0700 Subject: [PATCH 3/4] Fix PyProjectToml.toml_relpath. # 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] --- .../backend/python/macros/poetry_requirements.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/python/pants/backend/python/macros/poetry_requirements.py b/src/python/pants/backend/python/macros/poetry_requirements.py index e32bed27ffc..cb0ee6c5235 100644 --- a/src/python/pants/backend/python/macros/poetry_requirements.py +++ b/src/python/pants/backend/python/macros/poetry_requirements.py @@ -117,7 +117,7 @@ class PyProjectToml: @classmethod def create(cls, parse_context: ParseContext, pyproject_toml_relpath: str) -> PyProjectToml: build_root = Path(parse_context.build_root) - toml_relpath = PurePath(pyproject_toml_relpath) + toml_relpath = PurePath(parse_context.rel_path, pyproject_toml_relpath) return cls( build_root=build_root, toml_relpath=toml_relpath, @@ -155,7 +155,9 @@ def non_pants_project_abs_path(self, path: str) -> Path | None: # 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. + # 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) @@ -183,9 +185,9 @@ def produce_match(sep: str, feat: Optional[str]) -> str: path_lookup = attributes.get("path") if path_lookup is not None: - external_path = pyproject_toml.non_pants_project_abs_path(path_lookup) - if external_path: - return f"{proj_name} @ file://{external_path}" + 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 From 7999d7f3c2c11c7d78a97ce3b473781fa037696c Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 7 Jul 2021 08:58:49 -0700 Subject: [PATCH 4/4] Fix bad merge # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/base/parse_context.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/python/pants/base/parse_context.py b/src/python/pants/base/parse_context.py index c6b51403d51..17348200139 100644 --- a/src/python/pants/base/parse_context.py +++ b/src/python/pants/base/parse_context.py @@ -24,8 +24,6 @@ def __init__( ) -> None: """Create a ParseContext. - :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. :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.