diff --git a/bench/fastled-examples/src/build_comparison.rs b/bench/fastled-examples/src/build_comparison.rs index 003476c3..da8d8023 100644 --- a/bench/fastled-examples/src/build_comparison.rs +++ b/bench/fastled-examples/src/build_comparison.rs @@ -162,6 +162,9 @@ struct ToolResult { daemon_restarts: usize, /// Package versions actually selected by this tool, not requested pins. resolved_packages: BTreeMap, + /// Why package identity was unavailable, if a successful build omitted it. + #[serde(skip_serializing_if = "Option::is_none")] + package_metadata_warning: Option, /// Distinct Arduino core sources in the tool's compile database. core_source_count: Option, /// Full compiler invocation for a representative core source. @@ -331,6 +334,7 @@ fn measure_tool( let mut cold_phase_trials = Vec::new(); let mut daemon_restarts = 0; let mut resolved_packages = BTreeMap::new(); + let mut package_metadata_warning = None; let perf_log = output_dir.join(PERF_LOG_FILE); let envs = tool_envs(kind, &perf_log); @@ -380,8 +384,14 @@ fn measure_tool( &envs, )?; daemon_restarts += usize::from(restarted); - if matches!(kind, ToolKind::PlatformIo) && !packages.is_empty() { - resolved_packages = packages; + if matches!(kind, ToolKind::PlatformIo | ToolKind::Fbuild) { + record_package_metadata( + &mut resolved_packages, + &mut package_metadata_warning, + packages, + kind, + board, + ); } cold_trials_ms.push(round_millis(elapsed)); if matches!(kind, ToolKind::Fbuild) { @@ -419,22 +429,20 @@ fn measure_tool( } } - if matches!(kind, ToolKind::Fbuild) { - let output = run_logged_env( - fbuild.as_os_str(), - &os_args(&[ - "install", - &project_dir.to_string_lossy(), - "--environment", - board.environment, - "--check", - "--json", - ]), - repo_root, - log, - &[], - )?; - resolved_packages = parse_fbuild_packages(&output.stdout)?; + if matches!(kind, ToolKind::PlatformIo | ToolKind::Fbuild) + && !package_metadata_is_complete(board.key, &resolved_packages) + { + let warning = package_metadata_warning.get_or_insert_with(|| { + format!( + "{} build output omitted one or more required resolved package identities", + kind.style().label + ) + }); + println!( + "::warning title=benchmark package metadata unavailable::{} ({})", + board.name, warning + ); + writeln!(log, "Package metadata warning: {warning}")?; } if matches!(kind, ToolKind::PlatformIo) { run_logged_env( @@ -490,6 +498,7 @@ fn measure_tool( cold_phase_trials, daemon_restarts, resolved_packages, + package_metadata_warning, core_source_count, core_compile_argv, }) @@ -967,14 +976,101 @@ fn timed_build( let started = Instant::now(); let output = run_logged_env(program, &args, repo_root, log, envs)?; let elapsed_ms = started.elapsed().as_secs_f64() * 1000.0; - let packages = if matches!(kind, ToolKind::PlatformIo) { - parse_platformio_packages(&output.stdout, board) - } else { - BTreeMap::new() + let packages = match kind { + ToolKind::PlatformIo => parse_platformio_packages(&output.stdout, board), + ToolKind::Fbuild => parse_fbuild_build_packages(&output.stdout, board), + ToolKind::Arduino => BTreeMap::new(), }; Ok((elapsed_ms, restarted_daemon(&output.stderr), packages)) } +fn record_package_metadata( + current: &mut BTreeMap, + warning: &mut Option, + observed: BTreeMap, + kind: ToolKind, + board: Board, +) { + if observed.is_empty() { + current.clear(); + if warning.is_none() { + *warning = Some(format!( + "{} build output omitted resolved package identities on {}", + kind.style().label, + board.name + )); + } + return; + } + // Every timed cold trial must agree. Once any trial is missing or has a + // different identity, later observations cannot restore comparability. + if warning.is_some() { + return; + } + if current.is_empty() { + *current = observed; + } else if *current != observed { + current.clear(); + *warning = Some(format!( + "{} resolved package identities changed across cold trials on {}", + kind.style().label, + board.name + )); + } +} + +fn parse_fbuild_build_packages(stdout: &[u8], board: Board) -> BTreeMap { + let mut packages = BTreeMap::new(); + for line in String::from_utf8_lossy(stdout).lines() { + if board.key == "esp32s3" { + let Some((_, resolved)) = line + .split_once("ESP32 packages:") + .and_then(|(_, packages)| packages.split_once("; resolved ")) + else { + continue; + }; + for (key, marker) in [ + ("platform", "platform="), + ("framework", "framework="), + ("toolchain", "toolchain="), + ("sdk", "ESP-IDF SDK="), + ] { + if let Some(value) = resolved + .split_once(marker) + .map(|(_, value)| value.split([',', ';']).next().unwrap_or("").trim()) + { + let version = value.rsplit_once('@').map_or(value, |(_, version)| version); + if !version.is_empty() { + packages.insert(key.to_string(), version.to_string()); + } + } + } + } else if board.key == "uno" { + if let Some((_, resolved)) = line.split_once("AVR resolved:") { + for (key, package) in [ + ("toolchain", "toolchain-atmelavr"), + ("framework", "framework-"), + ] { + let field = if key == "toolchain" { + resolved.split(';').next().unwrap_or("") + } else { + resolved.split(';').nth(1).unwrap_or("") + }; + if let Some(value) = field.split_once(package).map(|(_, value)| value) { + if let Some((_, version)) = value.split_once('@') { + let version = version.split_whitespace().next().unwrap_or(""); + if !version.is_empty() { + packages.insert(key.to_string(), version.to_string()); + } + } + } + } + } + } + } + packages +} + fn parse_platformio_packages(stdout: &[u8], board: Board) -> BTreeMap { let mut packages = BTreeMap::new(); for line in String::from_utf8_lossy(stdout).lines() { @@ -1017,28 +1113,6 @@ fn parse_platformio_packages(stdout: &[u8], board: Board) -> BTreeMap AppResult> { - let value: Value = serde_json::from_slice(stdout)?; - let packages = value["environments"][0]["packages"] - .as_array() - .ok_or_else(|| io::Error::other("fbuild install --json omitted packages"))?; - let mut resolved = BTreeMap::new(); - for package in packages { - let Some(kind) = package["kind"].as_str() else { - continue; - }; - let key = match kind { - "platform" | "framework" | "toolchain" => kind, - "tool" if package["name"] == "tool-esptoolpy" => "flash_tool", - _ => continue, - }; - if let Some(version) = package["version"].as_str() { - resolved.insert(key.to_string(), version.to_string()); - } - } - Ok(resolved) -} - fn core_compile_metadata(path: &Path) -> AppResult<(Option, Option>)> { let entries: Vec = serde_json::from_slice(&fs::read(path)?)?; let core = entries @@ -1276,6 +1350,30 @@ fn board_cold_ratio(results: &[ToolResult], board: &str) -> Option { } fn board_stack_comparable(results: &[ToolResult], board: &str) -> bool { + board_stack_status(results, board) == StackStatus::Matched +} + +fn stack_package_keys(board: &str) -> &'static [&'static str] { + match board { + "esp32s3" => &["platform", "framework", "toolchain"], + "uno" => &["framework", "toolchain"], + _ => &[], + } +} + +fn package_metadata_is_complete(board: &str, packages: &BTreeMap) -> bool { + let required = stack_package_keys(board); + !required.is_empty() && required.iter().all(|key| packages.contains_key(*key)) +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum StackStatus { + Matched, + Different, + Unverified, +} + +fn board_stack_status(results: &[ToolResult], board: &str) -> StackStatus { results .iter() .find(|result| result.board == board && result.tool == "fbuild") @@ -1284,9 +1382,30 @@ fn board_stack_comparable(results: &[ToolResult], board: &str) -> bool { .iter() .find(|result| result.board == board && result.tool == "platformio"), ) - .is_some_and(|(fbuild, pio)| { - !fbuild.resolved_packages.is_empty() - && fbuild.resolved_packages == pio.resolved_packages + .map_or(StackStatus::Unverified, |(fbuild, pio)| { + let keys = stack_package_keys(board); + if keys.is_empty() { + return StackStatus::Unverified; + } + let resolved = keys + .iter() + .map(|key| { + fbuild + .resolved_packages + .get(*key) + .zip(pio.resolved_packages.get(*key)) + }) + .collect::>(); + if resolved.iter().any(Option::is_none) { + StackStatus::Unverified + } else if resolved + .iter() + .all(|pair| pair.is_some_and(|(fbuild, pio)| fbuild == pio)) + { + StackStatus::Matched + } else { + StackStatus::Different + } }) } @@ -1299,6 +1418,11 @@ fn board_metrics(metadata: &Metadata, results: &[ToolResult]) -> Value { json!({ "fbuild_vs_platformio_cold": board_cold_ratio(results, board.key), "stack_comparable": board_stack_comparable(results, board.key), + "stack_status": match board_stack_status(results, board.key) { + StackStatus::Matched => "matched", + StackStatus::Different => "different", + StackStatus::Unverified => "unverified", + }, "raw_baseline_ms": metadata.raw_baselines_ms.get(board.key), "fbuild_overhead_ms": cold_of(results, board.key, "fbuild") .zip(metadata.raw_baselines_ms.get(board.key)) @@ -1498,12 +1622,10 @@ fn render_svg(metadata: &Metadata, results: &[ToolResult]) -> String { .max(1.0) }; let mut rows = String::new(); - let stack_note = |board: &str| { - if board_stack_comparable(results, board) { - " | fbuild/PIO stack matched" - } else { - " | fbuild/PIO stack differs; ratio excluded" - } + let stack_note = |board: &str| match board_stack_status(results, board) { + StackStatus::Matched => " | fbuild/PIO stack matched", + StackStatus::Different => " | fbuild/PIO stack differs; ratio excluded", + StackStatus::Unverified => " | fbuild/PIO stack unverified; ratio excluded", }; for (index, result) in results.iter().enumerate() { let kind = match result.tool.as_str() { @@ -1616,10 +1738,10 @@ fn render_html(metadata: &Metadata, results: &[ToolResult]) -> String { let comparison_note = BOARDS .iter() .map(|board| { - let status = if board_stack_comparable(results, board.key) { - "matched; fbuild/PlatformIO ratio shown" - } else { - "different or unverified; fbuild/PlatformIO ratio excluded" + let status = match board_stack_status(results, board.key) { + StackStatus::Matched => "matched; fbuild/PlatformIO ratio shown", + StackStatus::Different => "different; fbuild/PlatformIO ratio excluded", + StackStatus::Unverified => "unverified; fbuild/PlatformIO ratio excluded", }; format!("{}: {}", board.name, status) }) diff --git a/bench/fastled-examples/src/build_comparison_tests.rs b/bench/fastled-examples/src/build_comparison_tests.rs index be87062f..be5c262a 100644 --- a/bench/fastled-examples/src/build_comparison_tests.rs +++ b/bench/fastled-examples/src/build_comparison_tests.rs @@ -35,7 +35,11 @@ fn sample_results() -> Vec { cold_phases_ms: BTreeMap::new(), cold_phase_trials: Vec::new(), daemon_restarts: 0, - resolved_packages: BTreeMap::from([("framework".into(), "1.8.8".into())]), + resolved_packages: BTreeMap::from([ + ("framework".into(), "1.8.8".into()), + ("toolchain".into(), "7.3.0".into()), + ]), + package_metadata_warning: None, core_source_count: None, core_compile_argv: None, }, @@ -53,7 +57,11 @@ fn sample_results() -> Vec { cold_phases_ms: BTreeMap::new(), cold_phase_trials: Vec::new(), daemon_restarts: 0, - resolved_packages: BTreeMap::from([("framework".into(), "1.8.8".into())]), + resolved_packages: BTreeMap::from([ + ("framework".into(), "1.8.8".into()), + ("toolchain".into(), "7.3.0".into()), + ]), + package_metadata_warning: None, core_source_count: None, core_compile_argv: None, }, @@ -71,7 +79,11 @@ fn sample_results() -> Vec { cold_phases_ms: BTreeMap::from([("compile".to_string(), 400.0)]), cold_phase_trials: vec![BTreeMap::from([("compile".to_string(), 400.0)])], daemon_restarts: 0, - resolved_packages: BTreeMap::from([("framework".into(), "1.8.8".into())]), + resolved_packages: BTreeMap::from([ + ("framework".into(), "1.8.8".into()), + ("toolchain".into(), "7.3.0".into()), + ]), + package_metadata_warning: None, core_source_count: None, core_compile_argv: None, }, @@ -505,10 +517,15 @@ fn esp32_regression_is_not_hidden_by_uno_ratio() { pio.board = "esp32s3".into(); pio.board_name = "ESP32-S3".into(); pio.cold_ms = 6_000.0; + pio.resolved_packages + .insert("platform".into(), "6.13.0".into()); let mut fbuild = results[2].clone(); fbuild.board = "esp32s3".into(); fbuild.board_name = "ESP32-S3".into(); fbuild.cold_ms = 15_000.0; + fbuild + .resolved_packages + .insert("platform".into(), "6.13.0".into()); results.extend([pio, fbuild]); let mut metadata = sample_metadata(); @@ -559,16 +576,146 @@ fn different_resolved_esp32_stacks_cannot_publish_a_ratio() { } #[test] -fn installed_package_parsers_capture_the_same_esp32_stack() { +fn build_output_package_parsers_capture_the_same_esp32_stack() { let board = BOARDS[1]; let pio = parse_platformio_packages( b"Processing esp32s3 (platform: espressif32@6.13.0; board: esp32-s3-devkitc-1; framework: arduino)\nPACKAGES:\n - framework-arduinoespressif32 @ 3.20017.241212+sha.dcc1105b\n - tool-esptoolpy @ 2.41100.260830 (4.11.0)\n - toolchain-riscv32-esp @ 8.4.0+2021r2-patch5\n - toolchain-xtensa-esp32s3 @ 8.4.0+2021r2-patch5\n", board, ); - let fbuild = parse_fbuild_packages( - br#"{"environments":[{"packages":[{"kind":"platform","name":"platform-espressif32","version":"6.13.0"},{"kind":"framework","name":"esp32-arduino","version":"3.20017.241212+sha.dcc1105b"},{"kind":"toolchain","name":"toolchain-xtensa-esp32s3","version":"8.4.0+2021r2-patch5"},{"kind":"tool","name":"tool-esptoolpy","version":"2.41100.260830"}]}]}"#, - ).unwrap(); - assert_eq!(pio, fbuild); + let fbuild = parse_fbuild_build_packages( + b"ESP32 packages: requested platform=espressif32@6.13.0, framework=old-framework@1.0; resolved platform=espressif32@6.13.0, framework=framework-arduinoespressif32@3.20017.241212+sha.dcc1105b, toolchain=toolchain-xtensa-esp32s3@8.4.0+2021r2-patch5, ESP-IDF SDK=4.4.7\n", + board, + ); + assert_eq!(pio["platform"], fbuild["platform"]); + assert_eq!(pio["framework"], fbuild["framework"]); + assert_eq!(pio["toolchain"], fbuild["toolchain"]); + assert_eq!(fbuild["sdk"], "4.4.7"); + + let mut results = sample_results(); + for result in &mut results { + result.board = "esp32s3".into(); + result.board_name = "ESP32-S3".into(); + } + results[1].resolved_packages = pio; + results[2].resolved_packages = fbuild; + assert_eq!( + board_stack_status(&results, "esp32s3"), + StackStatus::Matched + ); +} + +#[test] +fn fbuild_avr_build_log_reports_selected_package_versions() { + let packages = parse_fbuild_build_packages( + b"AVR resolved: toolchain-atmelavr@1.70300.191015 (https://example.test/toolchain); framework-arduino@1.8.8 (https://example.test/framework),\n", + BOARDS[0], + ); + assert_eq!(packages["toolchain"], "1.70300.191015"); + assert_eq!(packages["framework"], "1.8.8"); +} + +#[test] +fn changed_package_identities_across_trials_become_unverified() { + let mut packages = BTreeMap::new(); + let mut warning = None; + let first = BTreeMap::from([ + ("platform".into(), "6.13.0".into()), + ("framework".into(), "3.20017.241212+sha.dcc1105b".into()), + ("toolchain".into(), "8.4.0+2021r2-patch5".into()), + ]); + record_package_metadata( + &mut packages, + &mut warning, + first.clone(), + ToolKind::Fbuild, + BOARDS[1], + ); + assert!(package_metadata_is_complete("esp32s3", &packages)); + + let mut changed = first.clone(); + changed.insert("toolchain".into(), "14.2.0".into()); + record_package_metadata( + &mut packages, + &mut warning, + changed, + ToolKind::Fbuild, + BOARDS[1], + ); + record_package_metadata( + &mut packages, + &mut warning, + first, + ToolKind::Fbuild, + BOARDS[1], + ); + assert!(packages.is_empty()); + assert!( + warning + .as_deref() + .is_some_and(|message| message.contains("changed across cold trials")) + ); +} + +#[test] +fn missing_package_identity_in_any_cold_trial_stays_unverified() { + let complete = BTreeMap::from([ + ("platform".into(), "6.13.0".into()), + ("framework".into(), "3.20017.241212+sha.dcc1105b".into()), + ("toolchain".into(), "8.4.0+2021r2-patch5".into()), + ]); + for observations in [ + vec![BTreeMap::new(), complete.clone()], + vec![complete.clone(), BTreeMap::new()], + ] { + let mut packages = BTreeMap::new(); + let mut warning = None; + for observed in observations { + record_package_metadata( + &mut packages, + &mut warning, + observed, + ToolKind::Fbuild, + BOARDS[1], + ); + } + assert!(packages.is_empty()); + assert!( + warning + .as_deref() + .is_some_and(|message| message.contains("omitted resolved package identities")) + ); + } +} + +#[test] +fn unavailable_fbuild_package_versions_are_not_reported_as_a_mismatch() { + let mut results = sample_results(); + for result in &mut results { + result.board = "esp32s3".into(); + result.board_name = "ESP32-S3".into(); + result.resolved_packages = BTreeMap::from([ + ("platform".into(), "6.13.0".into()), + ("framework".into(), "3.20017.241212+sha.dcc1105b".into()), + ("toolchain".into(), "8.4.0+2021r2-patch5".into()), + ]); + } + results[2].resolved_packages.clear(); + results[2].package_metadata_warning = Some("build output omitted package identities".into()); + let latest = latest_payload(&sample_metadata(), &results); + assert_eq!( + latest["board_metrics"]["esp32s3"]["stack_status"], + "unverified" + ); + assert!(latest["board_metrics"]["esp32s3"]["fbuild_vs_platformio_cold"].is_null()); + assert_eq!( + latest["results"][2]["package_metadata_warning"], + "build output omitted package identities" + ); + assert!(render_svg(&sample_metadata(), &results).contains("stack unverified; ratio excluded")); + assert!( + render_html(&sample_metadata(), &results) + .contains("unverified; fbuild/PlatformIO ratio excluded") + ); } #[test]