From 12f0fa77d3bd499eff5d7c7b9f4f2fd2dfebb793 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sat, 28 Oct 2023 21:04:21 -0400 Subject: [PATCH 1/6] Clean up ctru-sys version checking - Don't issue a warning for "mismatched version" since the new bindings make this irrelevant - Minor build script refactoring and code cleanup - Output a `release` version (-X) for libctru env vars - Revert version to 0.5.0, which is higher than we were using before the major 22.x version but should be ok since we never released to crates.io --- ctru-rs/Cargo.toml | 2 +- ctru-sys/Cargo.toml | 3 +- ctru-sys/build.rs | 97 ++++++++++++++++++++++++++------------------- 3 files changed, 59 insertions(+), 43 deletions(-) diff --git a/ctru-rs/Cargo.toml b/ctru-rs/Cargo.toml index ac10586..3ccf64b 100644 --- a/ctru-rs/Cargo.toml +++ b/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" } diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index eba9db0..d324c4e 100644 --- a/ctru-sys/Cargo.toml +++ b/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 "] description = "Raw bindings to libctru" repository = "https://github.com/rust3ds/ctru-rs" @@ -18,6 +18,7 @@ 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" [package.metadata.docs.rs] diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index b1a48b3..f6b295c 100644 --- a/ctru-sys/build.rs +++ b/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; @@ -32,19 +33,7 @@ fn main() { } ); - match check_libctru_version() { - Ok((maj, min, patch)) => { - eprintln!("using libctru version {maj}.{min}.{patch}"); - - // 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 +124,39 @@ 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 get_libctru_version(&pacman) { + Ok((maj, min, patch, rel)) => { + eprintln!("using libctru version {maj}.{min}.{patch}-{rel}"); + + // 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}-{rel}"); + println!("cargo:rustc-env=LIBCTRU_MAJOR={maj}"); + println!("cargo:rustc-env=LIBCTRU_MINOR={min}"); + println!("cargo:rustc-env=LIBCTRU_PATCH={patch}"); + println!("cargo:rustc-env=LIBCTRU_RELEASE={rel}"); + } + Err(err) => println!("cargo:warning=unknown libctru version: {err}"), + } - match &versions[..] { - [major, minor, patch, _build] => Ok((major.clone(), minor.clone(), patch.clone())), - _ => Err("unexpected number of version segments"), + if let Err(err) = track_libctru_files(&pacman) { + println!("cargo:warning=unable to track `libctru` files for changes: {err}"); } } -fn check_libctru_version() -> Result<(String, String, String), Box> { - let pacman = which::which("dkp-pacman").or_else(|_| which::which("pacman"))?; - - let Output { stdout, .. } = Command::new(&pacman) +fn get_libctru_version(pacman: &Path) -> Result<(String, String, String, String), Box> { + let Output { stdout, .. } = Command::new(pacman) .args(["--query", "libctru"]) .stderr(Stdio::inherit()) .output()?; @@ -159,35 +165,44 @@ fn check_libctru_version() -> Result<(String, String, String), Box> { 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(()) } From 7d201d825b55d13b4d81deaa0fa1281d55aa7da5 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sun, 29 Oct 2023 00:47:53 -0400 Subject: [PATCH 2/6] Always link ctru, for now Also add a little extra description helper for unknown error types in ctru::error --- ctru-rs/src/error.rs | 8 +++++++- ctru-sys/Cargo.toml | 4 ++++ ctru-sys/build.rs | 25 +++++++++++++++++-------- ctru-sys/src/lib.rs | 10 ++++------ 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/ctru-rs/src/error.rs b/ctru-rs/src/error.rs index 0dd7cc5..dc5a421 100644 --- a/ctru-rs/src/error.rs +++ b/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})")), + } + } }) } diff --git a/ctru-sys/Cargo.toml b/ctru-sys/Cargo.toml index d324c4e..3cd651d 100644 --- a/ctru-sys/Cargo.toml +++ b/ctru-sys/Cargo.toml @@ -21,6 +21,10 @@ 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 = [] diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index f6b295c..3189455 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -19,19 +19,28 @@ 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 debuginfo = env::var("DEBUG").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", - } - ); + + let linked_libctru = 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", + // TODO 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 + // building the standard library, which is always `ctru` in practice. + // Ideally we should link `ctrud` if debug symbols are requested though: + _ => "ctru", + // _ => "ctrud", + }; + + println!("cargo:rustc-link-lib=static={linked_libctru}"); detect_and_track_libctru(); diff --git a/ctru-sys/src/lib.rs b/ctru-sys/src/lib.rs index 32252e6..34a214d 100644 --- a/ctru-sys/src/lib.rs +++ b/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; From 586f329c7a14d43674622bea623d176421157dc2 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sun, 29 Oct 2023 01:03:52 -0400 Subject: [PATCH 3/6] Test all packages in CI Since the linker issues are fixed by previous changes we should be able to run CI without `--package` and test the whole workspace instead. --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69a3978..358f2f7 100644 --- a/.github/workflows/ci.yml +++ b/.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 From b14cdf10f1c5df36cdd0dbeb4a8232d97e3e0ed3 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sun, 29 Oct 2023 21:53:59 -0400 Subject: [PATCH 4/6] Update README for new versioning scheme + env vars --- ctru-sys/README.md | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/ctru-sys/README.md b/ctru-sys/README.md index 280fd83..14febdc 100644 --- a/ctru-sys/README.md +++ b/ctru-sys/README.md @@ -10,23 +10,16 @@ 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 with [`env!`](https://doc.rust-lang.org/std/macro.env.html): + +* `LIBCTRU_VERSION`: full version string (e.g. `"2.3.1-4"`) +* `LIBCTRU_MAJOR`: major version (e.g. `"2"` for version `2.3.1-4`) +* `LIBCTRU_MINOR`: minor version (e.g. `"3"` for version `2.3.1-4`) +* `LIBCTRU_PATCH`: patch version (e.g. `"1"` for version `2.3.1-4`) +* `LIBCTRU_RELEASE`: release version (e.g. `"4"` for version `2.3.1-4`) ## License From 86bbb6fd0c738362c1d298d50fbb53d47b77262c Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sun, 19 Nov 2023 14:29:08 -0500 Subject: [PATCH 5/6] Defer to RUSTFLAGS when linking libctru --- ctru-sys/build.rs | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index 3189455..a066092 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -19,26 +19,35 @@ impl ParseCallbacks for CustomCallbacks { fn main() { let devkitpro = env::var("DEVKITPRO").unwrap(); let devkitarm = env::var("DEVKITARM").unwrap(); - let debuginfo = env::var("DEBUG").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"); - let linked_libctru = 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", - // TODO 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 - // building the standard library, which is always `ctru` in practice. - // Ideally we should link `ctrud` if debug symbols are requested though: - _ => "ctru", - // _ => "ctrud", - }; + // 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", + } + }); println!("cargo:rustc-link-lib=static={linked_libctru}"); From 0e7b139c7b8883ddd8a327f7df5f2aca5f650207 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Fri, 24 Nov 2023 17:08:47 -0500 Subject: [PATCH 6/6] Export ctru version vars via build script `rustc-env` only works for the current crate, but plain keys work for dependent crates via the `DEP_CTRU` keys, which is probably more useful in general. --- ctru-sys/README.md | 15 ++++++++------- ctru-sys/build.rs | 19 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/ctru-sys/README.md b/ctru-sys/README.md index 14febdc..4834895 100644 --- a/ctru-sys/README.md +++ b/ctru-sys/README.md @@ -13,13 +13,14 @@ to use this library. 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 with [`env!`](https://doc.rust-lang.org/std/macro.env.html): - -* `LIBCTRU_VERSION`: full version string (e.g. `"2.3.1-4"`) -* `LIBCTRU_MAJOR`: major version (e.g. `"2"` for version `2.3.1-4`) -* `LIBCTRU_MINOR`: minor version (e.g. `"3"` for version `2.3.1-4`) -* `LIBCTRU_PATCH`: patch version (e.g. `"1"` for version `2.3.1-4`) -* `LIBCTRU_RELEASE`: release version (e.g. `"4"` for version `2.3.1-4`) +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 diff --git a/ctru-sys/build.rs b/ctru-sys/build.rs index a066092..5e71ae0 100644 --- a/ctru-sys/build.rs +++ b/ctru-sys/build.rs @@ -155,15 +155,16 @@ fn detect_and_track_libctru() { match get_libctru_version(&pacman) { Ok((maj, min, patch, rel)) => { - eprintln!("using libctru version {maj}.{min}.{patch}-{rel}"); - - // 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}-{rel}"); - println!("cargo:rustc-env=LIBCTRU_MAJOR={maj}"); - println!("cargo:rustc-env=LIBCTRU_MINOR={min}"); - println!("cargo:rustc-env=LIBCTRU_PATCH={patch}"); - println!("cargo:rustc-env=LIBCTRU_RELEASE={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_")` 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}"), }