Skip to content

Commit af698f0

Browse files
benjywbenjyw
authored
Support plucking option values from inside structured files. (#23201)
This is an extension of the "fromfile" feature. Previously an option value of "@path/to/file" meant "take the value from this file", and we supported parsing JSON/YAML/TOML files for dict-valued options, and JSON/YAML for list-valued options. This feature allows "@path/to/file:dotted.trail", where "dotted.trail" is the nested "path" (called trail to avoid confusion) inside the dict to the value. This is to support things like setting requirements or interpreter constraints directly from inside pyproject.toml without any special-casing.
1 parent d79f0d8 commit af698f0

File tree

11 files changed

+643
-175
lines changed

11 files changed

+643
-175
lines changed

‎docs/notes/2.32.x.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ The plugin API's `Get()` and `MultiGet()` constructs, deprecated in 2.30, are no
2222

2323
The Pants [Contribution Overview](https://www.pantsbuild.org/2.32/docs/contributions) now contains guidance on LLM use.
2424

25-
Dict-valued options can now [read their values](https://www.pantsbuild.org/2.32/docs/using-pants/key-concepts/options#reading-individual-option-values-from-files) from `.toml` files.
25+
Dict-valued options can now [read their values](https://www.pantsbuild.org/2.32/docs/using-pants/key-concepts/options#reading-individual-option-values-from-files) from `.toml` files. And any option value can be read from
26+
a specific field inside a TOML/YAML/JSON file, using the `@path/to/file:trail.within.file` syntax.
2627

2728
#### Internal Python Upgrade
2829

‎src/rust/options/src/config.rs‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use toml::Value;
1515
use toml::value::Table;
1616

1717
use super::{DictEdit, DictEditAction, ListEdit, ListEditAction, OptionsSource, Val};
18+
use crate::FromVal;
1819
use crate::fromfile::FromfileExpander;
1920
use crate::id::{NameTransform, OptionId};
2021
use crate::parse::Parseable;
@@ -102,22 +103,16 @@ struct ValueConversionError<'a> {
102103
given_value: &'a Value,
103104
}
104105

105-
trait FromValue: Parseable {
106+
trait FromValue: FromVal {
106107
fn from_value(value: &Value) -> Result<Self, ValueConversionError<'_>>;
107108

108109
fn from_config(config: &ConfigReader, id: &OptionId) -> Result<Option<Self>, String> {
109110
if let Some(value) = config.get_value(id) {
110111
if value.is_str() {
111-
match config
112+
config
112113
.fromfile_expander
113-
.expand(value.as_str().unwrap().to_owned())
114-
.map_err(|e| e.render(config.display(id)))?
115-
{
116-
Some(expanded_value) => Ok(Some(
117-
Self::parse(&expanded_value).map_err(|e| e.render(config.display(id)))?,
118-
)),
119-
_ => Ok(None),
120-
}
114+
.expand::<Self>(value.as_str().unwrap().to_owned())
115+
.map_err(|e| e.render(config.display(id)))
121116
} else {
122117
match Self::from_value(value) {
123118
Ok(x) => Ok(Some(x)),

‎src/rust/options/src/env.rs‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@ use std::ffi::OsString;
88

99
use super::id::{NameTransform, OptionId};
1010
use super::{DictEdit, OptionsSource};
11-
use crate::ListEdit;
1211
use crate::fromfile::FromfileExpander;
13-
use crate::parse::Parseable;
1412
use crate::scope::Scope;
13+
use crate::{FromVal, ListEdit};
1514

1615
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
1716
pub struct Env {
@@ -87,7 +86,19 @@ impl EnvReader {
8786
names
8887
}
8988

90-
fn get_list<T: Parseable>(&self, id: &OptionId) -> Result<Option<Vec<ListEdit<T>>>, String> {
89+
fn get_scalar<T: FromVal>(&self, id: &OptionId) -> Result<Option<T>, String> {
90+
for env_var_name in &Self::env_var_names(id) {
91+
if let Some(value) = self.env.env.get(env_var_name) {
92+
return self
93+
.fromfile_expander
94+
.expand::<T>(value.to_owned())
95+
.map_err(|e| e.render(self.display(id)));
96+
}
97+
}
98+
Ok(None)
99+
}
100+
101+
fn get_list<T: FromVal>(&self, id: &OptionId) -> Result<Option<Vec<ListEdit<T>>>, String> {
91102
for env_var_name in &Self::env_var_names(id) {
92103
if let Some(value) = self.env.env.get(env_var_name) {
93104
return self
@@ -119,25 +130,19 @@ impl OptionsSource for EnvReader {
119130
}
120131

121132
fn get_string(&self, id: &OptionId) -> Result<Option<String>, String> {
122-
for env_var_name in &Self::env_var_names(id) {
123-
if let Some(value) = self.env.env.get(env_var_name) {
124-
return self
125-
.fromfile_expander
126-
.expand(value.to_owned())
127-
.map_err(|e| e.render(self.display(id)));
128-
}
129-
}
130-
Ok(None)
133+
self.get_scalar::<String>(id)
131134
}
132135

133136
fn get_bool(&self, id: &OptionId) -> Result<Option<bool>, String> {
134-
if let Some(value) = self.get_string(id)? {
135-
bool::parse(&value)
136-
.map(Some)
137-
.map_err(|e| e.render(self.display(id)))
138-
} else {
139-
Ok(None)
140-
}
137+
self.get_scalar::<bool>(id)
138+
}
139+
140+
fn get_int(&self, id: &OptionId) -> Result<Option<i64>, String> {
141+
self.get_scalar::<i64>(id)
142+
}
143+
144+
fn get_float(&self, id: &OptionId) -> Result<Option<f64>, String> {
145+
self.get_scalar::<f64>(id)
141146
}
142147

143148
fn get_bool_list(&self, id: &OptionId) -> Result<Option<Vec<ListEdit<bool>>>, String> {

‎src/rust/options/src/env_tests.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,9 @@ fn test_float() {
134134
("PANTS_BAD", "swallow"),
135135
]);
136136

137-
let assert_float =
138-
|expected: f64, id: OptionId| assert_eq!(expected, env.get_float(&id).unwrap().unwrap());
137+
let assert_float = |expected: f64, id: OptionId| {
138+
assert_eq!(expected, env.get_float_loose(&id).unwrap().unwrap())
139+
};
139140

140141
assert_float(4_f64, option_id!("foo"));
141142
assert_float(3.14, option_id!("bar", "baz"));

‎src/rust/options/src/flags.rs‎

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
use super::id::{NameTransform, OptionId};
55
use super::scope::Scope;
66
use super::{DictEdit, OptionsSource};
7-
use crate::ListEdit;
87
use crate::fromfile::FromfileExpander;
9-
use crate::parse::{ParseError, Parseable};
8+
use crate::parse::ParseError;
9+
use crate::{FromVal, ListEdit};
1010
use itertools::{Itertools, chain};
1111
use parking_lot::Mutex;
1212
use std::any::Any;
@@ -155,19 +155,32 @@ impl FlagsReader {
155155
ret
156156
}
157157

158+
fn get_scalar<T: FromVal>(&self, id: &OptionId) -> Result<Option<T>, String> {
159+
// We iterate in reverse so that the rightmost flag wins in case an option
160+
// is specified multiple times.
161+
for flag in self.flags.iter().rev() {
162+
if self.matches(flag, id) {
163+
return self
164+
.fromfile_expander
165+
.expand::<T>(flag.value.clone().ok_or_else(|| {
166+
format!("Expected option {} to have a value.", self.display(id))
167+
})?)
168+
.map_err(|e| e.render(&flag.key));
169+
};
170+
}
171+
Ok(None)
172+
}
173+
158174
fn to_bool(&self, arg: &Flag) -> Result<Option<bool>, ParseError> {
159175
// An arg can represent a bool either by having an explicit value parseable as a bool,
160176
// or by having no value (in which case it represents true).
161177
match &arg.value {
162-
Some(value) => match self.fromfile_expander.expand(value.to_string())? {
163-
Some(s) => bool::parse(&s).map(Some),
164-
_ => Ok(None),
165-
},
178+
Some(value) => self.fromfile_expander.expand::<bool>(value.to_string()),
166179
None => Ok(Some(true)),
167180
}
168181
}
169182

170-
fn get_list<T: Parseable>(&self, id: &OptionId) -> Result<Option<Vec<ListEdit<T>>>, String> {
183+
fn get_list<T: FromVal>(&self, id: &OptionId) -> Result<Option<Vec<ListEdit<T>>>, String> {
171184
let mut edits = vec![];
172185
for flag in &self.flags {
173186
if self.matches(flag, id) {
@@ -208,22 +221,7 @@ impl OptionsSource for FlagsReader {
208221
}
209222

210223
fn get_string(&self, id: &OptionId) -> Result<Option<String>, String> {
211-
// We iterate in reverse so that the rightmost flag wins in case an option
212-
// is specified multiple times.
213-
for flag in self.flags.iter().rev() {
214-
if self.matches(flag, id) {
215-
return self
216-
.fromfile_expander
217-
.expand(flag.value.clone().ok_or_else(|| {
218-
format!(
219-
"Expected string option {} to have a value.",
220-
self.display(id)
221-
)
222-
})?)
223-
.map_err(|e| e.render(&flag.key));
224-
};
225-
}
226-
Ok(None)
224+
self.get_scalar::<String>(id)
227225
}
228226

229227
fn get_bool(&self, id: &OptionId) -> Result<Option<bool>, String> {
@@ -242,6 +240,14 @@ impl OptionsSource for FlagsReader {
242240
Ok(None)
243241
}
244242

243+
fn get_int(&self, id: &OptionId) -> Result<Option<i64>, String> {
244+
self.get_scalar::<i64>(id)
245+
}
246+
247+
fn get_float(&self, id: &OptionId) -> Result<Option<f64>, String> {
248+
self.get_scalar::<f64>(id)
249+
}
250+
245251
fn get_bool_list(&self, id: &OptionId) -> Result<Option<Vec<ListEdit<bool>>>, String> {
246252
self.get_list::<bool>(id)
247253
}

‎src/rust/options/src/flags_tests.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,9 @@ fn test_float() {
123123
"--bad=swallow",
124124
]);
125125

126-
let assert_float =
127-
|expected: f64, id: OptionId| assert_eq!(expected, args.get_float(&id).unwrap().unwrap());
126+
let assert_float = |expected: f64, id: OptionId| {
127+
assert_eq!(expected, args.get_float_loose(&id).unwrap().unwrap())
128+
};
128129

129130
assert_float(4_f64, option_id!("jobs"));
130131
assert_float(3.14, option_id!("foo"));

0 commit comments

Comments
 (0)