Skip to content

Commit 880d381

Browse files
authored
Test msys2 w/ prebuilt-nasm (#830)
* Test msys2 w/ prebuilt-nasm * Select appropriate prebuilt-nasm script * CI properly cover msys2 * Generate warning when NASM test fails
1 parent a2ed335 commit 880d381

File tree

4 files changed

+81
-25
lines changed

4 files changed

+81
-25
lines changed

.github/workflows/cross.yml

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -313,17 +313,25 @@ jobs:
313313
docker run -v "${{ github.workspace }}:/aws_lc_rs" alpine:3.20
314314
315315
msys2:
316+
name: msys2 ${{ matrix.sys }} - ${{ matrix.nasm }}, ${{ matrix.generator }}
316317
if: github.repository_owner == 'aws'
317318
runs-on: windows-latest
318319
env:
319320
AWS_LC_SYS_EXTERNAL_BINDGEN: 1
320321
strategy:
321322
fail-fast: false
322323
matrix:
323-
include:
324-
- { sys: mingw64 }
325-
- { sys: ucrt64 }
326-
- { sys: clang64 }
324+
sys:
325+
- mingw64
326+
- ucrt64
327+
- clang64
328+
nasm:
329+
- 'prebuilt'
330+
- 'nasm'
331+
generator:
332+
- ninja
333+
- msys
334+
- mingw
327335
steps:
328336
- name: Install MSYS2
329337
uses: msys2/setup-msys2@v2
@@ -332,28 +340,44 @@ jobs:
332340
msystem: ${{ matrix.sys }}
333341
update: true
334342
pacboy: >-
343+
cmake
335344
toolchain
345+
llvm
336346
rust
337347
rust-bindgen
338348
ninja
339349
cc
340-
nasm
341350
go
351+
${{ (matrix.nasm == 'nasm' && 'nasm') || 'make' }}
342352
- name: Update Environment
343353
shell: bash
344354
run: |
345355
SYSROOT=$(cygpath -a -m '${{ steps.setup_msys2.outputs.msys2-location }}\${{ matrix.sys }}')
356+
echo "AWS_LC_SYS_PREBUILT_NASM=${{ (matrix.nasm == 'nasm' && '0') || '1' }}" >> $GITHUB_ENV
346357
echo "BINDGEN_EXTRA_CLANG_ARGS=-target x86_64-pc-windows-gnu" >> $GITHUB_ENV
347358
echo "GOPATH=${SYSROOT}" >> $GITHUB_ENV
348359
echo "GOROOT=${SYSROOT}/lib/go" >> $GITHUB_ENV
360+
cygpath -w ${SYSROOT}/bin >> $GITHUB_PATH
361+
- name: Set CMAKE_GENERATOR
362+
if: matrix.generator == 'ninja'
363+
shell: bash
364+
run: |
349365
echo "CMAKE_GENERATOR=Ninja" >> $GITHUB_ENV
366+
- name: Set LIBCLANG_PATH
367+
if: matrix.generator == 'msys'
368+
shell: bash
369+
run: |
370+
SYSROOT=$(cygpath -a -m '${{ steps.setup_msys2.outputs.msys2-location }}\${{ matrix.sys }}')
350371
echo "LIBCLANG_PATH=${SYSROOT}/lib" >> $GITHUB_ENV
351-
cygpath -w ${SYSROOT}/bin >> $GITHUB_PATH
352372
- name: Checkout
353373
uses: actions/checkout@v4
354374
with:
355375
submodules: "recursive"
356-
- name: Build
357-
run: cargo build --verbose --features bindgen --release -p aws-lc-rs
358376
- name: Test
359-
run: cargo test --verbose --features bindgen -p aws-lc-rs
377+
if: matrix.generator != 'msys'
378+
shell: bash
379+
run: cargo test --verbose --features bindgen --release -p aws-lc-rs
380+
- name: Test
381+
if: matrix.generator == 'msys'
382+
shell: msys2 {0}
383+
run: cargo test --verbose --features bindgen --release -p aws-lc-rs

aws-lc-sys/builder/cmake_builder.rs

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::{
1111
};
1212
use std::env;
1313
use std::ffi::OsString;
14-
use std::path::PathBuf;
14+
use std::path::{Path, PathBuf};
1515

1616
pub(crate) struct CmakeBuilder {
1717
manifest_dir: PathBuf,
@@ -24,6 +24,11 @@ fn test_clang_cl_command() -> bool {
2424
execute_command("clang-cl".as_ref(), &["--version".as_ref()]).status
2525
}
2626

27+
fn test_prebuilt_nasm_script(script_path: &Path) -> bool {
28+
// Call with no args - both scripts will exit with error, but we only care if they can execute
29+
execute_command(script_path.as_os_str(), &[]).executed
30+
}
31+
2732
fn find_cmake_command() -> Option<OsString> {
2833
if let Some(cmake) = optional_env_optional_crate_target("CMAKE") {
2934
emit_warning(&format!(
@@ -216,6 +221,35 @@ impl CmakeBuilder {
216221
}
217222
}
218223

224+
#[allow(clippy::unused_self)]
225+
fn select_prebuilt_nasm_script(&self) -> PathBuf {
226+
let sh_script = self.manifest_dir.join("builder").join("prebuilt-nasm.sh");
227+
let bat_script = self.manifest_dir.join("builder").join("prebuilt-nasm.bat");
228+
229+
// Test .sh first (more universal - works in MSYS2, WSL, native Unix)
230+
if test_prebuilt_nasm_script(&sh_script) {
231+
emit_warning("Selected prebuilt-nasm.sh (shell script can execute)");
232+
sh_script
233+
} else if test_prebuilt_nasm_script(&bat_script) {
234+
emit_warning(
235+
"Selected prebuilt-nasm.bat (batch script can execute, shell script cannot)",
236+
);
237+
bat_script
238+
} else {
239+
// Fallback to current logic if neither can execute
240+
let fallback_script = if cfg!(target_os = "windows") {
241+
bat_script
242+
} else {
243+
sh_script
244+
};
245+
emit_warning(
246+
&format!(
247+
"Neither script could be tested for execution, falling back to target-based selection: {}",
248+
fallback_script.file_name().unwrap().to_str().unwrap()));
249+
fallback_script
250+
}
251+
}
252+
219253
#[allow(clippy::unused_self)]
220254
fn configure_android(&self, _cmake_cfg: &mut cmake::Config) {
221255
// If we leave CMAKE_SYSTEM_PROCESSOR unset, then cmake-rs should handle properly setting
@@ -274,18 +308,8 @@ impl CmakeBuilder {
274308
emit_warning("!!! Using pre-built NASM binaries !!!");
275309
emit_warning("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!");
276310

277-
let script_name = if cfg!(target_os = "windows") {
278-
"prebuilt-nasm.bat"
279-
} else {
280-
"prebuilt-nasm.sh"
281-
};
282-
283-
let script_path = self
284-
.manifest_dir
285-
.join("builder")
286-
.join(script_name)
287-
.display()
288-
.to_string();
311+
let script_path = self.select_prebuilt_nasm_script();
312+
let script_path = script_path.display().to_string();
289313
let script_path = script_path.replace('\\', "/");
290314

291315
cmake_cfg.define("CMAKE_ASM_NASM_COMPILER", script_path.as_str());
@@ -404,7 +428,9 @@ impl crate::Builder for CmakeBuilder {
404428
}
405429
}
406430
if let Some(cmake_cmd) = find_cmake_command() {
407-
env::set_var("CMAKE", cmake_cmd);
431+
unsafe {
432+
env::set_var("CMAKE", cmake_cmd);
433+
}
408434
} else {
409435
eprintln!("Missing dependency: cmake");
410436
missing_dependency = true;

aws-lc-sys/builder/main.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,11 @@ fn has_pregenerated() -> bool {
629629
}
630630

631631
fn test_nasm_command() -> bool {
632-
execute_command("nasm".as_ref(), &["-version".as_ref()]).status
632+
let status = execute_command("nasm".as_ref(), &["-version".as_ref()]).status;
633+
if !status {
634+
emit_warning("NASM command not found or failed to execute.");
635+
}
636+
status
633637
}
634638

635639
fn prepare_cargo_cfg() {

aws-lc-sys/builder/prebuilt-nasm.bat

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ goto loop
1313
shift
1414
set "path=%~1"
1515
for %%f in ("%path%") do set "filename=%%~nxf"
16-
copy "%ScriptDir%\prebuilt-nasm\%filename%" "%path%"
16+
REM Extract the base filename before the first dot, similar to the shell script
17+
for /f "tokens=1 delims=." %%a in ("%filename%") do set "basename=%%a"
18+
copy "%ScriptDir%\prebuilt-nasm\%basename%.obj" "%path%"
1719
exit 0
1820

1921
:failure

0 commit comments

Comments
 (0)