From 336c8cdc9d732307f887e17d758a20d32a3ce51e Mon Sep 17 00:00:00 2001 From: zackees Date: Tue, 30 Jun 2026 20:00:29 -0700 Subject: [PATCH 1/4] fix(clippy): 4 residual lints from #618 squash-merge on main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #618 sync module and #912's backslash-rewrite fix were combined into a single squash-merge (ecd80c74). Four clippy fixes I'd already shipped on the sync branch didn't ride along, so main's `Check macOS` run is red on `-D warnings`. Reapplying: - fbuild-serial/src/boards.rs — field-reassign after Default::default() in the 1200bps-touch upload-hint test (from #906 carryover). Use struct-literal syntax + ..Default::default(). - fbuild-cli/src/sync/source.rs — swap the closure form `.rsplit(|c| c == '/' || c == '\')` for the array-pattern form `.rsplit(['/', '\'])`. Same behavior, satisfies clippy::manual_pattern_char_comparison. - fbuild-cli/src/sync/mod.rs — fold the nested `if !prompt_multi_env(..)` into the outer condition (clippy::collapsible_if). Same behavior. - fbuild-cli/src/cli/sync_cmd.rs — add `#[allow(clippy::too_many_arguments)]` on run_sync_cmd; the 8 args mirror clap's parsed variant 1:1 and wrapping them in a struct here would just push the noise into the dispatch site. No behavior changes. --- crates/fbuild-cli/src/cli/sync_cmd.rs | 1 + crates/fbuild-cli/src/sync/mod.rs | 9 +++++---- crates/fbuild-cli/src/sync/source.rs | 5 +---- crates/fbuild-serial/src/boards.rs | 6 ++++-- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/fbuild-cli/src/cli/sync_cmd.rs b/crates/fbuild-cli/src/cli/sync_cmd.rs index 8532ac87a..0fad2354b 100644 --- a/crates/fbuild-cli/src/cli/sync_cmd.rs +++ b/crates/fbuild-cli/src/cli/sync_cmd.rs @@ -10,6 +10,7 @@ use std::path::PathBuf; use crate::sync::{run_sync, SyncArgs, SyncOutcome}; /// Adapter for `Commands::Sync` — invoked from `cli::dispatch`. +#[allow(clippy::too_many_arguments)] // Matches clap's parsed variant fields 1:1. pub async fn run_sync_cmd( project_dir: Option, environment: Option, diff --git a/crates/fbuild-cli/src/sync/mod.rs b/crates/fbuild-cli/src/sync/mod.rs index dcf289201..eef54dff7 100644 --- a/crates/fbuild-cli/src/sync/mod.rs +++ b/crates/fbuild-cli/src/sync/mod.rs @@ -187,10 +187,11 @@ async fn do_run_sync(args: SyncArgs) -> Result { let selected_envs = select_envs(&all_envs, args.environment.as_deref())?; // Multi-env prompt (skipped by --yes / --check / -e ). - if selected_envs.len() > 1 && !args.skip_multi_env_prompt() { - if !prompt_multi_env(&selected_envs) { - return Ok(SyncOutcome::UserCancelled); - } + if selected_envs.len() > 1 + && !args.skip_multi_env_prompt() + && !prompt_multi_env(&selected_envs) + { + return Ok(SyncOutcome::UserCancelled); } // Classify every env's lib_deps. diff --git a/crates/fbuild-cli/src/sync/source.rs b/crates/fbuild-cli/src/sync/source.rs index 38af6d6b2..dc7f5ae32 100644 --- a/crates/fbuild-cli/src/sync/source.rs +++ b/crates/fbuild-cli/src/sync/source.rs @@ -231,10 +231,7 @@ pub fn classify(raw: &str) -> ClassifiedDep { /// `.git`. Never returns empty — falls back to the input. fn last_segment(s: &str) -> &str { let end = s.trim_end_matches(['/', '\\']); - let seg = end - .rsplit(|c| c == '/' || c == '\\') - .next() - .unwrap_or(end); + let seg = end.rsplit(['/', '\\']).next().unwrap_or(end); let seg = seg.trim_end_matches(".git"); if seg.is_empty() { end diff --git a/crates/fbuild-serial/src/boards.rs b/crates/fbuild-serial/src/boards.rs index ec7e6b907..02397f92c 100644 --- a/crates/fbuild-serial/src/boards.rs +++ b/crates/fbuild-serial/src/boards.rs @@ -1135,8 +1135,10 @@ mod tests { // No protocol string, but the touch flag is a strong // ecosystem-standard signal in its own right (every product // shipping this flag semantically means "1200-bps touch reset"). - let mut h = UploadHint::default(); - h.use_1200bps_touch = Some(true); + let h = UploadHint { + use_1200bps_touch: Some(true), + ..Default::default() + }; assert_eq!( family_from_upload_hint(&h), Some(BoardFamily::NativeUsbCdcReset1200Bps) From f6c2436369250053ace7678e94699ae4aec68f35 Mon Sep 17 00:00:00 2001 From: zackees Date: Tue, 30 Jun 2026 20:04:09 -0700 Subject: [PATCH 2/4] fix(serial): Linux port_name_stem privacy re-export Main's Ubuntu Check is red on E0364/E0603: the fbuild-serial `port_name_stem` fn is declared with no visibility, so the sibling `#[cfg(test)] pub(crate) use port_name_stem as port_name_stem_for_tests;` tries to re-export a private symbol. Loosen just enough for the re-export to type-check: - Make `port_name_stem` `pub(super)` so `port_class::linux` can hand it up to the sibling test block in `port_class` itself. - Downgrade the re-export from `pub(crate)` to `pub(super)` since only the parent module ever consumes it. No behavior change. Verified locally: `soldr cargo check --workspace --all-targets` and `soldr cargo test --workspace --lib` both pass. --- crates/fbuild-serial/src/port_class.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fbuild-serial/src/port_class.rs b/crates/fbuild-serial/src/port_class.rs index 8c7720575..f3448dbc1 100644 --- a/crates/fbuild-serial/src/port_class.rs +++ b/crates/fbuild-serial/src/port_class.rs @@ -181,7 +181,7 @@ mod linux { /// Strip `/dev/` (or `/devices/` in some odd configurations) and /// return the bare port name, e.g. `ttyACM0`. - fn port_name_stem(port_name: &str) -> Option<&str> { + pub(super) fn port_name_stem(port_name: &str) -> Option<&str> { if let Some(stem) = port_name.strip_prefix("/dev/") { return Some(stem); } @@ -220,7 +220,7 @@ mod linux { } #[cfg(test)] - pub(crate) use port_name_stem as port_name_stem_for_tests; + pub(super) use port_name_stem as port_name_stem_for_tests; } #[cfg(target_os = "macos")] From e9e51a3b474c69f1148e3472f179a40384e96447 Mon Sep 17 00:00:00 2001 From: zackees Date: Tue, 30 Jun 2026 23:12:09 -0700 Subject: [PATCH 3/4] fix(ci): regen nightly-platforms.yml from SOT (drift on main) Render-and-diff has been failing on main across dbc2245e/ecd80c74/ b0c36225 because .github/workflows/nightly-platforms.yml drifted from its ci/render_workflows.py source-of-truth. The regen produces a 1-line diff (removes a `timeout-minutes: 5` on the `guard` job that was hand-added post-render). Roll the fix into this PR since I have the CI red anyway and unblocking one of main's four pre-existing failure signals is cheap. Doesn't touch the sync module or the clippy fixes this PR was originally about. --- .github/workflows/nightly-platforms.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/nightly-platforms.yml b/.github/workflows/nightly-platforms.yml index a6beb3ee0..140118d84 100644 --- a/.github/workflows/nightly-platforms.yml +++ b/.github/workflows/nightly-platforms.yml @@ -21,7 +21,6 @@ jobs: guard: name: Guard (skip on quiet days) runs-on: ubuntu-latest - timeout-minutes: 5 outputs: should_run: ${{ steps.check.outputs.should_run }} steps: From 24dd508435db642e544762e19ce97f691b7d1a12 Mon Sep 17 00:00:00 2001 From: zackees Date: Tue, 30 Jun 2026 23:30:09 -0700 Subject: [PATCH 4/4] fix(tests): unmask 4 hidden failures the port_name_stem fix now exposes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main's check-ubuntu was RED on the compile error this PR now fixes (port_name_stem privacy), so the fbuild-cli test binary could never build on Linux CI. Once the compile succeeds, 4 previously-hidden test failures surface — none of which is caused by my clippy/privacy changes, but all of which are on the CI green-path I need for merge. 1. `sync::source::tests::github_case_insensitive_host` — real bug in `classify()`: line-167 dispatch prefix check was case-sensitive `starts_with("http://")` / `starts_with("https://")`, so `HTTPS://GITHUB.COM/…` fell through step 4 into step 6 (Registry). The inner `is_github_url` already lowercases before checking, so the intent was clearly case-insensitive at both layers. Fix: add `starts_with_ci()` helper and switch the outer check to it. Matches RFC 3986 §3.1 (URI scheme is case-insensitive). 2. `update_check::tests::ci_detection_via_ci_env` — test scrubbed only `CI` before its `assert!(!is_ci_env())` checks, but `is_ci_env()` also consults `GITHUB_ACTIONS`, `GITLAB_CI`, `CIRCLECI`, `JENKINS_URL`. On GitHub Actions `GITHUB_ACTIONS=true` is baked in, so the assertion always fails there. Fix: snapshot + clear the full set before the negative checks, restore at end. 3+4. `cli::clangd_config::tests::query_driver_glob_uses_bin_dir_forward_slashes` and `clangd_yaml_mentions_compiler_and_database` — both feed a raw `C:\tc\bin\…` literal through `NormalizedPath::display_slash()`, which only converts `\` → `/` on Windows targets (on Linux `\` is a valid filename byte). Gate the Windows-input arms to `#[cfg(windows)]` and split the POSIX arm out into its own test so it stays portable. Verified locally: - `soldr cargo test -p fbuild-cli --lib sync::source::tests::github_case_insensitive_host update_check::tests::ci_detection_via_ci_env cli::clangd_config::tests::query_driver_glob_uses_bin_dir_forward_slashes_posix cli::clangd_config::tests::clangd_yaml_mentions_compiler_and_database` — exit 0 - `soldr cargo test --workspace --lib --no-fail-fast` — exit 0 - `soldr cargo clippy --workspace --all-targets -- -D warnings` — exit 0 --- crates/fbuild-cli/src/cli/clangd_config.rs | 22 +++++++++++++++++----- crates/fbuild-cli/src/sync/source.rs | 14 ++++++++++++-- crates/fbuild-cli/src/update_check.rs | 19 ++++++++++++++----- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/crates/fbuild-cli/src/cli/clangd_config.rs b/crates/fbuild-cli/src/cli/clangd_config.rs index d81d07f2a..40e9ba03b 100644 --- a/crates/fbuild-cli/src/cli/clangd_config.rs +++ b/crates/fbuild-cli/src/cli/clangd_config.rs @@ -318,18 +318,30 @@ mod tests { ); } + // The Windows-input arms of these two tests feed a raw `C:\tc\...` + // literal through `NormalizedPath::display_slash()`, which only + // converts `\` → `/` on Windows targets. On Linux, `\` is a valid + // filename byte, so the normalizer leaves it alone and the asserted + // `"C:/tc/bin/*"` shape never materializes. Gate the Windows arms + // behind `#[cfg(windows)]` and keep the POSIX arm portable. #[test] - fn query_driver_glob_uses_bin_dir_forward_slashes() { - assert_eq!( - compiler_query_driver_glob(r"C:\tc\bin\avr-g++.exe"), - "C:/tc/bin/*" - ); + fn query_driver_glob_uses_bin_dir_forward_slashes_posix() { assert_eq!( compiler_query_driver_glob("/home/u/.platformio/packages/tc/bin/arm-none-eabi-g++"), "/home/u/.platformio/packages/tc/bin/*" ); } + #[cfg(windows)] + #[test] + fn query_driver_glob_uses_bin_dir_forward_slashes_windows() { + assert_eq!( + compiler_query_driver_glob(r"C:\tc\bin\avr-g++.exe"), + "C:/tc/bin/*" + ); + } + + #[cfg(windows)] #[test] fn clangd_yaml_mentions_compiler_and_database() { let yaml = render_clangd_yaml(r"C:\tc\bin\avr-g++"); diff --git a/crates/fbuild-cli/src/sync/source.rs b/crates/fbuild-cli/src/sync/source.rs index dc7f5ae32..dcdb06b0d 100644 --- a/crates/fbuild-cli/src/sync/source.rs +++ b/crates/fbuild-cli/src/sync/source.rs @@ -163,8 +163,12 @@ pub fn classify(raw: &str) -> ClassifiedDep { }; } - // 4. HTTP/HTTPS — could be GitHub or a plain archive. - if trimmed.starts_with("http://") || trimmed.starts_with("https://") { + // 4. HTTP/HTTPS — could be GitHub or a plain archive. Case-insensitive + // on the scheme because PlatformIO's `lib_deps` treats URLs as + // URIs per RFC 3986 §3.1 (scheme is case-insensitive), and the + // `github_case_insensitive_host` test expects `HTTPS://GITHUB.COM/...` + // to still classify as Github. + if starts_with_ci(trimmed, "http://") || starts_with_ci(trimmed, "https://") { // Split off the optional `#` (only meaningful for repo URLs; // if it's on an archive URL it's harmless noise). let (url_no_ref, hash_suffix) = split_ref(trimmed); @@ -227,6 +231,12 @@ pub fn classify(raw: &str) -> ClassifiedDep { // ---------- helpers ---------- +/// ASCII case-insensitive `starts_with`, used for the RFC 3986 URI +/// scheme prefix check in `classify()`. +fn starts_with_ci(s: &str, prefix: &str) -> bool { + s.len() >= prefix.len() && s.as_bytes()[..prefix.len()].eq_ignore_ascii_case(prefix.as_bytes()) +} + /// Take the last `/` or `\` segment of a string, trimming any trailing /// `.git`. Never returns empty — falls back to the input. fn last_segment(s: &str) -> &str { diff --git a/crates/fbuild-cli/src/update_check.rs b/crates/fbuild-cli/src/update_check.rs index 391f51273..5c74b7e39 100644 --- a/crates/fbuild-cli/src/update_check.rs +++ b/crates/fbuild-cli/src/update_check.rs @@ -736,9 +736,16 @@ mod tests { #[test] fn ci_detection_via_ci_env() { - // Snapshot env to restore after test. - let saved = std::env::var("CI").ok(); + // Snapshot + clear ALL CI markers we recognize, not just CI — + // otherwise GitHub Actions' own GITHUB_ACTIONS=true poisons the + // `assert!(!is_ci_env())` checks below. + const CI_KEYS: &[&str] = &["CI", "GITHUB_ACTIONS", "GITLAB_CI", "CIRCLECI", "JENKINS_URL"]; + let saved: Vec<(&str, Option)> = + CI_KEYS.iter().map(|k| (*k, std::env::var(*k).ok())).collect(); // SAFETY: single-threaded test process. + for k in CI_KEYS { + std::env::remove_var(k); + } std::env::set_var("CI", "true"); assert!(is_ci_env()); std::env::set_var("CI", "0"); @@ -748,9 +755,11 @@ mod tests { std::env::remove_var("CI"); assert!(!is_ci_env()); // Restore. - match saved { - Some(v) => std::env::set_var("CI", v), - None => std::env::remove_var("CI"), + for (k, v) in saved { + match v { + Some(val) => std::env::set_var(k, val), + None => std::env::remove_var(k), + } } } }