Skip to content

Commit

Permalink
Pass GoalInfo through to the Rust ArgSplitter. (#21834)
Browse files Browse the repository at this point in the history
Previously we only passed the scope name. Now we
pass more info about each goal scope.

A follow up change will actually use this info to
replace logic currently implemented in Python.
  • Loading branch information
benjyw authored Jan 15, 2025
1 parent b9587ac commit e061067
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 35 deletions.
9 changes: 7 additions & 2 deletions src/python/pants/engine/internals/native_engine.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,11 @@ class PantsdClientException(Exception):
# Options
# ------------------------------------------------------------------------------

class PyGoalInfo:
def __init__(
self, scope_name: str, is_builtin: bool, is_auxiliary: bool, aliases: tuple[str, ...]
) -> None: ...

class PyOptionId:
def __init__(
self, *components: str, scope: str | None = None, switch: str | None = None
Expand All @@ -669,8 +674,8 @@ class PySplitArgs:
def passthru(self) -> list[str]: ...

class PyArgSplitter:
def __init__(self, build_root: str, known_goals: list[str]) -> None: ...
def split_args(self, args: list[str]) -> PySplitArgs: ...
def __init__(self, build_root: str, known_goals: tuple[PyGoalInfo, ...]) -> None: ...
def split_args(self, args: tuple[str, ...]) -> PySplitArgs: ...

class PyConfigSource:
def __init__(self, path: str, content: bytes) -> None: ...
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/goal/auxiliary_goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class AuxiliaryGoal(ABC, GoalSubsystem):
which must run "outside" of the usual engine processing to function.
"""

# Used by `pants.option.arg_splitter.ArgSplitter()` to optionally allow aliasing auxiliary goals.
# Used by the arg splitter to optionally allow aliasing auxiliary goals.
aliases: ClassVar[tuple[str, ...]] = ()

@classmethod
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/goal/builtin_goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class BuiltinGoal(ABC, GoalSubsystem):
prioritized. This is to support things like `./pants some-builtin-goal --help`.
"""

# Used by `pants.option.arg_splitter.ArgSplitter()` to optionally allow aliasing builtin goals.
# Used by the arg splitter to optionally allow aliasing builtin goals.
aliases: ClassVar[tuple[str, ...]] = ()

@classmethod
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/goal/help.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class ThingHelpBuiltinGoalBase(HelpBuiltinGoalBase):

def create_help_request(self, options: Options) -> HelpRequest:
# Include the `options.specs` for things to give help on, as args with any . : * or # in
# them is deemed "likely a spec" by `pants.option.arg_splitter:ArgSplitter.likely_a_spec()`.
# them is deemed "likely a spec" by the arg splitter.
# We want to support getting help on API types that may contain periods.
return ThingHelp(
advanced=self._advanced,
Expand Down
11 changes: 8 additions & 3 deletions src/python/pants/option/arg_splitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from typing import Iterable, Iterator, Sequence

from pants.base.deprecated import warn_or_error
from pants.engine.internals.native_engine import PyArgSplitter
from pants.engine.internals.native_engine import PyArgSplitter, PyGoalInfo
from pants.option.scope import ScopeInfo


Expand Down Expand Up @@ -79,7 +79,12 @@ class ArgSplitter:

def __init__(self, known_scope_infos: Iterable[ScopeInfo], buildroot: str) -> None:
self._known_goal_scopes = dict(self._get_known_goal_scopes(known_scope_infos))
self._native_arg_splitter = PyArgSplitter(buildroot, list(self._known_goal_scopes.keys()))
native_known_goals = tuple(
PyGoalInfo(si.scope, si.is_builtin, si.is_auxiliary, si.scope_aliases)
for si in known_scope_infos
if si.is_goal
)
self._native_arg_splitter = PyArgSplitter(buildroot, native_known_goals)

@staticmethod
def _get_known_goal_scopes(
Expand All @@ -93,7 +98,7 @@ def _get_known_goal_scopes(
yield alias, si

def split_args(self, args: Sequence[str]) -> SplitArgs:
native_split_args = self._native_arg_splitter.split_args(list(args))
native_split_args = self._native_arg_splitter.split_args(tuple(args))

builtin_or_auxiliary_goal: str | None = None
canonical_goals = []
Expand Down
22 changes: 14 additions & 8 deletions src/rust/engine/options/src/arg_splitter.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// Copyright 2025 Pants project contributors (see CONTRIBUTORS.md).
// Licensed under the Apache License, Version 2.0 (see LICENSE).

use crate::GoalInfo;
use lazy_static::lazy_static;
use regex::Regex;
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};

lazy_static! {
Expand All @@ -22,17 +23,22 @@ pub struct SplitArgs {

pub struct ArgSplitter {
build_root: PathBuf,
known_goal_names: HashSet<String>,
known_goals: HashMap<String, GoalInfo>,
}

impl ArgSplitter {
pub fn new<'a, I: IntoIterator<Item = &'a str>>(
build_root: &Path,
known_goal_names: I,
) -> ArgSplitter {
pub fn new<I: IntoIterator<Item = GoalInfo>>(build_root: &Path, known_goals: I) -> ArgSplitter {
let mut known_goals_map = HashMap::new();
for goal_info in known_goals.into_iter() {
for alias in &goal_info.aliases {
known_goals_map.insert(alias.to_owned(), goal_info.clone());
}
known_goals_map.insert(goal_info.scope_name.to_owned(), goal_info);
}

ArgSplitter {
build_root: build_root.to_owned(),
known_goal_names: known_goal_names.into_iter().map(str::to_string).collect(),
known_goals: known_goals_map,
}
}

Expand Down Expand Up @@ -60,7 +66,7 @@ impl ArgSplitter {
while let Some(arg) = unconsumed_args.pop() {
// Some special flags, such as `-v` and `--help`, are implemented as
// goal aliases, so we must check this before checking for any dash prefixes.
if self.known_goal_names.contains(&arg) {
if self.known_goals.contains_key(arg.as_str()) {
goals.push(arg);
} else if arg == "--" {
// Arg is the passthru delimiter.
Expand Down
19 changes: 8 additions & 11 deletions src/rust/engine/options/src/arg_splitter_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0 (see LICENSE).

use crate::arg_splitter::{ArgSplitter, SplitArgs};
use crate::scope::GoalInfo;
use shlex;
use std::fs::File;
use std::path::Path;
Expand All @@ -15,17 +16,13 @@ fn shlex_and_split_args(build_root: Option<&Path>, args_str: &str) -> SplitArgs
ArgSplitter::new(
&build_root.unwrap_or(TempDir::new().unwrap().path()),
vec![
"run",
"check",
"fmt",
"test",
"help",
"jvm",
"bsp",
"-v",
"-h",
"--help",
"--help-advanced",
GoalInfo::new("run", false, false, vec![]),
GoalInfo::new("check", false, false, vec![]),
GoalInfo::new("fmt", false, false, vec![]),
GoalInfo::new("test", false, false, vec![]),
GoalInfo::new("help", true, false, vec!["-h", "--help", "--help-advanced"]),
GoalInfo::new("bsp", false, true, vec![]),
GoalInfo::new("version", true, false, vec!["-v", "-V"]),
],
)
.split_args(shlex::split(args_str).unwrap())
Expand Down
3 changes: 2 additions & 1 deletion src/rust/engine/options/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ use std::sync::Arc;

use serde::Deserialize;

pub use self::arg_splitter::{ArgSplitter, SplitArgs};
pub use self::args::Args;
use self::args::ArgsReader;
pub use self::config::ConfigSource;
Expand All @@ -66,7 +67,7 @@ use crate::fromfile::FromfileExpander;
use crate::parse::Parseable;
pub use build_root::BuildRoot;
pub use id::OptionId;
pub use scope::Scope;
pub use scope::{GoalInfo, Scope};
pub use types::OptionType;

// NB: The legacy Python options parser supported dicts with member_type "Any", which means
Expand Down
24 changes: 24 additions & 0 deletions src/rust/engine/options/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,27 @@ impl Scope {
}
}
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct GoalInfo {
pub scope_name: String,
pub is_builtin: bool,
pub is_auxiliary: bool,
pub aliases: Vec<String>,
}

impl GoalInfo {
pub fn new<'a, I: IntoIterator<Item = &'a str>>(
scope_name: &str,
is_builtin: bool,
is_auxiliary: bool,
aliases: I,
) -> Self {
Self {
scope_name: scope_name.to_owned(),
is_builtin,
is_auxiliary,
aliases: aliases.into_iter().map(str::to_owned).collect(),
}
}
}
35 changes: 28 additions & 7 deletions src/rust/engine/src/externs/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ use pyo3::exceptions::PyException;
use pyo3::types::{PyBool, PyDict, PyFloat, PyInt, PyList, PyString, PyTuple};
use pyo3::{prelude::*, BoundObject};

use options::arg_splitter::{ArgSplitter, SplitArgs};
use options::{
apply_dict_edits, apply_list_edits, Args, ConfigSource, DictEdit, DictEditAction, Env,
ListEdit, ListEditAction, ListOptionValue, OptionId, OptionParser, OptionalOptionValue, Scope,
Source, Val,
apply_dict_edits, apply_list_edits, ArgSplitter, Args, ConfigSource, DictEdit, DictEditAction,
Env, GoalInfo, ListEdit, ListEditAction, ListOptionValue, OptionId, OptionParser,
OptionalOptionValue, Scope, Source, SplitArgs, Val,
};

use itertools::Itertools;
Expand All @@ -19,6 +18,7 @@ use std::path::Path;
pyo3::import_exception!(pants.option.errors, ParseError);

pub(crate) fn register(m: &Bound<'_, PyModule>) -> PyResult<()> {
m.add_class::<PyGoalInfo>()?;
m.add_class::<PyOptionId>()?;
m.add_class::<PySplitArgs>()?;
m.add_class::<PyArgSplitter>()?;
Expand Down Expand Up @@ -117,6 +117,27 @@ pub(crate) fn py_object_to_val(obj: &Bound<'_, PyAny>) -> Result<Val, PyErr> {
}
}

#[pyclass]
struct PyGoalInfo(GoalInfo);

#[pymethods]
impl PyGoalInfo {
#[new]
fn __new__(
scope_name: &str,
is_builtin: bool,
is_auxiliary: bool,
aliases: Vec<String>,
) -> Self {
Self(GoalInfo::new(
scope_name,
is_builtin,
is_auxiliary,
aliases.iter().map(String::as_ref),
))
}
}

#[pyclass]
struct PyOptionId(OptionId);

Expand Down Expand Up @@ -178,12 +199,12 @@ struct PyArgSplitter(ArgSplitter);
#[pymethods]
impl PyArgSplitter {
#[new]
fn __new__(build_root: &str, known_goal_names: Vec<String>) -> Self {
fn __new__(build_root: &str, known_goals: Vec<Bound<'_, PyGoalInfo>>) -> Self {
Self(ArgSplitter::new(
Path::new(build_root),
known_goal_names
known_goals
.iter()
.map(AsRef::as_ref)
.map(|pgi| pgi.borrow().0.clone())
.collect::<Vec<_>>(),
))
}
Expand Down

0 comments on commit e061067

Please sign in to comment.