From c5588621fd4082298c751ee74cad93ead0ea6290 Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Wed, 2 Feb 2022 16:23:05 -0800 Subject: [PATCH 1/2] Simplify `Address.spec`. [ci skip-build-wheels] [ci skip-rust] --- src/python/pants/build_graph/address.py | 39 +++++++++++++++---------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/python/pants/build_graph/address.py b/src/python/pants/build_graph/address.py index ff1f0d4846c..282a68ca073 100644 --- a/src/python/pants/build_graph/address.py +++ b/src/python/pants/build_graph/address.py @@ -324,18 +324,26 @@ def spec(self) -> str: :API: public """ prefix = "//" if not self.spec_path else "" - if self._relative_file_path is not None: - file_portion = f"{prefix}{self.filename}" + if self._relative_file_path is None: + path = self.spec_path + target = "" if self._target_name is None and self.generated_name else self.target_name + else: + path = self.filename parent_prefix = "../" * self._relative_file_path.count(os.path.sep) - return ( - file_portion + target = ( + "" if self._target_name is None and not parent_prefix - else f"{file_portion}:{parent_prefix}{self.target_name}" + else f"{parent_prefix}{self.target_name}" ) - if self.generated_name is not None: - target_portion = f":{self._target_name}" if self._target_name is not None else "" - return f"{prefix}{self.spec_path}{target_portion}#{self.generated_name}" - return f"{prefix}{self.spec_path}:{self.target_name}" + target_sep = ":" if target else "" + if self.generated_name is None: + generated_sep = "" + generated = "" + else: + generated_sep = "#" + generated = self.generated_name + + return f"{prefix}{path}{target_sep}{target}{generated_sep}{generated}" @property def path_safe_spec(self) -> str: @@ -345,18 +353,19 @@ def path_safe_spec(self) -> str: if self._relative_file_path: parent_count = self._relative_file_path.count(os.path.sep) parent_prefix = "@" * parent_count if parent_count else "." - file_portion = f".{self._relative_file_path.replace(os.path.sep, '.')}" + path = f".{self._relative_file_path.replace(os.path.sep, '.')}" else: parent_prefix = "." - file_portion = "" + path = "" if parent_prefix == ".": - target_portion = f"{parent_prefix}{self._target_name}" if self._target_name else "" + target = f"{parent_prefix}{self._target_name}" if self._target_name else "" else: - target_portion = f"{parent_prefix}{self.target_name}" - generated_portion = ( + target = f"{parent_prefix}{self.target_name}" + generated = ( f"@{self.generated_name.replace(os.path.sep, '.')}" if self.generated_name else "" ) - return f"{self.spec_path.replace(os.path.sep, '.')}{file_portion}{target_portion}{generated_portion}" + prefix = self.spec_path.replace(os.path.sep, ".") + return f"{prefix}{path}{target}{generated}" def maybe_convert_to_target_generator(self) -> Address: """If this address is generated, convert it to its generator target. From 8d5a4da63f667cc38d5f579b5e9e82a66287826f Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Wed, 2 Feb 2022 11:39:36 -0800 Subject: [PATCH 2/2] Parse Addresses with a generated `peg` parser. [ci skip-build-wheels] --- src/python/pants/build_graph/address.py | 21 ++---- src/python/pants/build_graph/address_test.py | 25 +++++-- .../pants/engine/internals/native_engine.pyi | 9 +++ src/rust/engine/Cargo.lock | 8 +++ src/rust/engine/Cargo.toml | 3 + src/rust/engine/address/Cargo.toml | 9 +++ src/rust/engine/address/src/lib.rs | 66 +++++++++++++++++++ src/rust/engine/src/externs/address.rs | 30 +++++++++ src/rust/engine/src/externs/interface.rs | 1 + src/rust/engine/src/externs/mod.rs | 1 + 10 files changed, 153 insertions(+), 20 deletions(-) create mode 100644 src/rust/engine/address/Cargo.toml create mode 100644 src/rust/engine/address/src/lib.rs create mode 100644 src/rust/engine/src/externs/address.rs diff --git a/src/python/pants/build_graph/address.py b/src/python/pants/build_graph/address.py index 282a68ca073..5cbe9d2c5e6 100644 --- a/src/python/pants/build_graph/address.py +++ b/src/python/pants/build_graph/address.py @@ -9,6 +9,10 @@ from typing import Any, Sequence from pants.engine.engine_aware import EngineAwareParameter +from pants.engine.internals import native_engine +from pants.engine.internals.native_engine import ( # noqa: F401 + AddressParseException as AddressParseException, +) from pants.util.dirutil import fast_relpath, longest_dir_prefix from pants.util.strutil import strip_prefix @@ -127,22 +131,7 @@ def prefix_subproject(spec_path: str) -> str: return os.path.join(subproject, spec_path) return os.path.normpath(subproject) - spec_parts = spec.split(":", maxsplit=1) - path_component = spec_parts[0] - if len(spec_parts) == 1: - target_component = None - generated_parts = path_component.split("#", maxsplit=1) - if len(generated_parts) == 1: - generated_component = None - else: - path_component, generated_component = generated_parts - else: - generated_parts = spec_parts[1].split("#", maxsplit=1) - if len(generated_parts) == 1: - target_component = generated_parts[0] - generated_component = None - else: - target_component, generated_component = generated_parts + path_component, target_component, generated_component = native_engine.address_parse(spec) normalized_relative_to = None if relative_to: diff --git a/src/python/pants/build_graph/address_test.py b/src/python/pants/build_graph/address_test.py index 99544197351..81547fdf32d 100644 --- a/src/python/pants/build_graph/address_test.py +++ b/src/python/pants/build_graph/address_test.py @@ -5,7 +5,13 @@ import pytest -from pants.build_graph.address import Address, AddressInput, InvalidSpecPath, InvalidTargetName +from pants.build_graph.address import ( + Address, + AddressInput, + AddressParseException, + InvalidSpecPath, + InvalidTargetName, +) def test_address_input_parse_spec() -> None: @@ -90,15 +96,26 @@ def test_address_input_parse_bad_path_component(spec: str) -> None: AddressInput.parse(spec) +@pytest.mark.parametrize( + "spec,expected", + [ + ("a:", "non-empty target name"), + ("//:", "non-empty target name"), + ("//:@t", "non-empty target name"), + ], +) +def test_address_input_parse(spec: str, expected: str) -> None: + with pytest.raises(AddressParseException) as e: + AddressInput.parse(spec) + assert expected in str(e.value) + + @pytest.mark.parametrize( "spec", [ "", - "a:", "a::", "//", - "//:", - "//:@t", "//:!t", "//:?", "//:=", diff --git a/src/python/pants/engine/internals/native_engine.pyi b/src/python/pants/engine/internals/native_engine.pyi index cf2ccf4b991..9640d5c621a 100644 --- a/src/python/pants/engine/internals/native_engine.pyi +++ b/src/python/pants/engine/internals/native_engine.pyi @@ -17,6 +17,15 @@ from pants.engine.process import InteractiveProcessResult # see https://github.com/psf/black/issues/1548 # flake8: noqa: E302 +# ------------------------------------------------------------------------------ +# Address (parsing) +# ------------------------------------------------------------------------------ + +class AddressParseException(Exception): + pass + +def address_parse(spec: str) -> tuple[str, str | None, str | None]: ... + # ------------------------------------------------------------------------------ # Scheduler # ------------------------------------------------------------------------------ diff --git a/src/rust/engine/Cargo.lock b/src/rust/engine/Cargo.lock index 0ac9d360afa..4d61ded8351 100644 --- a/src/rust/engine/Cargo.lock +++ b/src/rust/engine/Cargo.lock @@ -2,6 +2,13 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "address" +version = "0.0.1" +dependencies = [ + "peg", +] + [[package]] name = "adler" version = "0.2.3" @@ -619,6 +626,7 @@ dependencies = [ name = "engine" version = "0.0.1" dependencies = [ + "address", "async-oncecell", "async-trait", "async_latch", diff --git a/src/rust/engine/Cargo.toml b/src/rust/engine/Cargo.toml index 46fc1127f88..e61c1eb6fc5 100644 --- a/src/rust/engine/Cargo.toml +++ b/src/rust/engine/Cargo.toml @@ -20,6 +20,7 @@ resolver = "2" # (e.g. fs_util) won't be included when we build/test things. members = [ ".", + "address", "async_latch", "async_value", "cache", @@ -60,6 +61,7 @@ members = [ # On Ubuntu, that means installing libfuse-dev. On OSX, that means installing OSXFUSE. default-members = [ ".", + "address", "async_latch", "async_value", "cache", @@ -102,6 +104,7 @@ extension-module = ["pyo3/extension-module"] default = [] [dependencies] +address = { path = "address" } async_latch = { path = "async_latch" } # Pin async-trait due to https://github.com/dtolnay/async-trait/issues/144. async-trait = "=0.1.42" diff --git a/src/rust/engine/address/Cargo.toml b/src/rust/engine/address/Cargo.toml new file mode 100644 index 00000000000..5818c4b6dcb --- /dev/null +++ b/src/rust/engine/address/Cargo.toml @@ -0,0 +1,9 @@ +[package] +version = "0.0.1" +edition = "2021" +name = "address" +authors = [ "Pants Build " ] +publish = false + +[dependencies] +peg = "0.7" diff --git a/src/rust/engine/address/src/lib.rs b/src/rust/engine/address/src/lib.rs new file mode 100644 index 00000000000..7a635be6f5d --- /dev/null +++ b/src/rust/engine/address/src/lib.rs @@ -0,0 +1,66 @@ +// Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +// Licensed under the Apache License, Version 2.0 (see LICENSE). + +#![deny(warnings)] +// Enable all clippy lints except for many of the pedantic ones. It's a shame this needs to be copied and pasted across crates, but there doesn't appear to be a way to include inner attributes from a common source. +#![deny( + clippy::all, + clippy::default_trait_access, + clippy::expl_impl_clone_on_copy, + clippy::if_not_else, + clippy::needless_continue, + clippy::unseparated_literal_suffix, + clippy::used_underscore_binding +)] +// It is often more clear to show that nothing is being moved. +#![allow(clippy::match_ref_pats)] +// Subjective style. +#![allow( + clippy::len_without_is_empty, + clippy::redundant_field_names, + clippy::too_many_arguments +)] +// Default isn't as big a deal as people seem to think it is. +#![allow(clippy::new_without_default, clippy::new_ret_no_self)] +// Arc can be more clear than needing to grok Orderings: +#![allow(clippy::mutex_atomic)] + +pub struct AddressInput<'a> { + pub path: &'a str, + pub target: Option<&'a str>, + pub generated: Option<&'a str>, +} + +peg::parser! { + grammar relative_address_parser() for str { + rule path() -> &'input str = s:$([^':' | '#']*) {s} + + rule target_name() -> &'input str + = quiet!{ s:$([^'#' | '@']+) { s } } + / expected!("a non-empty target name to follow a `:`.") + + rule target() -> &'input str = ":" s:target_name() { s } + + rule generated_name() -> &'input str + = quiet!{ s:$([_]+) { s } } + / expected!("a non-empty generated target name to follow a `#`.") + + rule generated() -> &'input str = "#" s:generated_name() { s } + + pub(crate) rule relative_address() -> AddressInput<'input> + = path:path() target:target()? generated:generated()? { + AddressInput { + path, + target, + generated, + } + } + } +} + +pub fn parse_address(value: &str) -> Result { + let relative_address = relative_address_parser::relative_address(value) + .map_err(|e| format!("Failed to parse Address `{value}`: {e}"))?; + + Ok(relative_address) +} diff --git a/src/rust/engine/src/externs/address.rs b/src/rust/engine/src/externs/address.rs new file mode 100644 index 00000000000..6346c76bdfd --- /dev/null +++ b/src/rust/engine/src/externs/address.rs @@ -0,0 +1,30 @@ +// Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +// Licensed under the Apache License, Version 2.0 (see LICENSE). + +use pyo3::create_exception; +use pyo3::exceptions::PyException; +use pyo3::prelude::*; + +use address::parse_address; + +create_exception!(native_engine, AddressParseException, PyException); + +pub fn register(py: Python, m: &PyModule) -> PyResult<()> { + m.add( + "AddressParseException", + py.get_type::(), + )?; + m.add_function(wrap_pyfunction!(address_parse, m)?)?; + Ok(()) +} + +/// Parses an Address spec into: +/// 1. a path component +/// 2. a target component +/// 3. a generated component +/// +#[pyfunction] +fn address_parse(spec: &str) -> PyResult<(&str, Option<&str>, Option<&str>)> { + let address = parse_address(spec).map_err(AddressParseException::new_err)?; + Ok((address.path, address.target, address.generated)) +} diff --git a/src/rust/engine/src/externs/interface.rs b/src/rust/engine/src/externs/interface.rs index 83759fe8f9b..133f5ca4bc6 100644 --- a/src/rust/engine/src/externs/interface.rs +++ b/src/rust/engine/src/externs/interface.rs @@ -53,6 +53,7 @@ use crate::{ #[pymodule] fn native_engine(py: Python, m: &PyModule) -> PyO3Result<()> { + externs::address::register(py, m)?; externs::fs::register(m)?; externs::nailgun::register(py, m)?; externs::scheduler::register(m)?; diff --git a/src/rust/engine/src/externs/mod.rs b/src/rust/engine/src/externs/mod.rs index 7097cc5b997..3713ad720fb 100644 --- a/src/rust/engine/src/externs/mod.rs +++ b/src/rust/engine/src/externs/mod.rs @@ -20,6 +20,7 @@ use logging::PythonLogLevel; use crate::interning::Interns; use crate::python::{Failure, Key, TypeId, Value}; +mod address; pub mod engine_aware; pub mod fs; mod interface;