Skip to content
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
11 changes: 7 additions & 4 deletions .github/workflows/ci_docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,13 @@ jobs:
- name: Assert version-one CLI and Rust API docs
run: |
set -euo pipefail
grep -q 'package plan' docs/build/reference/cli.html
grep -q 'campaign run' docs/build/reference/cli.html
grep -q 'eb_package_plan' docs/build/reference/cli.html
grep -q 'eb-stack mcp' docs/build/reference/cli.html
# Sphinx splits code literals into per-word <span class="pre"> elements,
# so multi-word phrases only exist in the tag-stripped text.
cli_text=$(sed -e 's/&#160;/ /g' -e 's/<[^>]*>//g' docs/build/reference/cli.html)
grep -q 'package plan' <<<"$cli_text"
grep -q 'campaign run' <<<"$cli_text"
grep -q 'eb_package_plan' <<<"$cli_text"
grep -q 'eb-stack mcp' <<<"$cli_text"
# In-site Rust API
test -f docs/build/reference/rust-api.html
test -f docs/build/crates/eb_stack/lib.html
Expand Down
9 changes: 9 additions & 0 deletions pixi.lock

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

68 changes: 46 additions & 22 deletions src/eb_maintainer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Maintainer-acceptability checks distilled from real upstream reviews.
//!
//! Primary sources:
//!
//! - easybuild-easyconfigs PR #26435 (CHANGES_REQUESTED): cross-generation
//! dependency pins ("mixing two different toolchain generations") and
//! staged/incomprehensible shell in preconfigopts/postinstallcmds.
Expand Down Expand Up @@ -105,16 +106,15 @@ fn dep_in_recipe_hierarchy(recipe_tc: &Toolchain, dep_tc: &Toolchain) -> bool {
if is_system_toolchain(dep_tc) {
return true;
}
if recipe_tc.name.eq_ignore_ascii_case(&dep_tc.name)
&& recipe_tc.version == dep_tc.version
{
if recipe_tc.name.eq_ignore_ascii_case(&dep_tc.name) && recipe_tc.version == dep_tc.version {
return true;
}
// Subtoolchain of the same generation (e.g. GCCcore-15.2.0 under foss-2026.1).
if let Some(h) = known_hierarchy(recipe_tc) {
return h.members.iter().any(|m| {
m.name.eq_ignore_ascii_case(&dep_tc.name) && m.version == dep_tc.version
});
return h
.members
.iter()
.any(|m| m.name.eq_ignore_ascii_case(&dep_tc.name) && m.version == dep_tc.version);
}
// Without a hierarchy fixture: same high-level version string only.
if is_high_level_toolchain(&recipe_tc.name) && is_high_level_toolchain(&dep_tc.name) {
Expand Down Expand Up @@ -161,9 +161,10 @@ pub fn check_cross_generation_pins(recipe: &ResolvedEasyconfig) -> Vec<Maintaine
parent.name,
parent.version
),
Some(format!(
Some(
"This is mixing two different toolchain generations, it shouldn't be done"
)),
.to_string(),
),
));
continue;
}
Expand Down Expand Up @@ -266,9 +267,8 @@ pub fn check_shell_monsters(text: &str) -> Vec<MaintainerFinding> {
if t.starts_with("preconfigopts +=") || t.starts_with("preconfigopts+=") {
consecutive += 1;
max_uncommented = max_uncommented.max(consecutive);
} else if t.starts_with('#') {
consecutive = 0;
} else if !t.is_empty() {
// Comments and any other content both break the uncommented run.
consecutive = 0;
}
}
Expand Down Expand Up @@ -383,7 +383,10 @@ pub fn check_fat_build(text: &str) -> Vec<MaintainerFinding> {
(t.starts_with("runtest") && !t.starts_with("runtest = False"))
|| t.starts_with("runtests = True")
});
if let Some(flag) = TESTS_OFF_FLAGS.iter().find(|f| configopts_blob.contains(**f)) {
if let Some(flag) = TESTS_OFF_FLAGS
.iter()
.find(|f| configopts_blob.contains(**f))
{
out.push(MaintainerFinding::warning(
"EB_MAINT_TESTS_OFF",
format!(
Expand Down Expand Up @@ -427,7 +430,10 @@ pub fn check_duplicate_upstream(
}
if !candidate.name.eq_ignore_ascii_case(&recipe.name)
|| candidate.version != recipe.version
|| !candidate.toolchain.name.eq_ignore_ascii_case(&recipe.toolchain.name)
|| !candidate
.toolchain
.name
.eq_ignore_ascii_case(&recipe.toolchain.name)
|| candidate.toolchain.version != recipe.toolchain.version
{
continue;
Expand Down Expand Up @@ -511,9 +517,7 @@ fn recipe_toolchain_version_from_text(text: &str) -> Option<String> {
if let Some(colon) = rest.find(':') {
let vpart = rest[colon + 1..].trim();
let vpart = vpart.trim_start_matches(['\'', '"', ' ']);
let end = vpart
.find(['\'', '"', ',', '}'])
.unwrap_or(vpart.len());
let end = vpart.find(['\'', '"', ',', '}']).unwrap_or(vpart.len());
let v = &vpart[..end];
if !v.is_empty() && v != "version" {
return Some(v.to_string());
Expand All @@ -526,7 +530,9 @@ fn recipe_toolchain_version_from_text(text: &str) -> Option<String> {
if let Some(idx) = t.find("\"version\"") {
let rest = &t[idx..];
if let Some(colon) = rest.find(':') {
let vpart = rest[colon + 1..].trim().trim_start_matches(['\'', '"', ' ']);
let vpart = rest[colon + 1..]
.trim()
.trim_start_matches(['\'', '"', ' ']);
let end = vpart.find(['\'', '"', ',', '}']).unwrap_or(vpart.len());
let v = &vpart[..end];
if !v.is_empty() {
Expand All @@ -543,8 +549,14 @@ fn high_level_dep_pins_from_text(text: &str) -> Vec<(String, String)> {
let mut out = Vec::new();
let re = regex_lite_high_level_pin();
for cap in re.captures_iter(text) {
let pkg = cap.get(1).map(|m| m.as_str().to_string()).unwrap_or_default();
let gen = cap.get(3).map(|m| m.as_str().to_string()).unwrap_or_default();
let pkg = cap
.get(1)
.map(|m| m.as_str().to_string())
.unwrap_or_default();
let gen = cap
.get(3)
.map(|m| m.as_str().to_string())
.unwrap_or_default();
if !pkg.is_empty() && !gen.is_empty() {
out.push((pkg, gen));
}
Expand Down Expand Up @@ -576,7 +588,10 @@ mod tests {
let (recipe, text) = load("fixtures/maintainer_reject_26435/bad_cross_gen.eb");
let report = check_maintainer_acceptability(&recipe, &text);
assert!(
report.findings.iter().any(|f| f.code == "EB_MAINT_CROSS_GEN"),
report
.findings
.iter()
.any(|f| f.code == "EB_MAINT_CROSS_GEN"),
"{:?}",
report
);
Expand Down Expand Up @@ -636,7 +651,10 @@ mod tests {
"{report:?}"
);
assert!(
!report.findings.iter().any(|f| f.code == "EB_MAINT_CROSS_GEN"),
!report
.findings
.iter()
.any(|f| f.code == "EB_MAINT_CROSS_GEN"),
"in-hierarchy pin is not the cross-generation error: {report:?}"
);
assert!(report.ok_for_upstream());
Expand All @@ -647,7 +665,10 @@ mod tests {
let (recipe, text) = load("fixtures/maintainer_fat_26480/bad_tests_off.eb");
let report = check_maintainer_acceptability(&recipe, &text);
assert!(
report.findings.iter().any(|f| f.code == "EB_MAINT_TESTS_OFF"),
report
.findings
.iter()
.any(|f| f.code == "EB_MAINT_TESTS_OFF"),
"{report:?}"
);
}
Expand Down Expand Up @@ -783,7 +804,10 @@ mod tests {
let recipe = resolve_easyconfig_str(&text).expect("parse 26435 eOn");
let report = check_maintainer_acceptability(&recipe, &text);
assert!(
report.findings.iter().any(|f| f.code == "EB_MAINT_CROSS_GEN"),
report
.findings
.iter()
.any(|f| f.code == "EB_MAINT_CROSS_GEN"),
"expected cross-gen on real #26435: {:?}",
report
);
Expand Down
152 changes: 143 additions & 9 deletions src/eb_parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,14 @@ impl<'a> Parser<'a> {
(Some(Value::Str(a)), Value::Str(b)) => {
self.env.insert(name, Value::Str(a + &b));
}
(Some(Value::List(mut a)), Value::List(b) | Value::Tuple(b)) => {
a.extend(b);
self.env.insert(name, Value::List(a));
}
(Some(Value::Tuple(mut a)), Value::Tuple(b)) => {
a.extend(b);
self.env.insert(name, Value::Tuple(a));
}
(_, v) => {
self.env.insert(name, v);
}
Expand Down Expand Up @@ -532,6 +540,33 @@ impl<'a> Parser<'a> {
(Value::Str(fmt), Value::Int(arg)) => {
Value::Str(python_percent_format_one(&fmt, &arg.to_string()))
}
(Value::Str(fmt), Value::Tuple(args)) => {
// Python raises TypeError when the operand count does not
// match the conversion count. Silently dropping extra
// operands (or leaving a `%s` unfilled) would put a wrong
// value in the field instead of skipping it, so refuse.
let need = count_percent_conversions(&fmt);
if need != args.len() {
return Err(self.err(format!(
"% format has {need} conversion(s) but {} operand(s)",
args.len()
)));
}
let mut out = fmt;
for arg in &args {
let s = match arg {
Value::Str(s) => s.clone(),
Value::Int(i) => i.to_string(),
other => {
return Err(self.err(format!(
"unsupported % tuple operand: {other:?}"
)));
}
};
out = python_percent_format_one(&out, &s);
}
Value::Str(out)
}
(a, b) => {
return Err(self.err(format!("unsupported % operands: {a:?} % {b:?}")));
}
Expand Down Expand Up @@ -787,16 +822,43 @@ fn unescape_python_str(s: &str) -> String {
s.to_string()
}

/// Minimal Python-style `%s` / `%d` single-arg formatting used in easyconfigs.
fn python_percent_format_one(fmt: &str, arg: &str) -> String {
if let Some(idx) = fmt.find("%s") {
let mut out = String::with_capacity(fmt.len() + arg.len());
out.push_str(&fmt[..idx]);
out.push_str(arg);
out.push_str(&fmt[idx + 2..]);
return out;
/// Count `%s` / `%d` conversions, skipping `%%` escapes. Used to reject a
/// tuple `%` whose operand count disagrees with the format string, which
/// Python treats as a TypeError rather than a partial substitution.
fn count_percent_conversions(fmt: &str) -> usize {
let b = fmt.as_bytes();
let mut i = 0;
let mut n = 0;
while i + 1 < b.len() {
if b[i] == b'%' {
match b[i + 1] {
b'%' => {
i += 2;
continue;
}
b's' | b'd' => {
n += 1;
i += 2;
continue;
}
_ => {}
}
}
i += 1;
}
if let Some(idx) = fmt.find("%d") {
n
}

/// Minimal Python-style `%s` / `%d` formatting used in easyconfigs. Replaces
/// the leftmost conversion so tuple args substitute in positional order.
fn python_percent_format_one(fmt: &str, arg: &str) -> String {
let idx = match (fmt.find("%s"), fmt.find("%d")) {
(Some(a), Some(b)) => Some(a.min(b)),
(Some(a), None) => Some(a),
(None, Some(b)) => Some(b),
(None, None) => None,
};
if let Some(idx) = idx {
let mut out = String::with_capacity(fmt.len() + arg.len());
out.push_str(&fmt[..idx]);
out.push_str(arg);
Expand Down Expand Up @@ -2798,4 +2860,76 @@ builddependencies = [
.iter()
.any(|m| m.name == "Meson" && m.role == "build"));
}
#[test]
fn aug_assign_extends_dependency_list() {
let src = r#"
name = 'demo'
version = '1.0'
toolchain = {'name': 'foss', 'version': '2025b'}
dependencies = [('zlib', '1.3.1')]
dependencies += [('OpenBLAS', '0.3.27')]
"#;
let r = resolve_easyconfig_str(src).expect("resolve");
let names: Vec<&str> = r.dependencies.iter().map(|d| d.name.as_str()).collect();
assert_eq!(names, ["zlib", "OpenBLAS"], "+= must extend, not replace");
}

#[test]
fn percent_format_with_tuple_args() {
let src = r#"
name = 'demo'
version = '2.1'
local_slug = '%s-%s' % (name, version)
homepage = 'https://example.org/%s' % local_slug
toolchain = {'name': 'GCCcore', 'version': '14.3.0'}
"#;
let r = resolve_easyconfig_str(src).expect("resolve");
assert_eq!(r.homepage.as_deref(), Some("https://example.org/demo-2.1"));
}

#[test]
fn percent_format_replaces_leftmost_conversion() {
let src = r#"
name = 'demo'
version = '1.0'
homepage = 'https://example.org/v%d/%s' % (3, name)
toolchain = {'name': 'GCCcore', 'version': '14.3.0'}
"#;
let r = resolve_easyconfig_str(src).expect("resolve");
assert_eq!(r.homepage.as_deref(), Some("https://example.org/v3/demo"));
}
#[test]
fn percent_format_arity_mismatch_skips_the_field_instead_of_guessing() {
// Python raises TypeError for both shapes. The parser must not emit a
// partially-substituted value: before the arity check, `too_many` silently
// dropped `version` and `too_few` left a literal `%s` in the URL.
let too_many = r#"
name = 'demo'
version = '1.0'
homepage = 'https://example.org/%s' % (name, version)
toolchain = {'name': 'GCCcore', 'version': '14.3.0'}
"#;
let r = resolve_easyconfig_str(too_many).expect("file still resolves");
assert_eq!(
r.homepage, None,
"extra operand must skip the field, not be dropped silently"
);

let too_few = r#"
name = 'demo'
version = '1.0'
homepage = 'https://example.org/%s/%s' % (name,)
toolchain = {'name': 'GCCcore', 'version': '14.3.0'}
"#;
let r = resolve_easyconfig_str(too_few).expect("file still resolves");
assert_eq!(r.homepage, None, "unfilled conversion must skip the field");
}

#[test]
fn percent_escape_is_not_counted_as_a_conversion() {
// `%%` is a literal percent, so this format has exactly one real conversion.
assert_eq!(count_percent_conversions("100%% of %s"), 1);
assert_eq!(count_percent_conversions("%s-%s"), 2);
assert_eq!(count_percent_conversions("no conversions here"), 0);
}
}
4 changes: 1 addition & 3 deletions src/eb_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,10 +362,8 @@ fn parse_string_assignment(line: &str) -> Option<StringAssignment<'_>> {
let after_key = rest[key_end..].trim_start();
let (op, after_op) = if let Some(r) = after_key.strip_prefix("+=") {
("+=", r.trim_start())
} else if let Some(r) = after_key.strip_prefix('=') {
("=", r.trim_start())
} else {
return None;
("=", after_key.strip_prefix('=')?.trim_start())
};
let quote = after_op.chars().next()?;
if quote != '\'' && quote != '"' {
Expand Down
Loading
Loading