From 23dcc8f49e43e463dffec52af80048cef0b3534b Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 29 Feb 2024 11:03:48 -0800 Subject: [PATCH] Use only predefined environment variables to compute name prefixes. Instead of having build.rs set a custom environment variable, just compute the value directly from environment variables that are guaranteed to be set. This way, non-Cargo build systems can avoid duplicating this custom logic. Move the consistency check of `links` and the package version so that the check is also done during pregeneration. Use `read_env_var()` for the relevant environment variables so that build.rs emits `cargo:rerun-if-env-changed`, as it was noticed that Cargo otherwise doesn't seem to rerun the build script when the Cargo.toml values are changed. --- Cargo.toml | 10 +++--- build.rs | 87 +++++++++++++++++++++++++++++++------------------ src/prefixed.rs | 26 ++++++++++++++- 3 files changed, 85 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b48593341..9f2d0513e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,12 +18,10 @@ version = "0.17.8" # Keep in sync with `version` above. # -# "ring_core_" + version, replacing punctuation with underscores. -# -# build.rs uses this to derive the prefix for FFI symbols and the file names -# of the FFI libraries, so it must be a valid identifier prefix and a valid -# filename prefix. -links = "ring_core_0_17_8" +# build.rs verifies that this equals "ring_core_{major}_{minor}_{patch}_{pre}" +# as keeping this in sync with the symbol prefixing is crucial for ensuring +# the safety of multiple versions of *ring* being used in a program. +links = "ring_core_0_17_8_" include = [ "LICENSE", diff --git a/build.rs b/build.rs index 54dc44fb9..169c74a9a 100644 --- a/build.rs +++ b/build.rs @@ -269,19 +269,35 @@ fn main() { env::var_os("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR should always be set"), ); + // Keep in sync with `core_name_and_version!` in prefixed.rs. + let core_name_and_version = [ + &env::var("CARGO_PKG_NAME").unwrap(), + "core", + &env::var("CARGO_PKG_VERSION_MAJOR").unwrap(), + &env::var("CARGO_PKG_VERSION_MINOR").unwrap(), + &env::var("CARGO_PKG_VERSION_PATCH").unwrap(), + &env::var("CARGO_PKG_VERSION_PRE").unwrap(), // Often empty + ] + .join("_"); + // Ensure `links` in Cargo.toml is consistent with the version. + assert_eq!( + &env::var("CARGO_MANIFEST_LINKS").unwrap(), + &core_name_and_version + ); + const RING_PREGENERATE_ASM: &str = "RING_PREGENERATE_ASM"; match env::var_os(RING_PREGENERATE_ASM).as_deref() { Some(s) if s == "1" => { - pregenerate_asm_main(&c_root_dir); + pregenerate_asm_main(&c_root_dir, &core_name_and_version); } - None => ring_build_rs_main(&c_root_dir), + None => ring_build_rs_main(&c_root_dir, &core_name_and_version), _ => { panic!("${} has an invalid value", RING_PREGENERATE_ASM); } } } -fn ring_build_rs_main(c_root_dir: &Path) { +fn ring_build_rs_main(c_root_dir: &Path, core_name_and_version: &str) { let out_dir = env::var_os("OUT_DIR").unwrap(); let out_dir = PathBuf::from(out_dir); @@ -323,7 +339,12 @@ fn ring_build_rs_main(c_root_dir: &Path) { let generated_dir = if !is_git { c_root_dir.join(PREGENERATED) } else { - generate_sources_and_preassemble(&out_dir, asm_target.into_iter(), c_root_dir); + generate_sources_and_preassemble( + &out_dir, + asm_target.into_iter(), + c_root_dir, + core_name_and_version, + ); out_dir.clone() }; @@ -333,25 +354,31 @@ fn ring_build_rs_main(c_root_dir: &Path) { &generated_dir, c_root_dir, &out_dir, - &ring_core_prefix(), + core_name_and_version, ); emit_rerun_if_changed() } -fn pregenerate_asm_main(c_root_dir: &Path) { +fn pregenerate_asm_main(c_root_dir: &Path, core_name_and_version: &str) { println!("cargo:rustc-cfg=pregenerate_asm_only"); let pregenerated = c_root_dir.join(PREGENERATED); std::fs::create_dir(&pregenerated).unwrap(); - generate_sources_and_preassemble(&pregenerated, ASM_TARGETS.iter(), c_root_dir); + generate_sources_and_preassemble( + &pregenerated, + ASM_TARGETS.iter(), + c_root_dir, + core_name_and_version, + ); } fn generate_sources_and_preassemble<'a>( out_dir: &Path, asm_targets: impl Iterator, c_root_dir: &Path, + core_name_and_version: &str, ) { - generate_prefix_symbols_headers(out_dir, &ring_core_prefix()).unwrap(); + generate_prefix_symbols_headers(out_dir, core_name_and_version).unwrap(); let perl_exe = get_perl_exe(); @@ -391,10 +418,8 @@ fn build_c_code( generated_dir: &Path, c_root_dir: &Path, out_dir: &Path, - ring_core_prefix: &str, + core_name_and_version: &str, ) { - println!("cargo:rustc-env=RING_CORE_PREFIX={}", ring_core_prefix); - let (asm_srcs, obj_srcs) = if let Some(asm_target) = asm_target { let perlasm_src_dsts = perlasm_src_dsts(generated_dir, asm_target); @@ -436,21 +461,30 @@ fn build_c_code( let test_srcs = RING_TEST_SRCS.iter().map(PathBuf::from).collect::>(); let libs = [ - ("", &core_srcs[..], &asm_srcs[..], &obj_srcs[..]), - ("test", &test_srcs[..], &[], &[]), + ( + core_name_and_version, + &core_srcs[..], + &asm_srcs[..], + &obj_srcs[..], + ), + ( + &(String::from(core_name_and_version) + "_test"), + &test_srcs[..], + &[], + &[], + ), ]; // XXX: Ideally, ring-test would only be built for `cargo test`, but Cargo // can't do that yet. libs.iter() - .for_each(|&(lib_name_suffix, srcs, asm_srcs, obj_srcs)| { - let lib_name = String::from(ring_core_prefix) + lib_name_suffix; + .for_each(|&(lib_name, srcs, asm_srcs, obj_srcs)| { let srcs = srcs.iter().chain(asm_srcs); build_library( target, c_root_dir, out_dir, - &lib_name, + lib_name, srcs, generated_dir, obj_srcs, @@ -740,25 +774,16 @@ fn walk_dir(dir: &Path, cb: &impl Fn(&DirEntry)) { } } -fn ring_core_prefix() -> String { - let links = env::var("CARGO_MANIFEST_LINKS").unwrap(); - - let computed = { - let name = env::var("CARGO_PKG_NAME").unwrap(); - let version = env::var("CARGO_PKG_VERSION").unwrap(); - name + "_core_" + &version.replace(&['-', '.'][..], "_") - }; - - assert_eq!(links, computed); - - links + "_" -} - /// Creates the necessary header files for symbol renaming. /// /// For simplicity, both non-Nasm- and Nasm- style headers are always /// generated, even though local non-packaged builds need only one of them. -fn generate_prefix_symbols_headers(out_dir: &Path, prefix: &str) -> Result<(), std::io::Error> { +fn generate_prefix_symbols_headers( + out_dir: &Path, + core_name_and_version: &str, +) -> Result<(), std::io::Error> { + let prefix = &(String::from(core_name_and_version) + "_"); + generate_prefix_symbols_header(out_dir, "prefix_symbols.h", '#', None, prefix)?; generate_prefix_symbols_header( diff --git a/src/prefixed.rs b/src/prefixed.rs index 1491f9b88..14d69ed73 100644 --- a/src/prefixed.rs +++ b/src/prefixed.rs @@ -1,3 +1,27 @@ +// Keep in sync with `core_name_and_version` in build.rs. +macro_rules! core_name_and_version { + () => { + concat!( + env!("CARGO_PKG_NAME"), + "_core_", + env!("CARGO_PKG_VERSION_MAJOR"), + "_", + env!("CARGO_PKG_VERSION_MINOR"), + "_", + env!("CARGO_PKG_VERSION_PATCH"), + "_", + env!("CARGO_PKG_VERSION_PRE"), // Often empty + ) + }; +} + +// Keep in sync with `prefix` in build.rs. +macro_rules! prefix { + ( ) => { + concat!(core_name_and_version!(), "_") + }; +} + macro_rules! prefixed_extern { // Functions. { @@ -70,7 +94,7 @@ macro_rules! prefixed_item { $name:ident { $item:item } } => { - #[$attr = concat!(env!("RING_CORE_PREFIX"), stringify!($name))] + #[$attr = concat!(prefix!(), stringify!($name))] $item }; }