Merge pull request #2363 from Kaedo17/split/ci-alsa

ci: name the missing ALSA package instead of dying at link time

CONTRIBUTING.md

@@ -7,6 +7,22 @@ work apply to everyone, people and AI agents alike, and they are not negotiable.
 
 You need [Rust](https://rustup.rs/) 1.95 or newer.
 
+On Linux the desktop app plays preview sound through `cpal`, so the build needs a usable ALSA
+pkg-config package: an `alsa.pc` that pkg-config can find, naming the linkable ALSA runtime
+library (usually `libasound.so.2`). `alsa-sys` runs pkg-config at build time; it does not compile
+ALSA's headers. Without a usable package the build fails at link time with an unrelated-looking
+error; `cargo xtask ci` probes pkg-config up front and names the package:
+
+```sh
+sudo dnf install alsa-lib-devel        # Fedora
+sudo apt install libasound2-dev        # Debian / Ubuntu
+sudo pacman -S alsa-lib                # Arch
+```
+
+If you cannot install the package, point `PKG_CONFIG_PATH` at a directory holding an `alsa.pc`
+whose `Libs` line names your system's `libasound` (usually `libasound.so.2`); every CI run needs
+that variable set.
+
 ```sh
 cargo run -p effectcraft -- --demo       # the desktop app, with the demo project
 cargo test --workspace

Cargo.lock

@@ -7199,6 +7199,7 @@ name = "xtask"
 version = "0.18.0"
 dependencies = [
  "chrono",
+ "pkg-config",
  "serde_json",
  "tempfile",
 ]

xtask/Cargo.toml

@@ -13,3 +13,4 @@ serde_json = { workspace = true }
 
 chrono = { workspace = true }
 tempfile = "3"
+pkg-config = "0.3"

xtask/src/main.rs

@@ -407,6 +407,62 @@ fn serve_dir(dir: &std::path::Path, port: u16) -> Result<(), String> {
     Ok(())
 }
 
+/// The guidance printed when the ALSA pkg-config package is missing. Split out so the tests can
+/// check the package names without needing a machine that is actually missing them.
+#[cfg(target_os = "linux")]
+fn audio_backend_missing_hint(pkg_config_stderr: Option<&str>) -> String {
+    let mut hint = String::from(
+        "Linux audio needs the ALSA development package, and pkg-config cannot find it.\n\
+         The desktop app plays preview sound through cpal, whose alsa-sys dependency asks\n\
+         pkg-config for `alsa`; without the package the build dies minutes later with an\n\
+         unrelated-looking linker error.\n\
+         \n\
+         Fedora:         sudo dnf install alsa-lib-devel\n\
+         Debian/Ubuntu:  sudo apt install libasound2-dev\n\
+         Arch:           sudo pacman -S alsa-lib\n\
+         \n\
+         Install one of those and run `cargo xtask ci` again. If you cannot install it, point\n\
+         PKG_CONFIG_PATH at a directory holding an `alsa.pc` whose Libs line names your system's\n\
+         libasound (usually `libasound.so.2`); every CI run will need that variable set.",
+    );
+    if let Some(stderr) = pkg_config_stderr {
+        hint.push_str("\n\npkg-config said: ");
+        hint.push_str(stderr);
+    }
+    hint
+}
+
+/// `cargo xtask ci` builds the desktop app on the host, so on Linux it needs a usable ALSA
+/// pkg-config package before the build dies minutes later at link time. The probe goes through
+/// the same `pkg-config` crate alsa-sys's build script uses, so the executable and search-path
+/// lookup match by construction: `PKG_CONFIG` and its target-scoped variants, then `pkg-config`
+/// on `PATH` with a `pkgconf` fallback, plus the scoped `PKG_CONFIG_PATH`/`LIBDIR`/`SYSROOT_DIR`.
+/// A probe that ran and said no is "ALSA missing"; one that could not run is reported separately,
+/// so a machine with only `pkgconf` does not fail a build that would succeed.
+#[cfg(target_os = "linux")]
+fn audio_backend() -> Result<(), String> {
+    // cargo_metadata off: xtask is not a build script, so the crate's rerun-if-env lines would
+    // only clutter the ci log.
+    match pkg_config::Config::new().cargo_metadata(false).probe("alsa") {
+        Ok(_) => Ok(()),
+        Err(pkg_config::Error::ProbeFailure { output, .. }) | Err(pkg_config::Error::Failure { output, .. }) => {
+            let stderr = String::from_utf8_lossy(&output.stderr);
+            let stderr = stderr.trim();
+            Err(audio_backend_missing_hint((!stderr.is_empty()).then_some(stderr)))
+        }
+        Err(e) => Err(format!(
+            "The pkg-config probe for `alsa` could not be run: {e}. Install pkg-config (or pkgconf), \
+             or point PKG_CONFIG at a working one — it is the same lookup alsa-sys's pkg-config \
+             dependency performs."
+        )),
+    }
+}
+
+#[cfg(not(target_os = "linux"))]
+fn audio_backend() -> Result<(), String> {
+    Ok(())
+}
+
 /// Lib, bin and integration tests (examples aren't linked; clippy checked them), then doctests,
 /// which nextest doesn't run.
 ///
@@ -438,7 +494,8 @@ type Stage = fn() -> Result<(), String>;
 
 /// Every stage runs even when an earlier one fails, so one run reports all the problems.
 fn ci() -> Result<(), String> {
-    let stages: [(&str, Stage); 7] = [
+    let stages: [(&str, Stage); 8] = [
+        ("audio backend", audio_backend),
         ("fmt", || run(Command::new(env!("CARGO")).args(["fmt", "--check"]))),
         ("clippy", || run(Command::new(env!("CARGO")).args(["clippy", "--workspace", "--all-targets", "--", "-D", "warnings"]))),
         ("tests", tests),
@@ -459,6 +516,7 @@ fn main() -> ExitCode {
         "assets" => assets(),
         "pdf-deps" => pdf_deps::run(&root()),
         "ci" => ci(),
+        "audio-backend" => audio_backend(),
         "web" => web(&std::env::args().skip(2).collect::<Vec<_>>()),
         "bench" => bench::run(&root(), &std::env::args().skip(2).collect::<Vec<_>>()),
         "ico" => {
@@ -469,7 +527,7 @@ fn main() -> ExitCode {
             let rest: Vec<String> = std::env::args().skip(2).collect();
             version::run(&root(), &rest.iter().map(String::as_str).collect::<Vec<_>>())
         }
-        _ => Err("usage: cargo xtask <layers|assets|pdf-deps|wasm|web [--dev] [--serve PORT]|ico OUT.ico IN.png…|version [set X.Y.Z]|ci|bench [--bin CLI] [--quick] [--project F --comp C] [--compare OLD.jsonl] (--help)>".into()),
+        _ => Err("usage: cargo xtask <layers|assets|pdf-deps|wasm|web [--dev] [--serve PORT]|ico OUT.ico IN.png…|version [set X.Y.Z]|ci|audio-backend|bench [--bin CLI] [--quick] [--project F --comp C] [--compare OLD.jsonl] (--help)>".into()),
     };
     match r {
         Ok(()) => ExitCode::SUCCESS,
@@ -484,3 +542,22 @@ fn main() -> ExitCode {
 pub fn root() -> std::path::PathBuf {
     std::path::Path::new(env!("CARGO_MANIFEST_DIR")).parent().expect("xtask has a parent dir").to_path_buf()
 }
+
+#[cfg(all(test, target_os = "linux"))]
+mod tests {
+    use super::audio_backend_missing_hint;
+
+    #[test]
+    fn alsa_missing_names_the_package_for_each_distro() {
+        let e = audio_backend_missing_hint(None);
+        for want in ["alsa-lib-devel", "libasound2-dev", "alsa-lib", "pkg-config", "PKG_CONFIG_PATH"] {
+            assert!(e.contains(want), "hint lost `{want}`:\n{e}");
+        }
+    }
+
+    #[test]
+    fn alsa_missing_appends_pkg_config_s_own_detail() {
+        let e = audio_backend_missing_hint(Some("Package 'alsa' not found"));
+        assert!(e.contains("pkg-config said: Package 'alsa' not found"), "detail lost:\n{e}");
+    }
+}

xtask/tests/it/audio_probe.rs

@@ -0,0 +1,126 @@
+//! The Linux audio-backend preflight against fake pkg-config tools.
+//!
+//! The probe resolves its executable through the pkg-config crate, which reads the environment,
+//! so the fake-tool scenarios run the real `xtask audio-backend` binary in a child process with a
+//! controlled environment: `Command::env` changes only the child's environment, so the tests need
+//! no global env mutation and stay parallel-safe.
+
+use std::ffi::OsStr;
+use std::path::PathBuf;
+use std::process::{Command, Output};
+
+/// A scratch directory holding the fake tools for one scenario: `dir` is the child's `PATH`,
+/// `off_path` holds executables reachable only by an explicit `*_PKG_CONFIG` override.
+struct FakeTools {
+    dir: PathBuf,
+    off_path: PathBuf,
+}
+
+impl FakeTools {
+    fn new(tag: &str) -> Self {
+        let base = std::env::temp_dir().join(format!("effectcraft-xtask-audio-{tag}-{}", std::process::id()));
+        let dir = base.join("bin");
+        let off_path = base.join("off-path");
+        std::fs::create_dir_all(&dir).expect("create PATH dir");
+        std::fs::create_dir_all(&off_path).expect("create off-PATH dir");
+        Self { dir, off_path }
+    }
+
+    /// Write a plain (not executable) file.
+    fn file(&self, on_path: bool, name: &str, body: &str) -> PathBuf {
+        let path = if on_path { self.dir.clone() } else { self.off_path.clone() }.join(name);
+        std::fs::write(&path, body).expect("write fake tool");
+        path
+    }
+
+    /// Write an executable shell script.
+    fn script(&self, on_path: bool, name: &str, body: &str) -> PathBuf {
+        use std::os::unix::fs::PermissionsExt;
+        let path = self.file(on_path, name, body);
+        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).expect("chmod fake tool");
+        path
+    }
+
+    /// A fake `pkgconf` that answers as alsa 1.2.11 linking -lasound.
+    fn working_pkgconf(&self, on_path: bool) -> PathBuf {
+        self.script(on_path, "pkgconf", "#!/bin/sh\ncase \" $* \" in\n  *\" --modversion \"*) echo 1.2.11 ;;\n  *) echo -lasound ;;\nesac\n")
+    }
+}
+
+impl Drop for FakeTools {
+    fn drop(&mut self) {
+        let _ = std::fs::remove_dir_all(&self.dir);
+        let _ = std::fs::remove_dir_all(&self.off_path);
+    }
+}
+
+/// Run the built `xtask audio-backend` with `dir` as the only `PATH`, the given variables set,
+/// and every variable the probe reads otherwise cleared, so a developer's own environment
+/// cannot leak into the scenario.
+fn run_audio_backend(tools: &FakeTools, set: &[(&str, &OsStr)]) -> Output {
+    Command::new(env!("CARGO_BIN_EXE_xtask"))
+        .arg("audio-backend")
+        .env("PATH", &tools.dir)
+        .env_remove("PKG_CONFIG")
+        .env_remove("PKG_CONFIG_PATH")
+        .env_remove("PKG_CONFIG_LIBDIR")
+        .env_remove("PKG_CONFIG_SYSROOT_DIR")
+        .env_remove("PKG_CONFIG_ALLOW_CROSS")
+        .env_remove("ALSA_NO_PKG_CONFIG")
+        .env_remove("HOST_PKG_CONFIG")
+        .env_remove("TARGET_PKG_CONFIG")
+        .env_remove("HOST")
+        .env_remove("TARGET")
+        .envs(set.iter().copied())
+        .output()
+        .expect("run xtask audio-backend")
+}
+
+#[test]
+fn a_host_with_only_pkgconf_still_finds_alsa() {
+    // The reviewed case: PKG_CONFIG unset, no pkg-config on PATH, only pkgconf. The pkg-config
+    // crate alsa-sys uses falls back to pkgconf, so the stage must stay green.
+    let tools = FakeTools::new("pkgconf-only");
+    tools.working_pkgconf(true);
+    let out = run_audio_backend(&tools, &[]);
+    assert!(out.status.success(), "pkgconf-only host must pass:\n{}", String::from_utf8_lossy(&out.stderr));
+}
+
+#[test]
+fn a_probe_that_runs_and_says_no_names_the_package() {
+    let tools = FakeTools::new("says-no");
+    tools.script(true, "pkg-config", "#!/bin/sh\necho 'Package alsa was not found' >&2\nexit 1\n");
+    let out = run_audio_backend(&tools, &[]);
+    assert!(!out.status.success());
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    for want in ["alsa-lib-devel", "libasound2-dev", "alsa-lib", "pkg-config"] {
+        assert!(stderr.contains(want), "hint lost `{want}`:\n{stderr}");
+    }
+    assert!(stderr.contains("not found"), "pkg-config's own detail should be appended:\n{stderr}");
+}
+
+#[test]
+fn a_probe_that_cannot_run_is_not_alsa_missing() {
+    // PKG_CONFIG names a file that is not an executable; with PKG_CONFIG set the crate does not
+    // fall back to pkgconf, so the lookup fails to launch — reported separately from "missing".
+    let tools = FakeTools::new("cannot-run");
+    let broken = tools.file(true, "not-an-executable", "this is not a program\n");
+    let out = run_audio_backend(&tools, &[("PKG_CONFIG", broken.as_os_str())]);
+    assert!(!out.status.success());
+    let stderr = String::from_utf8_lossy(&out.stderr);
+    assert!(stderr.contains("could not"), "expected the launch-failure error:\n{stderr}");
+    assert!(!stderr.contains("alsa-lib-devel"), "must not read as ALSA missing:\n{stderr}");
+}
+
+#[test]
+fn a_host_scoped_override_selects_that_executable() {
+    // With TARGET and HOST set (build-script style), HOST_PKG_CONFIG wins over the PATH lookup,
+    // and here it is the only working tool — so success proves the override was used.
+    let tools = FakeTools::new("scoped");
+    let good = tools.working_pkgconf(false);
+    let out = run_audio_backend(
+        &tools,
+        &[("HOST", OsStr::new("x86_64-unknown-linux-gnu")), ("TARGET", OsStr::new("x86_64-unknown-linux-gnu")), ("HOST_PKG_CONFIG", good.as_os_str())],
+    );
+    assert!(out.status.success(), "HOST_PKG_CONFIG override must be honored:\n{}", String::from_utf8_lossy(&out.stderr));
+}

xtask/tests/it/main.rs

@@ -0,0 +1,4 @@
+//! xtask's integration tests: one binary, modules in `tests/it/`.
+
+#[cfg(target_os = "linux")]
+mod audio_probe;