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

[internal] Parse addresses using a generated parser #14346

Merged
merged 2 commits into from
Feb 3, 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
60 changes: 29 additions & 31 deletions src/python/pants/build_graph/address.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -324,18 +313,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:
Expand All @@ -345,18 +342,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.
Expand Down
25 changes: 21 additions & 4 deletions src/python/pants/build_graph/address_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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",
"//:?",
"//:=",
Expand Down
9 changes: 9 additions & 0 deletions src/python/pants/engine/internals/native_engine.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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
# ------------------------------------------------------------------------------
Expand Down
8 changes: 8 additions & 0 deletions src/rust/engine/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/rust/engine/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"
Expand Down
9 changes: 9 additions & 0 deletions src/rust/engine/address/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
version = "0.0.1"
edition = "2021"
name = "address"
authors = [ "Pants Build <pantsbuild@gmail.com>" ]
publish = false

[dependencies]
peg = "0.7"
66 changes: 66 additions & 0 deletions src/rust/engine/address/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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<Mutex> 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 {
Comment on lines +34 to +35
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was really impressed with how easy this crate was to use: good choice @jsirois!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love PEG's. (first and only placed I've used/seen them is in Lua, which I think is where they originated, from Mr Roberto I)

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<AddressInput, String> {
let relative_address = relative_address_parser::relative_address(value)
.map_err(|e| format!("Failed to parse Address `{value}`: {e}"))?;

Ok(relative_address)
}
30 changes: 30 additions & 0 deletions src/rust/engine/src/externs/address.rs
Original file line number Diff line number Diff line change
@@ -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::<AddressParseException>(),
)?;
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))
}
1 change: 1 addition & 0 deletions src/rust/engine/src/externs/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
1 change: 1 addition & 0 deletions src/rust/engine/src/externs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down