Merge pull request #141 from rust3ds/fix/cleanup-sys-version-checking

Clean up ctru-sys version checking

.github/workflows/ci.yml

@@ -75,17 +75,17 @@ jobs:
       # unless cargo-3ds actually runs them as separate commands. See
       # https://github.com/rust3ds/cargo-3ds/issues/44 for more details
       - name: Build lib and integration tests
-        run: cargo 3ds test --no-run --tests --package ctru-rs
+        run: cargo 3ds test --no-run --tests
 
       - name: Run lib and integration tests
         uses: rust3ds/test-runner/run-tests@v1
         with:
-          args: --tests --package ctru-rs
+          args: --tests
 
       - name: Build and run doc tests
         uses: rust3ds/test-runner/run-tests@v1
         with:
-          args: --doc --package ctru-rs
+          args: --doc
 
       - name: Upload citra logs and capture videos
         uses: actions/upload-artifact@v3

ctru-rs/Cargo.toml

@@ -17,7 +17,7 @@ name = "ctru"
 
 [dependencies]
 cfg-if = "1.0"
-ctru-sys = { path = "../ctru-sys", version = "22.2" }
+ctru-sys = { path = "../ctru-sys", version = "0.5.0" }
 const-zero = "0.1.0"
 shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" }
 pthread-3ds = { git = "https://github.com/rust3ds/pthread-3ds.git" }

ctru-rs/src/error.rs

@@ -256,7 +256,13 @@ fn result_code_description_str(result: ctru_sys::Result) -> Cow<'static, str> {
         RD_NOT_AUTHORIZED => "not_authorized",
         RD_TOO_LARGE => "too_large",
         RD_INVALID_SELECTION => "invalid_selection",
-        code => return Cow::Owned(format!("(unknown description: {code:#x})")),
+        code => {
+            let error = unsafe { CStr::from_ptr(ctru_sys::osStrError(result)) }.to_str();
+            match error {
+                Ok(err) => err,
+                Err(_) => return Cow::Owned(format!("(unknown description: {code:#x})")),
+            }
+        }
     })
 }
 

ctru-sys/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ctru-sys"
-version = "22.2.0+2.2.2-1"
+version = "0.5.0"
 authors = ["Rust3DS Org", "Ronald Kinard <furyhunter600@gmail.com>"]
 description = "Raw bindings to libctru"
 repository = "https://github.com/rust3ds/ctru-rs"
@@ -18,8 +18,13 @@ libc = { version = "0.2.121", default-features = false }
 bindgen = { version = "0.65.1", features = ["experimental"] }
 cc = "1.0"
 doxygen-rs = "0.4.2"
+itertools = "0.11.0"
 which = "4.4.0"
 
+[dev-dependencies]
+shim-3ds = { git = "https://github.com/rust3ds/shim-3ds.git" }
+test-runner = { git = "https://github.com/rust3ds/test-runner.git" }
+
 [package.metadata.docs.rs]
 default-target = "armv6k-nintendo-3ds"
 targets = []

ctru-sys/README.md

@@ -10,23 +10,17 @@ to use this library.
 
 ## Version
 
