Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for parsing address parameters to CLI specs parsing (cherrypick of #14949) #14957

Merged
merged 1 commit into from
Mar 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/python/pants/base/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior
from pants.engine.fs import GlobExpansionConjunction, PathGlobs
from pants.util.dirutil import fast_relpath_optional, recursive_dirname
from pants.util.frozendict import FrozenDict
from pants.util.meta import frozen_after_init


Expand Down Expand Up @@ -42,6 +43,7 @@ class AddressLiteralSpec(AddressSpec):
path_component: str
target_component: str | None = None
generated_component: str | None = None
parameters: FrozenDict[str, str] = FrozenDict()

def __str__(self) -> str:
tgt = f":{self.target_component}" if self.target_component else ""
Expand All @@ -51,7 +53,11 @@ def __str__(self) -> str:
@property
def is_directory_shorthand(self) -> bool:
"""Is in the format `path/to/dir`, which is shorthand for `path/to/dir:dir`."""
return self.target_component is None and self.generated_component is None
return (
self.target_component is None
and self.generated_component is None
and not self.parameters
)


@dataclass(frozen=True) # type: ignore[misc]
Expand Down Expand Up @@ -261,9 +267,7 @@ def to_glob(self) -> str:

def to_address_literal(self) -> AddressLiteralSpec:
"""For now, `dir` can also be shorthand for `dir:dir`."""
return AddressLiteralSpec(
path_component=self.v, target_component=None, generated_component=None
)
return AddressLiteralSpec(path_component=self.v)


@frozen_after_init
Expand Down
57 changes: 31 additions & 26 deletions src/python/pants/base/specs_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
SiblingAddresses,
Specs,
)
from pants.engine.internals import native_engine
from pants.util.frozendict import FrozenDict
from pants.util.ordered_set import OrderedSet


Expand Down Expand Up @@ -71,37 +73,40 @@ def parse_spec(self, spec: str) -> AddressSpec | FilesystemSpec:

:raises: CmdLineSpecParser.BadSpecError if the address selector could not be parsed.
"""
if spec.endswith("::"):
spec_path = spec[: -len("::")]
return DescendantAddresses(directory=self._normalize_spec_path(spec_path))
if spec.endswith(":"):
spec_path = spec[: -len(":")]
return SiblingAddresses(directory=self._normalize_spec_path(spec_path))
if ":" in spec or "#" in spec:
tgt_parts = spec.split(":", maxsplit=1)
path_component = tgt_parts[0]
if len(tgt_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 = tgt_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
(
is_ignored,
(
path_component,
target_component,
generated_component,
parameters,
),
wildcard,
) = native_engine.address_spec_parse(spec)

def assert_not_ignored(spec_descriptor: str) -> None:
if is_ignored:
raise self.BadSpecError(
f"The {spec_descriptor} spec `{spec}` does not support ignore (`!`) syntax."
)

if wildcard == "::":
assert_not_ignored("address wildcard")
return DescendantAddresses(directory=self._normalize_spec_path(path_component))
if wildcard == ":":
assert_not_ignored("address wildcard")
return SiblingAddresses(directory=self._normalize_spec_path(path_component))
if target_component or generated_component or parameters:
assert_not_ignored("address")
return AddressLiteralSpec(
path_component=self._normalize_spec_path(path_component),
target_component=target_component,
generated_component=generated_component,
parameters=FrozenDict(sorted(parameters)),
)
if spec.startswith("!"):
return FileIgnoreSpec(spec[1:])
if "*" in spec:
if is_ignored:
return FileIgnoreSpec(path_component)
if "*" in path_component:
return FileGlobSpec(spec)
if PurePath(spec).suffix:
return FileLiteralSpec(self._normalize_spec_path(spec))
Expand Down
13 changes: 11 additions & 2 deletions src/python/pants/base/specs_parser_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,18 @@
Spec,
)
from pants.base.specs_parser import SpecsParser
from pants.util.frozendict import FrozenDict


def address_literal(
directory: str, name: str | None, generated: str | None = None
directory: str,
name: str | None = None,
generated: str | None = None,
parameters: dict[str, str] | None = None,
) -> AddressLiteralSpec:
return AddressLiteralSpec(directory, name, generated)
return AddressLiteralSpec(
directory, name, generated, FrozenDict(sorted(parameters.items()) if parameters else ())
)


def desc(directory: str) -> DescendantAddresses:
Expand Down Expand Up @@ -66,6 +72,9 @@ def assert_spec_parsed(build_root: Path, spec_str: str, expected_spec: Spec) ->
(":root", address_literal("", "root")),
("//:root", address_literal("", "root")),
("a", dir_literal("a")),
("a@k=v", address_literal("a", parameters={"k": "v"})),
("a@k=v,x=y", address_literal("a", parameters={"k": "v", "x": "y"})),
("a:b@k=v", address_literal("a", "b", parameters={"k": "v"})),
("a:a", address_literal("a", "a")),
("a/b", dir_literal("a/b")),
("a/b:b", address_literal("a/b", "b")),
Expand Down
34 changes: 29 additions & 5 deletions src/python/pants/build_graph/address.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ class InvalidParameters(InvalidAddress):
"""Indicate invalid parameter values for `Address`."""


class UnsupportedIgnore(InvalidAddress):
"""Indicate that a `!` ignore was used."""


class UnsupportedWildcard(InvalidAddress):
"""Indicate that an address wildcard was used."""


@dataclass(frozen=True)
class AddressInput:
"""A string that has been parsed and normalized using the Address syntax.
Expand Down Expand Up @@ -149,11 +157,27 @@ def prefix_subproject(spec_path: str) -> str:
return os.path.normpath(subproject)

