diff --git a/.github/workflows/ci_docs.yml b/.github/workflows/ci_docs.yml index 73e05241..89cac0ca 100644 --- a/.github/workflows/ci_docs.yml +++ b/.github/workflows/ci_docs.yml @@ -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 elements, + # so multi-word phrases only exist in the tag-stripped text. + cli_text=$(sed -e 's/ / /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 diff --git a/pixi.lock b/pixi.lock index 5da624bd..416fa8e7 100644 --- a/pixi.lock +++ b/pixi.lock @@ -1,11 +1,20 @@ version: 7 platforms: - name: linux-64 + virtual-packages: + - __unix=0=0 + - __linux=4.18 + - __glibc=2.28 + - __archspec=0=x86_64 environments: default: channels: - url: https://conda.anaconda.org/conda-forge/ packages: {} + dev: + channels: + - url: https://conda.anaconda.org/conda-forge/ + packages: {} docs: channels: - url: https://conda.anaconda.org/conda-forge/ diff --git a/src/eb_maintainer.rs b/src/eb_maintainer.rs index 45cba335..704e3b69 100644 --- a/src/eb_maintainer.rs +++ b/src/eb_maintainer.rs @@ -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. @@ -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) { @@ -161,9 +161,10 @@ pub fn check_cross_generation_pins(recipe: &ResolvedEasyconfig) -> Vec Vec { 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; } } @@ -383,7 +383,10 @@ pub fn check_fat_build(text: &str) -> Vec { (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!( @@ -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; @@ -511,9 +517,7 @@ fn recipe_toolchain_version_from_text(text: &str) -> Option { 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()); @@ -526,7 +530,9 @@ fn recipe_toolchain_version_from_text(text: &str) -> Option { 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() { @@ -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)); } @@ -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 ); @@ -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()); @@ -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:?}" ); } @@ -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 ); diff --git a/src/eb_parse.rs b/src/eb_parse.rs index 8a719928..1e5975f9 100644 --- a/src/eb_parse.rs +++ b/src/eb_parse.rs @@ -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); } @@ -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:?}"))); } @@ -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); @@ -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); + } } diff --git a/src/eb_style.rs b/src/eb_style.rs index b48a7363..5fc54e91 100644 --- a/src/eb_style.rs +++ b/src/eb_style.rs @@ -362,10 +362,8 @@ fn parse_string_assignment(line: &str) -> Option> { 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 != '"' { diff --git a/src/lib.rs b/src/lib.rs index 3261ce75..324530ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,11 @@ pub mod target; pub mod version; pub use domain::*; +pub use eb_maintainer::{ + check_cross_generation_pins, check_dep_toolchain_pins, check_duplicate_upstream, + check_fat_build, check_maintainer_acceptability, check_maintainer_acceptability_text, + check_shell_monsters, MaintainerFinding, MaintainerReport, MaintainerSeverity, +}; pub use eb_parse::candidate_matches_dep_for_recipe; pub use eb_parse::{ candidate_matches_dep, check_recipe_deps, easyconfig_basename, easyconfig_letter_dir, @@ -36,11 +41,6 @@ pub use eb_parse::{ resolve_easyconfig_str, validate_lock_deps, version_field_to_req, MissingDep, ParseTreeResult, RecipeDepCheck, ResolvedDep, ResolvedEasyconfig, ResolvedExt, SkippedEasyconfig, }; -pub use eb_maintainer::{ - check_cross_generation_pins, check_dep_toolchain_pins, check_duplicate_upstream, - check_fat_build, check_maintainer_acceptability, check_maintainer_acceptability_text, - check_shell_monsters, MaintainerFinding, MaintainerReport, MaintainerSeverity, -}; pub use eb_style::{ format_style, format_style_file, lint_style, FormatStyleResult, StyleError, StyleFinding, EB_MAX_LINE, diff --git a/src/main.rs b/src/main.rs index e8f41cbb..772be3f5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,13 +12,13 @@ use eb_stack::package_sources::{PackageSourceRoots, SourceRootKind}; use eb_stack::target::{doctor_target, resolve_target_layers, BuildTarget, TargetConfigLayer}; use eb_stack::{ check_duplicate_upstream, check_maintainer_acceptability, check_maintainer_acceptability_text, - check_recipe_deps, - format_style, format_style_file, inspect_new_package, lint_style, load_json_file, - lock_to_cyclonedx, packaging_gate, parse_easyconfig_trees, plan_new_package, plan_package_bump, - plan_package_closure_with_sources, resolve_easyconfig_file, resolve_package_catalog_layers, - solve_from_easyconfigs_with_baseline_version_and_extras, write_json_pretty, - write_package_bundle, write_package_closure, BumpPackageRequest, ForeignFormat, - NewPackageRequest, PackageBundle, PackageCatalogLayer, SolveExtraOut, StackLock, Toolchain, + check_recipe_deps, format_style, format_style_file, inspect_new_package, lint_style, + load_json_file, lock_to_cyclonedx, packaging_gate, parse_easyconfig_trees, plan_new_package, + plan_package_bump, plan_package_closure_with_sources, resolve_easyconfig_file, + resolve_package_catalog_layers, solve_from_easyconfigs_with_baseline_version_and_extras, + write_json_pretty, write_package_bundle, write_package_closure, BumpPackageRequest, + ForeignFormat, NewPackageRequest, PackageBundle, PackageCatalogLayer, SolveExtraOut, StackLock, + Toolchain, }; use std::collections::HashMap; use std::path::{Path, PathBuf}; diff --git a/src/package_emit.rs b/src/package_emit.rs index 8a60c425..cd8cc54a 100644 --- a/src/package_emit.rs +++ b/src/package_emit.rs @@ -1,6 +1,7 @@ //! Deterministic EasyBuild recipe-set emission from materialized package profiles. //! //! Emission targets conventional EasyBuild style used by easybuilders/easyconfigs: +//! //! - primary GitHub tag archives use `github_account` / `GITHUB_SOURCE` / //! `SOURCELOWER_TAR_GZ` when they match the package identity; //! - dependency tuples omit the toolchain when the lock identity sits on the diff --git a/tests/qmcpack_foss_2026_1.rs b/tests/qmcpack_foss_2026_1.rs index a703db27..2c58c5ab 100644 --- a/tests/qmcpack_foss_2026_1.rs +++ b/tests/qmcpack_foss_2026_1.rs @@ -52,8 +52,7 @@ fn resolve_qmcpack_foss_2026_1() { "must set test_cmd=ctest (CMakeNinja ninja no-op trap)" ); assert!( - txt.contains("PRTE_MCA_rmaps_default_mapping_policy") - && txt.contains(":oversubscribe"), + txt.contains("PRTE_MCA_rmaps_default_mapping_policy") && txt.contains(":oversubscribe"), "parallel ctest needs PRRTE oversubscribe (netCDF/OTF-CPT pattern)" ); assert!(