-This crate's version changes according to the version of `libctru`
-used to generate the bindings, with the following convention:
-
-  * [`libctru`](https://github.com/devkitPro/libctru) version `X.Y.Z-W`
-  * `ctru-sys` version `XY.Z.P+X.Y.Z-W`
-
-  where `P` is usually 0 but may be incremented for fixes in e.g.
-  binding generation, `libc` dependency bump, etc.
-
-It may be possible to build this crate against a different version of [`libctru`](https://github.com/devkitPro/libctru),
-but you may encounter linker errors or ABI issues. A build-time Cargo warning
-(displayed when built with `-vv`) will be issued if the build script detects
-a mismatch or is unable to check the installed [`libctru`](https://github.com/devkitPro/libctru) version.
-
-## Generating bindings
-
-Bindings of new versions of [`libctru`](https://github.com/devkitPro/libctru) can be built using the integrated [`bindgen.sh`](./bindgen.sh) script.
+Crate bindings are generated at build time, so the available APIs will depend on the
+installed version of `libctru` when the crate is built. If you want to check
+what version of `libctru` is being built, you can examine these environment
+variables from your crate's build script via to its
+[`links` variables](https://doc.rust-lang.org/cargo/reference/build-scripts.html#the-links-manifest-key):
+
+* `DEP_CTRU_VERSION`: full version string (e.g. `"2.3.1-4"`)
+* `DEP_CTRU_MAJOR_VERSION`: major version (e.g. `"2"` for version `2.3.1-4`)
+* `DEP_CTRU_MINOR_VERSION`: minor version (e.g. `"3"` for version `2.3.1-4`)
+* `DEP_CTRU_PATCH_VERSION`: patch version (e.g. `"1"` for version `2.3.1-4`)
+* `DEP_CTRU_RELEASE`: release version (e.g. `"4"` for version `2.3.1-4`)
 
 ## License
 

ctru-sys/build.rs

@@ -1,5 +1,6 @@
 use bindgen::callbacks::ParseCallbacks;
 use bindgen::{Builder, RustTarget};
+use itertools::Itertools;
 
 use std::env;
 use std::error::Error;
@@ -18,33 +19,39 @@ impl ParseCallbacks for CustomCallbacks {
 fn main() {
     let devkitpro = env::var("DEVKITPRO").unwrap();
     let devkitarm = env::var("DEVKITARM").unwrap();
-    let profile = env::var("PROFILE").unwrap();
     let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
 
     println!("cargo:rerun-if-changed=build.rs");
     println!("cargo:rerun-if-env-changed=DEVKITPRO");
     println!("cargo:rustc-link-search=native={devkitpro}/libctru/lib");
-    println!(
-        "cargo:rustc-link-lib=static={}",
-        match profile.as_str() {
-            "debug" => "ctrud",
-            _ => "ctru",
+
+    // https://github.com/rust3ds/cargo-3ds/issues/14#issuecomment-1783991872
+    // To link properly, this must be the same as the library linked by cargo-3ds when building
+    // the standard library, so if `-lctru[d]` is found in RUSTFLAGS we always defer to that
+    // https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts
+    let cargo_rustflags = env::var("CARGO_ENCODED_RUSTFLAGS").unwrap();
+    let rustflags_libctru = cargo_rustflags
+        .split('\x1F')
+        // Technically this  could also be `-l ctru`, or `-lstatic=ctru` etc.
+        // but for now we'll just rely on cargo-3ds implementation to pass it like this
+        .find(|flag| flag.starts_with("-lctru"))
+        .and_then(|flag| flag.strip_prefix("-l"));
+
+    let linked_libctru = rustflags_libctru.unwrap_or_else(|| {
+        let debuginfo = env::var("DEBUG").unwrap();
+        match debuginfo.as_str() {
+            // Normally this should just be "true" or "false", but just in case,
+            // we don't support all the different options documented in
+            // https://doc.rust-lang.org/cargo/reference/profiles.html#debug
+            // so just default to linking with debuginfo if it wasn't disabled
+            "0" | "false" | "none" => "ctru",
+            _ => "ctrud",
         }
-    );
+    });
 
-    match check_libctru_version() {
-        Ok((maj, min, patch)) => {
-            eprintln!("using libctru version {maj}.{min}.{patch}");
+    println!("cargo:rustc-link-lib=static={linked_libctru}");
 
-            // These are accessible by the crate during build with `env!()`.
-            // We might consider exporting some public constants or something.
-            println!("cargo:rustc-env=LIBCTRU_VERSION={maj}.{min}.{patch}");
-            println!("cargo:rustc-env=LIBCTRU_MAJOR={maj}");
-            println!("cargo:rustc-env=LIBCTRU_MINOR={min}");
-            println!("cargo:rustc-env=LIBCTRU_PATCH={patch}");
-        }
-        Err(err) => println!("cargo:warning=failed to check libctru version: {err}"),
-    }
+    detect_and_track_libctru();
 
     let gcc_version = get_gcc_version(PathBuf::from(&devkitarm).join("bin/arm-none-eabi-gcc"));
 
@@ -135,22 +142,40 @@ fn get_gcc_version(path_to_gcc: PathBuf) -> String {
         .to_string()
 }
 
-fn parse_libctru_version(version: &str) -> Result<(String, String, String), &str> {
-    let versions: Vec<_> = version
-        .split(|c| c == '.' || c == '-')
-        .map(String::from)
-        .collect();
+fn detect_and_track_libctru() {
+    let pacman = match which::which("dkp-pacman")
+        .or_else(|err1| which::which("pacman").map_err(|err2| format!("{err1}; {err2}")))
+    {
+        Ok(pacman) => pacman,
+        Err(err) => {
+            println!("cargo:warning=unable to find `pacman` or `dkp-pacman`: {err}");
+            return;
+        }
+    };
 
-    match &versions[..] {
-        [major, minor, patch, _build] => Ok((major.clone(), minor.clone(), patch.clone())),
-        _ => Err("unexpected number of version segments"),
+    match get_libctru_version(&pacman) {
+        Ok((maj, min, patch, rel)) => {
+            let version = format!("{maj}.{min}.{patch}-{rel}");
+            eprintln!("using libctru version {version}");
+            // These are exported as build script output variables, accessible
+            // via `env::var("DEP_CTRU_<key>")` in other crates' build scripts.
+            // https://doc.rust-lang.org/cargo/reference/build-scripts.html#the-links-manifest-key
+            println!("cargo:VERSION={version}");
+            println!("cargo:MAJOR_VERSION={maj}");
+            println!("cargo:MINOR_VERSION={min}");
+            println!("cargo:PATCH_VERSION={patch}");
+            println!("cargo:RELEASE={rel}");
         }
+        Err(err) => println!("cargo:warning=unknown libctru version: {err}"),
     }
 
-fn check_libctru_version() -> Result<(String, String, String), Box<dyn Error>> {
-    let pacman = which::which("dkp-pacman").or_else(|_| which::which("pacman"))?;
+    if let Err(err) = track_libctru_files(&pacman) {
+        println!("cargo:warning=unable to track `libctru` files for changes: {err}");
+    }
+}
 
-    let Output { stdout, .. } = Command::new(&pacman)
+fn get_libctru_version(pacman: &Path) -> Result<(String, String, String, String), Box<dyn Error>> {
+    let Output { stdout, .. } = Command::new(pacman)
         .args(["--query", "libctru"])
         .stderr(Stdio::inherit())
         .output()?;
@@ -159,35 +184,44 @@ fn check_libctru_version() -> Result<(String, String, String), Box<dyn Error>> {
 
     let (_pkg, lib_version) = output_str
         .split_once(char::is_whitespace)
-        .ok_or("unexpected pacman output format")?;
+        .ok_or_else(|| format!("unexpected pacman output format: {output_str:?}"))?;
 
     let lib_version = lib_version.trim();
 
-    let cargo_pkg_version = env::var("CARGO_PKG_VERSION").unwrap();
-    let (_, crate_built_version) = cargo_pkg_version
-        .split_once('+')
-        .expect("crate version should have '+' delimeter");
+    parse_libctru_version(lib_version).map_err(Into::into)
+}
 
-    if lib_version != crate_built_version {
-        return Err(format!(
-            "libctru version is {lib_version} but this crate was built for {crate_built_version}"
-        )
-        .into());
+fn parse_libctru_version(version: &str) -> Result<(String, String, String, String), String> {
+    version
+        .split(|c| c == '.' || c == '-')
+        .map(String::from)
+        .collect_tuple()
+        .ok_or_else(|| format!("unexpected number of version segments: {version:?}"))
 }
 
-    let Output { stdout, .. } = Command::new(pacman)
+fn track_libctru_files(pacman: &Path) -> Result<(), String> {
+    let stdout = match Command::new(pacman)
         .args(["--query", "--list", "libctru"])
         .stderr(Stdio::inherit())
-        .output()?;
+        .output()
+    {
+        Ok(Output { stdout, status, .. }) if status.success() => stdout,
+        Ok(Output { status, .. }) => {
+            return Err(format!("pacman query failed with status {status}"));
+        }
+        Err(err) => {
+            return Err(format!("pacman query failed: {err}"));
+        }
+    };
 
-    for line in String::from_utf8_lossy(&stdout).split('\n') {
+    for line in String::from_utf8_lossy(&stdout).trim().split('\n') {
         let Some((_pkg, file)) = line.split_once(char::is_whitespace) else {
+            println!("cargo:warning=unexpected line from pacman query: {line:?}");
             continue;
         };
 
         println!("cargo:rerun-if-changed={file}");
     }
 
-    let (lib_major, lib_minor, lib_patch) = parse_libctru_version(lib_version)?;
-    Ok((lib_major, lib_minor, lib_patch))
+    Ok(())
 }

ctru-sys/src/lib.rs

@@ -3,6 +3,8 @@
 #![allow(non_camel_case_types)]
 #![allow(non_snake_case)]
 #![allow(clippy::all)]
+#![cfg_attr(test, feature(custom_test_frameworks))]
+#![cfg_attr(test, test_runner(test_runner::run_gdb))]
 
 pub mod result;
 pub use result::*;
@@ -15,10 +17,6 @@ pub unsafe fn errno() -> s32 {
     *__errno()
 }
 
-// TODO: not sure if there's a better way to do this, but I have gotten myself
-// with this a couple times so having the hint seems nice to have.
+// Prevent linking errors from the standard `test` library when running `cargo 3ds test --lib`.
 #[cfg(test)]
-compile_error!(concat!(
-    "ctru-sys doesn't have tests and its lib test will fail to build at link time. ",
-    "Try specifying `--package ctru-rs` to build those tests.",
-));
+extern crate shim_3ds;