(
path_component,
target_component,
generated_component,
parameters,
) = native_engine.address_parse(spec)
is_ignored,
(
path_component,
target_component,
generated_component,
parameters,
),
wildcard,
) = native_engine.address_spec_parse(spec)

if is_ignored:
# NB: BUILD dependency ignore parsing occurs at a different level, because AddressInput
# does not support encoding negation.
raise UnsupportedIgnore(
f"The address `{spec}` was prefixed with a `!` ignore, which is not supported."
)

if wildcard:
raise UnsupportedWildcard(
f"The address `{spec}` included a wildcard (`{wildcard}`), which is not supported."
)

normalized_relative_to = None
if relative_to:
Expand Down
33 changes: 31 additions & 2 deletions src/python/pants/build_graph/address_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
InvalidParameters,
InvalidSpecPath,
InvalidTargetName,
UnsupportedIgnore,
UnsupportedWildcard,
)
from pants.util.frozendict import FrozenDict

Expand Down Expand Up @@ -110,7 +112,6 @@ def test_address_input_parse_bad_path_component(spec: str) -> None:
@pytest.mark.parametrize(
"spec,expected",
[
("a:", "non-empty target name"),
("a@t", "one or more key=value pairs"),
("a@=", "one or more key=value pairs"),
("a@t,y", "one or more key=value pairs"),
Expand All @@ -128,7 +129,6 @@ def test_address_input_parse(spec: str, expected: str) -> None:
"spec",
[
"",
"a::",
"//",
"//:!t",
"//:?",
Expand All @@ -143,6 +143,35 @@ def test_address_bad_target_component(spec: str) -> None:
AddressInput.parse(spec).dir_to_address()


@pytest.mark.parametrize(
"spec",
[
"!",
"!a",
"!a:x",
"!a#x",
],
)
def test_address_bad_ignore(spec: str) -> None:
with pytest.raises(UnsupportedIgnore):
AddressInput.parse(spec).dir_to_address()


@pytest.mark.parametrize(
"spec",
[
"a::",
"a:",
"a:b:",
"a:b::",
"a#b:",
],
)
def test_address_bad_wildcard(spec: str) -> None:
with pytest.raises(UnsupportedWildcard):
AddressInput.parse(spec).dir_to_address()


@pytest.mark.parametrize("spec", ["//:t#gen!", "//:t#gen?", "//:t#gen=", "//:t#gen#"])
def test_address_generated_name(spec: str) -> None:
with pytest.raises(InvalidTargetName):
Expand Down
7 changes: 6 additions & 1 deletion src/python/pants/engine/internals/build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,12 @@ async def addresses_from_address_specs(
literal_wrapped_targets = await MultiGet(
Get(
WrappedTarget,
AddressInput(spec.path_component, spec.target_component, spec.generated_component),
AddressInput(
spec.path_component,
spec.target_component,
spec.generated_component,
spec.parameters,
),
)
for spec in address_specs.literals
)
Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/engine/internals/native_engine.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ from pants.engine.process import InteractiveProcessResult
class AddressParseException(Exception):
pass

def address_parse(spec: str) -> tuple[str, str | None, str | None, tuple[tuple[str, str], ...]]: ...
def address_spec_parse(
spec: str,
) -> tuple[bool, tuple[str, str | None, str | None, tuple[tuple[str, str], ...]], str | None]: ...

# ------------------------------------------------------------------------------
# Scheduler
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/init/specs_calculator.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def calculate_specs(
path_component=address_input.path_component,
target_component=address_input.target_component,
generated_component=address_input.generated_component,
parameters=address_input.parameters,
)
)
return Specs(AddressSpecs(address_specs, filter_by_global_options=True), FilesystemSpecs([]))
Expand Down
44 changes: 33 additions & 11 deletions src/rust/engine/address/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,30 @@ pub struct AddressInput<'a> {
pub parameters: Vec<(&'a str, &'a str)>,
}

pub struct SpecInput<'a> {
/// True if the spec started with an `!`.
pub is_ignored: bool,
/// The address (or literal, if no target/generated/parameters were specified) portion.
pub address: AddressInput<'a>,
/// If a spec wildcard was specified (`:` or `::`), its value.
pub wildcard: Option<&'a str>,
}

peg::parser! {
grammar relative_address_parser() for str {
grammar parsers() for str {
rule path() -> &'input str = s:$([^':' | '@' | '#']*) { s }

rule target_name() -> &'input str
= quiet!{ s:$([^'#' | '@']+) { s } }
= quiet!{ s:$([^'#' | '@' | ':']+) { s } }
/ expected!("a non-empty target name to follow a `:`.")

rule target() -> &'input str = ":" s:target_name() { s }
rule target() -> &'input str =
// NB: We use `&[_]` to differentiate from a wildcard by ensuring that a non-EOF
// character follows the `:`.
":" &[_] s:target_name() { s }

rule generated_name() -> &'input str
= quiet!{ s:$([^'@']+) { s } }
= quiet!{ s:$([^'@' | ':']+) { s } }
/ expected!("a non-empty generated target name to follow a `#`.")

rule generated() -> &'input str = "#" s:generated_name() { s }
Expand All @@ -52,10 +64,10 @@ peg::parser! {
= "@" parameters:parameter() ++ "," { parameters }

rule parameter() -> (&'input str, &'input str)
= quiet!{ key:$([^'=']+) "=" value:$([^',']*) { (key, value) } }
= quiet!{ key:$([^'=' | ':']+) "=" value:$([^',' | ':']*) { (key, value) } }
/ expected!("one or more key=value pairs to follow a `@`.")

pub(crate) rule relative_address() -> AddressInput<'input>
rule address() -> AddressInput<'input>
= path:path() target:target()? generated:generated()? parameters:parameters()? {
AddressInput {
path,
Expand All @@ -64,12 +76,22 @@ peg::parser! {
parameters: parameters.unwrap_or_else(Vec::new),
}
}

rule ignore() -> () = "!" {}

rule wildcard() -> &'input str = s:$("::" / ":") { s }

pub(crate) rule spec() -> SpecInput<'input>
= is_ignored:ignore()? address:address() wildcard:wildcard()? {
SpecInput {
is_ignored: is_ignored == Some(()),
address,
wildcard,
}
}
}
}

pub fn parse_address(value: &str) -> Result<AddressInput, String> {
let relative_address = relative_address_parser::relative_address(value)
.map_err(|e| format!("Failed to parse Address `{value}`: {e}"))?;

Ok(relative_address)
pub fn parse_address_spec(value: &str) -> Result<SpecInput, String> {
parsers::spec(value).map_err(|e| format!("Failed to parse address spec `{value}`: {e}"))
}
Loading