From 0bd27cafeff40b90bff8c055e698acd4d54a3bf1 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Tue, 12 Jul 2022 13:58:52 +0200 Subject: [PATCH 1/4] Rename rustflags variable to make global scope clear The currently available way of setting Rustflags is by setting the `RUSTFLAGS` environment variable, which affects all crates in the dependency graph. This may be undesirable in some circumstances, so a future commit will the option to specify RUSTFLAGS via `cargo rustc`, which only affects the current crate and not dependencies. This commit prepares by refactoring some names to make the distinction easier. --- cmake/Corrosion.cmake | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/cmake/Corrosion.cmake b/cmake/Corrosion.cmake index abbfd254..1af19949 100644 --- a/cmake/Corrosion.cmake +++ b/cmake/Corrosion.cmake @@ -252,10 +252,7 @@ function(_add_cargo_build) set(no_default_libraries_arg --no-default-libraries) endif() - set(rustflags_target_property "$>") - # `rustflags_target_property` may contain multiple arguments and double quotes, so we _should_ single quote it to - # preserve any double quotes and keep it as one argument value. However single quotes don't work on windows, so we - # can only add double quotes here. Any double quotes _in_ the rustflags must be escaped like `\\\"`. + set(global_rustflags_target_property "$>") set(features_target_property "$>") set(features_genex "$<$:--features=$>>") @@ -358,8 +355,8 @@ function(_add_cargo_build) corrosion_add_target_rustflags("${target_name}" "-Cdefault-linker-libraries=yes") endif() - set(joined_rustflags "$") - set(rustflags_genex "$<$:RUSTFLAGS=${joined_rustflags}>") + set(global_joined_rustflags "$") + set(global_rustflags_genex "$<$:RUSTFLAGS=${global_joined_rustflags}>") # Used to set a linker for a specific target-triple. set(cargo_target_linker_var "CARGO_TARGET_${_CORROSION_RUST_CARGO_TARGET_UPPER}_LINKER") @@ -397,7 +394,7 @@ function(_add_cargo_build) COMMAND ${CMAKE_COMMAND} -E env "${build_env_variable_genex}" - "${rustflags_genex}" + "${global_rustflags_genex}" "${cargo_target_linker}" "${corrosion_cc_rs_flags}" "${cargo_library_path}" From 54096b6768f5b1945e40e71564a8d1927d9cedec Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Tue, 12 Jul 2022 14:29:46 +0200 Subject: [PATCH 2/4] Add support for local Rustflags For some scenarios it is better to not set Rustflags for all crates in the dependency graph and instead only set it for the top-level crate. For example https://github.com/rust-lang/cargo/issues/8716 can be avoided in some scenarios by setting the rustflags via rustc, which allows for faster rebuilds in such cases. --- cmake/Corrosion.cmake | 27 ++++++++++++++++++- test/custom_profiles/rust/Cargo.lock | 7 +++++ test/rustflags/CMakeLists.txt | 3 +++ test/rustflags/rust/Cargo.lock | 7 +++++ test/rustflags/rust/Cargo.toml | 1 + .../rustflags/rust/some_dependency/Cargo.toml | 6 +++++ .../rustflags/rust/some_dependency/src/lib.rs | 10 +++++++ test/rustflags/rust/src/lib.rs | 7 +++++ 8 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 test/custom_profiles/rust/Cargo.lock create mode 100644 test/rustflags/rust/some_dependency/Cargo.toml create mode 100644 test/rustflags/rust/some_dependency/src/lib.rs diff --git a/cmake/Corrosion.cmake b/cmake/Corrosion.cmake index 1af19949..92cb4f1b 100644 --- a/cmake/Corrosion.cmake +++ b/cmake/Corrosion.cmake @@ -253,6 +253,7 @@ function(_add_cargo_build) endif() set(global_rustflags_target_property "$>") + set(local_rustflags_target_property "$>") set(features_target_property "$>") set(features_genex "$<$:--features=$>>") @@ -357,6 +358,9 @@ function(_add_cargo_build) set(global_joined_rustflags "$") set(global_rustflags_genex "$<$:RUSTFLAGS=${global_joined_rustflags}>") + set(local_rustflags_delimiter "$<$:-->") + set(local_rustflags_genex "$<$:${local_rustflags_target_property}>") + # Used to set a linker for a specific target-triple. set(cargo_target_linker_var "CARGO_TARGET_${_CORROSION_RUST_CARGO_TARGET_UPPER}_LINKER") @@ -401,7 +405,7 @@ function(_add_cargo_build) "CORROSION_BUILD_DIR=${CMAKE_CURRENT_BINARY_DIR}" "CARGO_BUILD_RUSTC=${_CORROSION_RUSTC}" "${_CORROSION_CARGO}" - build + rustc ${cargo_target_option} ${_CORROSION_VERBOSE_OUTPUT_FLAG} # Global --features arguments added via corrosion_import_crate() @@ -416,6 +420,9 @@ function(_add_cargo_build) ${cargo_profile} ${flag_args} ${flags_genex} + # Any arguments to cargo must be placed before this line + ${local_rustflags_delimiter} + ${local_rustflags_genex} # Copy crate artifacts to the binary dir COMMAND @@ -601,6 +608,12 @@ function(corrosion_set_hostbuild target_name) ) endfunction() +# Add flags for rustc (RUSTFLAGS) which affect the target and all of it's Rust dependencies +# +# Additional rustflags may be passed as optional parameters after rustflag. +# Please note, that if you import multiple targets from a package or workspace, but set different +# Rustflags via this function, the Rust dependencies will have to be rebuilt when changing targets. +# Consider `corrosion_add_target_local_rustflags()` as an alternative to avoid this. function(corrosion_add_target_rustflags target_name rustflag) # Additional rustflags may be passed as optional parameters after rustflag. set_property( @@ -610,6 +623,18 @@ function(corrosion_add_target_rustflags target_name rustflag) ) endfunction() +# Add flags for rustc (RUSTFLAGS) which only affect the target, but none of it's (Rust) dependencies +# +# Additional rustflags may be passed as optional parameters after rustc_flag. +function(corrosion_add_target_local_rustflags target_name rustc_flag) + # Set Rustflags via `cargo rustc` which only affect the current crate, but not dependencies. + set_property( + TARGET ${target_name} + APPEND + PROPERTY INTERFACE_CORROSION_LOCAL_RUSTFLAGS ${rustc_flag} ${ARGN} + ) +endfunction() + function(corrosion_set_env_vars target_name env_var) # Additional environment variables may be passed as optional parameters after env_var. set_property( diff --git a/test/custom_profiles/rust/Cargo.lock b/test/custom_profiles/rust/Cargo.lock new file mode 100644 index 00000000..6c1abd7f --- /dev/null +++ b/test/custom_profiles/rust/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "custom-profiles-lib" +version = "0.1.0" diff --git a/test/rustflags/CMakeLists.txt b/test/rustflags/CMakeLists.txt index c18f6d4b..016bf739 100644 --- a/test/rustflags/CMakeLists.txt +++ b/test/rustflags/CMakeLists.txt @@ -11,3 +11,6 @@ corrosion_add_target_rustflags(rustflag-test-lib --cfg=test_rustflag_cfg2="$,$>,debug,release>" "--cfg=test_rustflag_cfg3" ) + +corrosion_add_target_local_rustflags(rustflag-test-lib "--cfg=test_local_rustflag1") +corrosion_add_target_local_rustflags(rustflag-test-lib --cfg=test_local_rustflag2="value") diff --git a/test/rustflags/rust/Cargo.lock b/test/rustflags/rust/Cargo.lock index fcc72380..333ce249 100644 --- a/test/rustflags/rust/Cargo.lock +++ b/test/rustflags/rust/Cargo.lock @@ -5,3 +5,10 @@ version = 3 [[package]] name = "rustflag-test-lib" version = "0.1.0" +dependencies = [ + "some_dependency", +] + +[[package]] +name = "some_dependency" +version = "0.1.0" diff --git a/test/rustflags/rust/Cargo.toml b/test/rustflags/rust/Cargo.toml index e7f53152..2bdd26f7 100644 --- a/test/rustflags/rust/Cargo.toml +++ b/test/rustflags/rust/Cargo.toml @@ -5,6 +5,7 @@ license = "MIT" edition = "2018" [dependencies] +some_dependency = { path = "some_dependency" } [lib] crate-type=["staticlib"] diff --git a/test/rustflags/rust/some_dependency/Cargo.toml b/test/rustflags/rust/some_dependency/Cargo.toml new file mode 100644 index 00000000..94627d03 --- /dev/null +++ b/test/rustflags/rust/some_dependency/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "some_dependency" +version = "0.1.0" +license = "MIT" +edition = "2018" + diff --git a/test/rustflags/rust/some_dependency/src/lib.rs b/test/rustflags/rust/some_dependency/src/lib.rs new file mode 100644 index 00000000..d240a7ca --- /dev/null +++ b/test/rustflags/rust/some_dependency/src/lib.rs @@ -0,0 +1,10 @@ +//! Test that the local rustflags are only passed to the main crate and not to dependencies. +#[cfg(test_local_rustflag1)] +const _: [(); 1] = [(); 2]; + +#[cfg(test_local_rustflag2 = "value")] +const _: [(); 1] = [(); 2]; + +pub fn some_function() -> u32 { + 42 +} diff --git a/test/rustflags/rust/src/lib.rs b/test/rustflags/rust/src/lib.rs index 0d68a2fd..f6da1f6e 100644 --- a/test/rustflags/rust/src/lib.rs +++ b/test/rustflags/rust/src/lib.rs @@ -27,7 +27,14 @@ pub extern "C" fn rust_second_function(name: *const c_char) { pub extern "C" fn rust_third_function(name: *const c_char) { let name = unsafe { std::ffi::CStr::from_ptr(name).to_str().unwrap() }; println!("Hello, {}! I'm Rust again, third time the charm!", name); + assert_eq!(some_dependency::some_function(), 42); } #[cfg(not(test_rustflag_cfg3))] const _: [(); 1] = [(); 2]; + +#[cfg(not(test_local_rustflag1))] +const _: [(); 1] = [(); 2]; + +#[cfg(not(test_local_rustflag2 = "value"))] +const _: [(); 1] = [(); 2]; From b0b16b989a4359b7a3f578f142ff2d58c675081d Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Tue, 12 Jul 2022 14:57:41 +0200 Subject: [PATCH 3/4] Use local_rustflags where appropriate Using local rustflags passed via `cargo rustc` has the advantage, that they are not set for dependencies. This is mostly benenficial in workspaces, where different crates link different C dependencies, but share common rust dependencies. If the RUSTFLAGS environment variable is used this will cause rebuilds of the common rust dependencies, since the RUSTFLAGS differ. When passing rustflags via cargo rustc, this does not happen, since they only affects the main crate. --- cmake/Corrosion.cmake | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmake/Corrosion.cmake b/cmake/Corrosion.cmake index 92cb4f1b..d8b6d0f1 100644 --- a/cmake/Corrosion.cmake +++ b/cmake/Corrosion.cmake @@ -347,13 +347,13 @@ function(_add_cargo_build) list(APPEND corrosion_cc_rs_flags "SDKROOT=${CMAKE_OSX_SYSROOT}") endif() - corrosion_add_target_rustflags("${target_name}" "$<$:-Clink-args=${corrosion_link_args}>") + corrosion_add_target_local_rustflags("${target_name}" "$<$:-Clink-args=${corrosion_link_args}>") # todo: this should probably also be guarded by if_not_host_build_condition. if(COR_NO_STD) - corrosion_add_target_rustflags("${target_name}" "-Cdefault-linker-libraries=no") + corrosion_add_target_local_rustflags("${target_name}" "-Cdefault-linker-libraries=no") else() - corrosion_add_target_rustflags("${target_name}" "-Cdefault-linker-libraries=yes") + corrosion_add_target_local_rustflags("${target_name}" "-Cdefault-linker-libraries=yes") endif() set(global_joined_rustflags "$") @@ -379,7 +379,7 @@ function(_add_cargo_build) # Skip adding the linker argument, if the linker is explicitly set, since the # explicit_linker_property will not be set when this function runs. # Passing this rustflag is necessary for clang. - corrosion_add_target_rustflags("${target_name}" "$<$>:${rustflag_linker_arg}>") + corrosion_add_target_local_rustflags("${target_name}" "$<$>:${rustflag_linker_arg}>") endif() else() message(DEBUG "No linker preference for target ${target_name} could be detected.") @@ -704,8 +704,8 @@ function(corrosion_link_libraries target_name) $ ) - corrosion_add_target_rustflags(${target_name} "-L$") - corrosion_add_target_rustflags(${target_name} "-l$") + corrosion_add_target_local_rustflags(${target_name} "-L$") + corrosion_add_target_local_rustflags(${target_name} "-l$") endforeach() endfunction(corrosion_link_libraries) From d9dfdefaa3d9ec4ba1245c7070727359c65c7869 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Sat, 10 Sep 2022 16:18:41 +0200 Subject: [PATCH 4/4] Update release notes and readme Document recent changes --- README.md | 11 +++++++++++ RELEASES.md | 26 +++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 56eb2ffb..5ef00156 100644 --- a/README.md +++ b/README.md @@ -220,6 +220,11 @@ Some configuration options can be specified individually for each target. You ca the `RUSTFLAGS` environment variable will contain the flags added via this function. Please note that any dependencies (built by cargo) will also see these flags. In the future corrosion may offer a second function to allow specifying flags only for the target in question, utilizing `cargo rustc` instead of `cargo build`. +- `corrosion_add_target_local_rustflags(target_name rustc_flag [more_flags ...])`: Support setting + rustflags for only the main target (crate ) and none of it's dependencies. + This is useful in cases where you only need rustflags on the main-crate, but need to set different + flags for different targets. Without "local" Rustflags this would require rebuilds of the + dependencies when switching targets. - `corrosion_set_hostbuild()`: The target should be compiled for the Host target and ignore any cross-compile configuration. - `corrosion_set_features( [ALL_FEATURES ] [NO_DEFAULT_FEATURES] [FEATURES ... ])`: @@ -229,6 +234,12 @@ Some configuration options can be specified individually for each target. You ca - `corrosion_set_flags( ...])`: For a given target, add options and flags at the end of `cargo build` invocation. This will be appended after any arguments passed through the `FLAGS` during the crate import. +- `corrosion_set_linker(target_name linker)`: Use `linker` to link the target. + Please note that this only has an effect for targets where the final linker invocation is done + by cargo, i.e. targets where foreign code is linked into rust code and not the other way around. + Please also note that if you are cross-compiling and specify a linker such as `clang`, you are + responsible for also adding a rustflag which adds the necessary `--target=` argument for the + linker. ### Selecting a custom cargo profile diff --git a/RELEASES.md b/RELEASES.md index 94761e9c..3ae53d08 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,21 +1,41 @@ # Unreleased -# Breaking +## Breaking - The minimum supported rust version was increased to 1.46, due to a cargo issue that recently surfaced on CI when using crates.io. +- Increase the minimum required CMake version to 3.15 (may be bumped to 3.16 before the next release). -# Potentially breaking +## Potentially breaking - The working directory when invoking `cargo build` was changed to the directory of the Manifest file. This now allows cargo to pick up `.cargo/config.toml` files located in the source tree. + ([205](https://github.com/corrosion-rs/corrosion/pull/205)) - Corrosion internally invokes `cargo build`. When passing arguments to `cargo build`, Corrosion now uses the CMake `VERBATIM` option. In rare cases this may require you to change how you quote parameters passed to corrosion (e.g. via `corrosion_add_target_rustflags()`). For example setting a `cfg` option previously required double escaping the rustflag like this `"--cfg=something=\\\"value\\\""`, but now it can be passed to corrosion without any escapes: `--cfg=something="value"`. - + +## New features + +- Support setting rustflags for only the main target and none of it's dependencies ([215](https://github.com/corrosion-rs/corrosion/pull/215)). + A new function `corrosion_add_target_local_rustflags(target_name rustc_flag [more_flags ...])` + is added for this purpose. + This is useful in cases where you only need rustflags on the main-crate, but need to set different + flags for different targets. Without "local" Rustflags this would require rebuilds of the + dependencies when switching targets. +- Support explicitly selecting a linker ([208](https://github.com/corrosion-rs/corrosion/pull/208)). + The linker can be selected via `corrosion_set_linker(target_name linker)`. + Please note that this only has an effect for targets, where the final linker invocation is done + by cargo, i.e. targets where foreign code is linked into rust code and not the other way around. + +## Fixes + +- Fix a CMake developer Warning when a Multi-Config Generator and Rust executable targets + ([#213](https://github.com/corrosion-rs/corrosion/pull/213)). + # 0.2.2 (2022-09-01) ## Fixes