Skip to content

Commit 03a18db

Browse files
justsmthjustsmth
authored
Cleanup environment variable usage (#763)
* Fix MacOS w/ CMake 4.0 build * Cleanup CI environment variable usage * MacOS - test w/ latest CMake * Refactor environment variable handling * Use release builds for cross. Per Wine/x86_64-pc-windows-gnu
1 parent 4583ec5 commit 03a18db

File tree

9 files changed

+217
-121
lines changed

9 files changed

+217
-121
lines changed

‎.github/workflows/compilers.yml

+4-3
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ jobs:
150150
if: github.repository_owner == 'aws'
151151
name: C-std ${{ matrix.os }} - ${{ matrix.c_std }} - Force CMake ${{ matrix.cmake }}
152152
runs-on: ${{ matrix.os }}
153+
env:
154+
AWS_LC_SYS_CMAKE_BUILDER: ${{ matrix.cmake }}
153155
strategy:
154156
fail-fast: false
155157
matrix:
@@ -165,9 +167,8 @@ jobs:
165167
id: toolchain
166168
with:
167169
toolchain: stable
168-
- run: |
169-
echo 'export AWS_LC_SYS_CMAKE_BUILDER=${{ matrix.cmake }}' >> "$GITHUB_ENV"
170-
- name: Run cargo test
170+
- if: ${{ !startsWith(matrix.os, 'windows-') || matrix.cmake == '1' }} # Windows requires CMake build
171+
name: Run cargo test
171172
working-directory: ./aws-lc-rs
172173
env:
173174
AWS_LC_SYS_PREBUILT_NASM: 1

‎.github/workflows/cross.yml

+2-6
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ jobs:
4242
- [ powerpc64le-unknown-linux-gnu, 1, 0 ]
4343
- [ riscv64gc-unknown-linux-gnu, 0, 1 ]
4444
- [ s390x-unknown-linux-gnu, 0, 1 ]
45-
- [ x86_64-pc-windows-gnu, 0, 1 ]
45+
- [ x86_64-pc-windows-gnu, 0, 1 ] # Requires release build. See: https://github.com/rust-lang/rust/issues/139380
4646
- [ x86_64-unknown-linux-musl, 0, 1 ]
4747
- [ x86_64-unknown-illumos, 0, 0 ]
4848
steps:
@@ -92,7 +92,7 @@ jobs:
9292
if: ${{ matrix.target[0] != 'arm-linux-androideabi' }}
9393
run: |
9494
unset AWS_LC_SYS_EXTERNAL_BINDGEN
95-
cross test -p aws-lc-sys "${CROSS_TEST_EXTRA_FLAG}" --features ssl --target ${{ matrix.target[0] }}
95+
cross test -p aws-lc-sys --release "${CROSS_TEST_EXTRA_FLAG}" --features ssl --target ${{ matrix.target[0] }}
9696
9797
aws-lc-rs-cross-0_2_5-test:
9898
if: github.repository_owner == 'aws'
@@ -136,8 +136,6 @@ jobs:
136136
submodules: "recursive"
137137
- run: |
138138
brew install llvm
139-
echo 'export PATH="/opt/homebrew/opt/llvm/bin:$PATH"'
140-
echo 'export LIBCLANG_PATH=/opt/homebrew/opt/llvm' >> "$GITHUB_ENV"
141139
- uses: dtolnay/rust-toolchain@master
142140
id: toolchain
143141
with:
@@ -158,8 +156,6 @@ jobs:
158156
submodules: "recursive"
159157
- run: |
160158
brew install llvm
161-
echo 'export PATH="/opt/homebrew/opt/llvm/bin:$PATH"'
162-
echo 'export LIBCLANG_PATH=/opt/homebrew/opt/llvm' >> "$GITHUB_ENV"
163159
- uses: dtolnay/rust-toolchain@master
164160
id: toolchain
165161
with:

‎.github/workflows/fips.yml

+8
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ jobs:
5151
- uses: actions/setup-go@v4
5252
with:
5353
go-version: '>=1.18'
54+
- if: ${{ startsWith( matrix.os, 'macos-') }}
55+
name: Update Cmake
56+
# Use latest CMake
57+
run: |
58+
brew update
59+
brew upgrade cmake
60+
cmake --version
61+
echo 'CMAKE=${{ (matrix.os == 'macos-13' && '/usr/local') || '/opt/homebrew' }}/bin/cmake' >> $GITHUB_ENV
5462
- name: Run cargo test
5563
working-directory: ./aws-lc-rs
5664
run: cargo test ${{ matrix.args }}

‎.github/workflows/integration.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,8 @@ jobs:
213213
- if: ${{ startsWith(matrix.os, 'macos-') }}
214214
run: |
215215
brew install llvm
216-
echo 'export PATH="/opt/homebrew/opt/llvm/bin:$PATH"'
217-
echo 'export LIBCLANG_PATH=/opt/homebrew/opt/llvm' >> "$GITHUB_ENV"
216+
echo 'LIBCLANG_PATH=${{ (matrix.os == 'macos-13' && '/usr/local') || '/opt/homebrew' }}/opt/llvm/lib' >> $GITHUB_ENV
217+
echo 'LLVM_CONFIG_PATH=${{ (matrix.os == 'macos-13' && '/usr/local') || '/opt/homebrew' }}/opt/llvm/bin/llvm-config' >> $GITHUB_ENV
218218
- name: Install NASM on Windows
219219
if: runner.os == 'Windows'
220220
uses: ilammy/setup-nasm@v1

‎.github/workflows/tests.yml

+11-2
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,9 @@ jobs:
495495
if: github.repository_owner == 'aws'
496496
name: Clang 19.1 + bindgen tests
497497
runs-on: macos-14-xlarge
498+
env:
499+
LIBCLANG_PATH: '/opt/homebrew/opt/llvm@19/lib'
500+
LLVM_CONFIG_PATH: '/opt/homebrew/opt/llvm@19/bin/llvm-config'
498501
steps:
499502
- uses: actions/checkout@v4
500503
with:
@@ -510,8 +513,6 @@ jobs:
510513
brew update
511514
brew uninstall --force llvm
512515
brew install llvm@19
513-
echo 'LIBCLANG_PATH=/opt/homebrew/opt/llvm@19/lib' >> $GITHUB_ENV
514-
echo 'LLVM_CONFIG_PATH=/opt/homebrew/opt/llvm@19/bin/llvm-config' >> $GITHUB_ENV
515516
- name: aws-lc-sys bindgen build
516517
working-directory: ./aws-lc-sys
517518
run: cargo test --features bindgen
@@ -577,6 +578,14 @@ jobs:
577578
- uses: actions/setup-go@v4
578579
with:
579580
go-version: '>=1.18'
581+
- if: ${{ startsWith( matrix.os, 'macos-') }}
582+
name: Update Cmake
583+
# Use latest CMake
584+
run: |
585+
brew update
586+
brew upgrade cmake
587+
cmake --version
588+
echo 'CMAKE=${{ (matrix.os == 'macos-13' && '/usr/local') || '/opt/homebrew' }}/bin/cmake' >> $GITHUB_ENV
580589
- name: Run cargo test
581590
working-directory: "path has spaces/aws-lc-rs"
582591
run: cargo test --tests -p aws-lc-rs --features bindgen

‎aws-lc-fips-sys/builder/cmake_builder.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,16 @@ impl CmakeBuilder {
238238
}
239239
if target_os().trim() == "ios" {
240240
cmake_cfg.define("CMAKE_SYSTEM_NAME", "iOS");
241-
if effective_target().trim().ends_with("-ios-sim") {
241+
if effective_target().ends_with("-ios-sim") || target_arch() == "x86_64" {
242242
cmake_cfg.define("CMAKE_OSX_SYSROOT", "iphonesimulator");
243+
} else {
244+
cmake_cfg.define("CMAKE_OSX_SYSROOT", "iphoneos");
243245
}
246+
cmake_cfg.define("CMAKE_THREAD_LIBS_INIT", "-lpthread");
247+
}
248+
if target_os().trim() == "macos" {
249+
cmake_cfg.define("CMAKE_SYSTEM_NAME", "Darwin");
250+
cmake_cfg.define("CMAKE_OSX_SYSROOT", "macosx");
244251
}
245252
}
246253

‎aws-lc-sys/builder/cc_builder.rs

+8-11
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ mod x86_64_unknown_linux_musl;
1616

1717
use crate::{
1818
cargo_env, effective_target, emit_warning, env_var_to_bool, execute_command, get_crate_cflags,
19-
is_no_asm, option_env, out_dir, requested_c_std, target, target_arch, target_env, target_os,
20-
target_vendor, CStdRequested, OutputLibType,
19+
is_no_asm, optional_env_optional_crate_target, out_dir, requested_c_std, set_env_for_target,
20+
target, target_arch, target_env, target_os, target_vendor, CStdRequested, OutputLibType,
2121
};
2222
use std::path::PathBuf;
2323

@@ -28,7 +28,7 @@ pub(crate) struct CcBuilder {
2828
output_lib_type: OutputLibType,
2929
}
3030

31-
use std::{env, fs};
31+
use std::fs;
3232

3333
pub(crate) struct Library {
3434
name: &'static str,
@@ -198,11 +198,11 @@ impl CcBuilder {
198198
}
199199
}
200200

201-
if let Some(cc) = option_env("CC") {
202-
emit_warning(&format!("CC environment variable set: {}", cc.clone()));
201+
if let Some(cc) = optional_env_optional_crate_target("CC") {
202+
set_env_for_target("CC", &cc);
203203
}
204-
if let Some(cxx) = option_env("CXX") {
205-
emit_warning(&format!("CXX environment variable set: {}", cxx.clone()));
204+
if let Some(cxx) = optional_env_optional_crate_target("CXX") {
205+
set_env_for_target("CC", &cxx);
206206
}
207207

208208
if target_arch() == "x86" && !compiler_is_msvc {
@@ -328,10 +328,7 @@ impl CcBuilder {
328328
}
329329
let cflags = get_crate_cflags();
330330
if !cflags.is_empty() {
331-
emit_warning(&format!(
332-
"AWS_LC_SYS_CFLAGS found. Setting CFLAGS: '{cflags}'"
333-
));
334-
env::set_var("CFLAGS", cflags);
331+
set_env_for_target("CFLAGS", cflags);
335332
}
336333
cc_build
337334
}

‎aws-lc-sys/builder/cmake_builder.rs

+56-59
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ use crate::cc_builder::CcBuilder;
55
use crate::OutputLib::{Crypto, RustWrapper, Ssl};
66
use crate::{
77
allow_prebuilt_nasm, cargo_env, effective_target, emit_warning, execute_command,
8-
get_crate_cflags, is_crt_static, is_no_asm, is_no_pregenerated_src, option_env, target_arch,
9-
target_env, target_os, target_underscored, target_vendor, test_nasm_command, use_prebuilt_nasm,
10-
OutputLibType,
8+
get_crate_cflags, is_crt_static, is_no_asm, is_no_pregenerated_src, optional_env,
9+
optional_env_optional_crate_target, set_env, set_env_for_target, target_arch, target_env,
10+
target_os, test_nasm_command, use_prebuilt_nasm, OutputLibType,
1111
};
1212
use std::env;
1313
use std::ffi::OsString;
@@ -25,7 +25,7 @@ fn test_clang_cl_command() -> bool {
2525
}
2626

2727
fn find_cmake_command() -> Option<OsString> {
28-
if let Some(cmake) = option_env("CMAKE") {
28+
if let Some(cmake) = optional_env_optional_crate_target("CMAKE") {
2929
emit_warning(&format!(
3030
"CMAKE environment variable set: {}",
3131
cmake.clone()
@@ -89,6 +89,9 @@ impl CmakeBuilder {
8989
#[allow(clippy::too_many_lines)]
9090
fn prepare_cmake_build(&self) -> cmake::Config {
9191
let mut cmake_cfg = self.get_cmake_config();
92+
if let Some(generator) = optional_env_optional_crate_target("CMAKE_GENERATOR") {
93+
set_env("CMAKE_GENERATOR", generator);
94+
}
9295

9396
if OutputLibType::default() == OutputLibType::Dynamic {
9497
cmake_cfg.define("BUILD_SHARED_LIBS", "1");
@@ -132,24 +135,20 @@ impl CmakeBuilder {
132135
}
133136

134137
if cfg!(feature = "asan") {
135-
env::set_var("CC", "clang");
136-
env::set_var("CXX", "clang++");
137-
env::set_var("ASM", "clang");
138+
set_env_for_target("CC", "clang");
139+
set_env_for_target("CXX", "clang++");
138140

139141
cmake_cfg.define("ASAN", "1");
140142
}
141143

142-
if target_env() == "ohos" {
143-
Self::configure_open_harmony(&mut cmake_cfg, get_crate_cflags());
144-
return cmake_cfg;
145-
}
146-
147144
let cflags = get_crate_cflags();
148145
if !cflags.is_empty() {
149-
emit_warning(&format!(
150-
"AWS_LC_SYS_CFLAGS found. Setting CFLAGS: '{cflags}'"
151-
));
152-
env::set_var("CFLAGS", cflags);
146+
set_env_for_target("CFLAGS", cflags);
147+
}
148+
149+
if target_env() == "ohos" {
150+
Self::configure_open_harmony(&mut cmake_cfg);
151+
return cmake_cfg;
153152
}
154153

155154
// cmake-rs has logic that strips Optimization/Debug options that are passed via CFLAGS:
@@ -159,13 +158,8 @@ impl CmakeBuilder {
159158
Self::preserve_cflag_optimization_flags(&mut cmake_cfg);
160159

161160
// Allow environment to specify CMake toolchain.
162-
let toolchain_var_name = format!("CMAKE_TOOLCHAIN_FILE_{}", target_underscored());
163-
if let Some(toolchain) =
164-
option_env(&toolchain_var_name).or(option_env("CMAKE_TOOLCHAIN_FILE"))
165-
{
166-
emit_warning(&format!(
167-
"CMAKE_TOOLCHAIN_FILE environment variable set: {toolchain}"
168-
));
161+
if let Some(toolchain) = optional_env_optional_crate_target("CMAKE_TOOLCHAIN_FILE") {
162+
set_env_for_target("CMAKE_TOOLCHAIN_FILE", toolchain);
169163
return cmake_cfg;
170164
}
171165
// We only consider compiler CFLAGS when no cmake toolchain is set
@@ -187,22 +181,25 @@ impl CmakeBuilder {
187181
cmake_cfg.define("CMAKE_OSX_ARCHITECTURES", "x86_64");
188182
cmake_cfg.define("CMAKE_SYSTEM_PROCESSOR", "x86_64");
189183
}
184+
if target_os().trim() == "ios" {
185+
cmake_cfg.define("CMAKE_SYSTEM_NAME", "iOS");
186+
if effective_target().ends_with("-ios-sim") || target_arch() == "x86_64" {
187+
cmake_cfg.define("CMAKE_OSX_SYSROOT", "iphonesimulator");
188+
} else {
189+
cmake_cfg.define("CMAKE_OSX_SYSROOT", "iphoneos");
190+
}
191+
cmake_cfg.define("CMAKE_THREAD_LIBS_INIT", "-lpthread");
192+
}
193+
if target_os().trim() == "macos" {
194+
cmake_cfg.define("CMAKE_SYSTEM_NAME", "Darwin");
195+
cmake_cfg.define("CMAKE_OSX_SYSROOT", "macosx");
196+
}
190197
}
191198

192199
if target_os() == "android" {
193200
self.configure_android(&mut cmake_cfg);
194201
}
195202

196-
if target_vendor() == "apple" && target_os().to_lowercase() == "ios" {
197-
cmake_cfg.define("CMAKE_SYSTEM_NAME", "iOS");
198-
if effective_target().ends_with("-ios-sim") || target_arch() == "x86_64" {
199-
cmake_cfg.define("CMAKE_OSX_SYSROOT", "iphonesimulator");
200-
} else {
201-
cmake_cfg.define("CMAKE_OSX_SYSROOT", "iphoneos");
202-
}
203-
cmake_cfg.define("CMAKE_THREAD_LIBS_INIT", "-lpthread");
204-
}
205-
206203
cmake_cfg
207204
}
208205

@@ -225,18 +222,18 @@ impl CmakeBuilder {
225222
// https://github.com/rust-lang/cmake-rs/blob/b689783b5448966e810d515c798465f2e0ab56fd/src/lib.rs#L450-L499
226223

227224
// Log relevant environment variables.
228-
if let Some(value) = option_env("ANDROID_NDK_ROOT") {
229-
emit_warning(&format!("Found ANDROID_NDK_ROOT={value}"));
225+
if let Some(value) = optional_env_optional_crate_target("ANDROID_NDK_ROOT") {
226+
set_env("ANDROID_NDK_ROOT", value);
230227
} else {
231228
emit_warning("ANDROID_NDK_ROOT not set.");
232229
}
233-
if let Some(value) = option_env("ANDROID_NDK") {
234-
emit_warning(&format!("Found ANDROID_NDK={value}"));
230+
if let Some(value) = optional_env_optional_crate_target("ANDROID_NDK") {
231+
set_env("ANDROID_NDK", value);
235232
} else {
236233
emit_warning("ANDROID_NDK not set.");
237234
}
238-
if let Some(value) = option_env("ANDROID_STANDALONE_TOOLCHAIN") {
239-
emit_warning(&format!("Found ANDROID_STANDALONE_TOOLCHAIN={value}"));
235+
if let Some(value) = optional_env_optional_crate_target("ANDROID_STANDALONE_TOOLCHAIN") {
236+
set_env("ANDROID_STANDALONE_TOOLCHAIN", value);
240237
} else {
241238
emit_warning("ANDROID_STANDALONE_TOOLCHAIN not set.");
242239
}
@@ -245,16 +242,21 @@ impl CmakeBuilder {
245242
fn configure_windows(&self, cmake_cfg: &mut cmake::Config) {
246243
match (target_env().as_str(), target_arch().as_str()) {
247244
("msvc", "aarch64") => {
248-
cmake_cfg.generator_toolset(format!(
249-
"ClangCL{}",
250-
if cfg!(target_arch = "x86_64") {
251-
",host=x64"
252-
} else {
253-
""
254-
}
255-
));
245+
// If CMAKE_GENERATOR is either not set or not set to "Ninja"
246+
let cmake_generator = optional_env("CMAKE_GENERATOR");
247+
if cmake_generator.is_none() || cmake_generator.unwrap().to_lowercase() != "ninja" {
248+
// The following is not supported by the Ninja generator
249+
cmake_cfg.generator_toolset(format!(
250+
"ClangCL{}",
251+
if cfg!(target_arch = "x86_64") {
252+
",host=x64"
253+
} else {
254+
""
255+
}
256+
));
257+
cmake_cfg.define("CMAKE_GENERATOR_PLATFORM", "ARM64");
258+
}
256259
cmake_cfg.static_crt(is_crt_static());
257-
cmake_cfg.define("CMAKE_GENERATOR_PLATFORM", "ARM64");
258260
cmake_cfg.define("CMAKE_SYSTEM_NAME", "Windows");
259261
cmake_cfg.define("CMAKE_SYSTEM_PROCESSOR", "ARM64");
260262
}
@@ -319,25 +321,20 @@ impl CmakeBuilder {
319321
}
320322
}
321323

322-
fn configure_open_harmony(cmake_cfg: &mut cmake::Config, crate_cflags: &str) {
323-
env::set_var("CFLAGS", crate_cflags);
324+
fn configure_open_harmony(cmake_cfg: &mut cmake::Config) {
324325
let mut cflags = vec!["-Wno-unused-command-line-argument"];
325326
let mut asmflags = vec![];
326327

327-
let toolchain_var_name = format!("CMAKE_TOOLCHAIN_FILE_{}", target_underscored());
328328
// If a toolchain is not specified by the environment
329-
if option_env(&toolchain_var_name)
330-
.or(option_env("CMAKE_TOOLCHAIN_FILE"))
331-
.is_none()
332-
{
329+
if optional_env_optional_crate_target("CMAKE_TOOLCHAIN_FILE").is_none() {
333330
if let Ok(ndk) = env::var("OHOS_NDK_HOME") {
334-
env::set_var(
335-
toolchain_var_name,
331+
set_env_for_target(
332+
"CMAKE_TOOLCHAIN_FILE",
336333
format!("{ndk}/native/build/cmake/ohos.toolchain.cmake"),
337334
);
338335
} else if let Ok(sdk) = env::var("OHOS_SDK_NATIVE") {
339-
env::set_var(
340-
toolchain_var_name,
336+
set_env_for_target(
337+
"CMAKE_TOOLCHAIN_FILE",
341338
format!("{sdk}/build/cmake/ohos.toolchain.cmake"),
342339
);
343340
} else {

0 commit comments

Comments
 (0)