Fix pregeneration of bindings (#541)

* Fix pregeneration of bindings * Fix fips pregen test
aws · Sep 27, 2024 · 1a44d74 · 1a44d74
1 parent 21da240
commit 1a44d74
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 48 deletions.
diff --git a/.github/workflows/fips-bindings-generator.yml b/.github/workflows/fips-bindings-generator.yml
@@ -122,7 +122,7 @@ jobs:
         run: brew install bash coreutils
       - name: Generate bindings for ${{ matrix.os }}
         env:
-          AWS_LC_FIPS_SYS_INTERNAL_BINDGEN: "1"
+          AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS: "1"
         run: |
           cargo test -p aws-lc-fips-sys --features bindgen,ssl
       - name: Commit & Push changes
@@ -145,7 +145,7 @@ jobs:
         run: cargo install cross --locked --git https://github.com/cross-rs/cross
       - name: Generate bindings for ${{ matrix.target }}
         env:
-          AWS_LC_FIPS_SYS_INTERNAL_BINDGEN: "1"
+          AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS: "1"
         run: |
           cross test -p aws-lc-fips-sys --features bindgen,ssl --target ${{ matrix.target }}
       - name: Commit & Push changes

diff --git a/.github/workflows/pregen-bindings.yml b/.github/workflows/pregen-bindings.yml
@@ -19,9 +19,8 @@ env:
   RUST_NIGHTLY_TOOLCHAIN: nightly
   RUST_SCRIPT_NIGHTLY_TOOLCHAIN: nightly-2024-05-22
   GOPROXY: https://proxy.golang.org,direct
-  AWS_LC_SYS_CMAKE_BUILDER: 1
 jobs:
-  bindings:
+  sys-bindings:
     if: github.repository_owner == 'aws'
     runs-on: ubuntu-latest
     steps:
@@ -69,3 +68,51 @@ jobs:
         working-directory: ./aws-lc-rs
         run: |
           cargo test
+  fips-sys-bindings:
+    if: github.repository_owner == 'aws'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: true
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          # Our aws-lc-fips-sys generation scripts require nightly.
+          toolchain: ${{ env.RUST_SCRIPT_NIGHTLY_TOOLCHAIN }}
+      - run: rustup override set $RUST_NIGHTLY_TOOLCHAIN
+      - uses: actions-rs/cargo@v1
+        with:
+          command: install
+          args: rust-script
+      - name: Install OS Dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get -y --no-install-recommends install cmake gcc clang ninja-build golang
+      - name: Regenerate aws-lc-fips-sys crate
+        working-directory: ./aws-lc-fips-sys
+        run: |
+          rm -rf symbols/*
+          rm -rf generated-include/openssl/*
+          ../scripts/generate/_collect_symbols_build.sh -c aws-lc-fips-sys -f
+          ../scripts/generate/_generate_prefix_headers.sh -c aws-lc-fips-sys -f
+          ../scripts/generate/_generate_bindings.sh -c aws-lc-fips-sys
+      - name: aws-lc-fips-sys build
+        working-directory: ./aws-lc-fips-sys
+        run: |
+          cargo build
+      - name: aws-lc-fips-sys test
+        working-directory: ./aws-lc-fips-sys
+        run: |
+          cargo test
+      - name: aws-lc-fips-sys packaging
+        working-directory: ./aws-lc-fips-sys
+        run: |
+          cargo package --allow-dirty
+      - name: aws-lc-rs build
+        working-directory: ./aws-lc-rs
+        run: |
+          cargo build --no-default-features --features=fips
+      - name: aws-lc-rs test
+        working-directory: ./aws-lc-rs
+        run: |
+          cargo test --no-default-features --features=fips
diff --git a/.github/workflows/sys-bindings-generator.yml b/.github/workflows/sys-bindings-generator.yml
@@ -168,7 +168,7 @@ jobs:
         run: brew install bash coreutils
       - name: Generate bindings for ${{ matrix.os }}
         env:
-          AWS_LC_SYS_INTERNAL_BINDGEN: "1"
+          AWS_LC_SYS_PREGENERATING_BINDINGS: "1"
         run: |
           cargo test -p aws-lc-sys --features bindgen
       - name: Commit & Push changes
@@ -202,7 +202,7 @@ jobs:
           go-version: '>=1.20'
       - name: Generate bindings for ${{ matrix.target }}
         env:
-          AWS_LC_SYS_INTERNAL_BINDGEN: "1"
+          AWS_LC_SYS_PREGENERATING_BINDINGS: "1"
         run: |
           cargo ${{ (matrix.target == 'aarch64-pc-windows-msvc' && 'build') || 'test' }} -p aws-lc-sys --features bindgen --target ${{ matrix.target }}
       - name: Commit & Push changes
@@ -226,7 +226,7 @@ jobs:
         run: cargo install cross --locked --git https://github.com/cross-rs/cross
       - name: Generate bindings for ${{ matrix.target }}
         env:
-          AWS_LC_SYS_INTERNAL_BINDGEN: "1"
+          AWS_LC_SYS_PREGENERATING_BINDINGS: "1"
         run: |
           cross test -p aws-lc-sys --features bindgen --target ${{ matrix.target }}
       - name: Commit & Push changes

diff --git a/Cross.toml b/Cross.toml
@@ -23,7 +23,7 @@ pre-build = [
 [build.env]
 passthrough = [
     "AWS_LC_SYS_NO_PREFIX", "AWS_LC_FIPS_SYS_NO_PREFIX",
-    "AWS_LC_SYS_INTERNAL_BINDGEN", "AWS_LC_FIPS_SYS_INTERNAL_BINDGEN",
+    "AWS_LC_SYS_PREGENERATING_BINDINGS", "AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS",
     "AWS_LC_SYS_EXTERNAL_BINDGEN", "AWS_LC_FIPS_SYS_EXTERNAL_BINDGEN",
     "AWS_LC_SYS_STATIC", "AWS_LC_FIPS_SYS_STATIC",
     "AWS_LC_SYS_CMAKE_BUILDER", "AWS_LC_SYS_CC_SRC_COLLECTOR",

diff --git a/aws-lc-fips-sys/builder/main.rs b/aws-lc-fips-sys/builder/main.rs
@@ -274,20 +274,20 @@ trait Builder {
 
 static mut PREGENERATED: bool = false;
 static mut AWS_LC_FIPS_SYS_NO_PREFIX: bool = false;
-static mut AWS_LC_FIPS_SYS_INTERNAL_BINDGEN: bool = false;
+static mut AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS: bool = false;
 static mut AWS_LC_FIPS_SYS_EXTERNAL_BINDGEN: bool = false;
 static mut AWS_LC_FIPS_SYS_NO_ASM: bool = false;
 fn initialize() {
     unsafe {
         AWS_LC_FIPS_SYS_NO_PREFIX = env_var_to_bool("AWS_LC_FIPS_SYS_NO_PREFIX").unwrap_or(false);
-        AWS_LC_FIPS_SYS_INTERNAL_BINDGEN =
-            env_var_to_bool("AWS_LC_FIPS_SYS_INTERNAL_BINDGEN").unwrap_or(false);
+        AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS =
+            env_var_to_bool("AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS").unwrap_or(false);
         AWS_LC_FIPS_SYS_EXTERNAL_BINDGEN =
             env_var_to_bool("AWS_LC_FIPS_SYS_EXTERNAL_BINDGEN").unwrap_or(false);
         AWS_LC_FIPS_SYS_NO_ASM = env_var_to_bool("AWS_LC_FIPS_SYS_NO_ASM").unwrap_or(false);
     }
 
-    if !is_external_bindgen() && (is_internal_bindgen() || !has_bindgen_feature()) {
+    if !is_external_bindgen() && (is_pregenerating_bindings() || !has_bindgen_feature()) {
         let target = target();
         let supported_platform = match target.as_str() {
             "x86_64-unknown-linux-gnu"
@@ -310,7 +310,7 @@ fn initialize() {
 
 fn is_bindgen_required() -> bool {
     is_no_prefix()
-        || is_internal_bindgen()
+        || is_pregenerating_bindings()
         || is_external_bindgen()
         || has_bindgen_feature()
         || !has_pregenerated()
@@ -327,8 +327,8 @@ fn is_no_prefix() -> bool {
     unsafe { AWS_LC_FIPS_SYS_NO_PREFIX }
 }
 
-fn is_internal_bindgen() -> bool {
-    unsafe { AWS_LC_FIPS_SYS_INTERNAL_BINDGEN }
+fn is_pregenerating_bindings() -> bool {
+    unsafe { AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS }
 }
 
 fn is_external_bindgen() -> bool {
@@ -381,7 +381,7 @@ fn main() {
 
     #[allow(unused_assignments)]
     let mut bindings_available = false;
-    if is_internal_bindgen() {
+    if is_pregenerating_bindings() {
         #[cfg(feature = "bindgen")]
         {
             emit_warning(&format!("Generating src bindings. Platform: {}", target()));

diff --git a/aws-lc-sys/builder/cc_builder.rs b/aws-lc-sys/builder/cc_builder.rs
@@ -109,11 +109,6 @@ impl CcBuilder {
             }
         }
 
-        if let Some(prefix) = &self.build_prefix {
-            cc_build
-                .define("BORINGSSL_IMPLEMENTATION", "1")
-                .define("BORINGSSL_PREFIX", prefix.as_str());
-        }
         self.add_includes(&mut cc_build);
 
         cc_build
@@ -131,7 +126,7 @@ impl CcBuilder {
             CStdRequested::None => {
                 if target_env() == "msvc" && target_arch() == "aarch64" {
                     // clang-cl (not "clang") will be used.
-                } else if self.compiler_check(&mut cc_build, "c11", "") {
+                } else if self.compiler_check("c11", "") {
                     cc_build.std("c11");
                 } else {
                     cc_build.std("c99");
@@ -191,9 +186,15 @@ impl CcBuilder {
     }
 
     fn add_includes(&self, cc_build: &mut cc::Build) {
+        // The order of includes matters
+        if let Some(prefix) = &self.build_prefix {
+            cc_build
+                .define("BORINGSSL_IMPLEMENTATION", "1")
+                .define("BORINGSSL_PREFIX", prefix.as_str());
+            cc_build.include(self.manifest_dir.join("generated-include"));
+        }
         cc_build
             .include(self.manifest_dir.join("include"))
-            .include(self.manifest_dir.join("generated-include"))
             .include(self.manifest_dir.join("aws-lc").join("include"))
             .include(
                 self.manifest_dir
@@ -236,7 +237,7 @@ impl CcBuilder {
         for flag in lib.flags {
             cc_build.flag(flag);
         }
-        self.compiler_checks(&mut cc_build);
+        self.run_compiler_checks();
 
         if let Some(prefix) = &self.build_prefix {
             cc_build.compile(format!("{}_crypto", prefix.as_str()).as_str());
@@ -248,10 +249,11 @@ impl CcBuilder {
     // This performs basic checks of compiler capabilities and sets an appropriate flag on success.
     // This should be kept in alignment with the checks performed by AWS-LC's CMake build.
     // See: https://github.com/search?q=repo%3Aaws%2Faws-lc%20check_compiler&type=code
-    fn compiler_check(&self, cc_build: &mut cc::Build, basename: &str, flag: &str) -> bool {
+    fn compiler_check(&self, basename: &str, flag: &str) -> bool {
         let mut ret_val = false;
-        let output_path = format!("{basename}.o");
-        let result = cc::Build::default()
+        let output_dir = self.out_dir.join(format!("out-{basename}"));
+        let mut cc_build = self.create_builder();
+        let result = cc_build
             .file(
                 self.manifest_dir
                     .join("aws-lc")
@@ -261,15 +263,18 @@ impl CcBuilder {
             )
             .flag("-Wno-unused-parameter")
             .warnings_into_errors(true)
-            .try_compile(output_path.as_str());
+            .out_dir(&output_dir)
+            .try_compile_intermediates();
 
-        if let Ok(()) = result {
+        if result.is_ok() {
             if !flag.is_empty() {
                 cc_build.define(flag, "1");
             }
             ret_val = true;
         }
-        let _ = fs::remove_file(output_path);
+        if fs::remove_dir_all(&output_dir).is_err() {
+            emit_warning(&format!("Failed to remove {:?}", &output_dir));
+        }
         emit_warning(&format!(
             "Compilation of '{basename}.c' {} - {:?}.",
             if ret_val { "succeeded" } else { "failed" },
@@ -342,13 +347,9 @@ impl CcBuilder {
         }
         let _ = fs::remove_file(exec_path);
     }
-    fn compiler_checks(&self, cc_build: &mut cc::Build) {
-        self.compiler_check(cc_build, "stdalign_check", "AWS_LC_STDALIGN_AVAILABLE");
-        self.compiler_check(
-            cc_build,
-            "builtin_swap_check",
-            "AWS_LC_BUILTIN_SWAP_SUPPORTED",
-        );
+    fn run_compiler_checks(&self) {
+        self.compiler_check("stdalign_check", "AWS_LC_STDALIGN_AVAILABLE");
+        self.compiler_check("builtin_swap_check", "AWS_LC_BUILTIN_SWAP_SUPPORTED");
         self.memcmp_check();
     }
 }
@@ -378,4 +379,8 @@ impl crate::Builder for CcBuilder {
         self.build_library(&libcrypto);
         Ok(())
     }
+
+    fn name(&self) -> &str {
+        "CC"
+    }
 }
diff --git a/aws-lc-sys/builder/cmake_builder.rs b/aws-lc-sys/builder/cmake_builder.rs
@@ -389,4 +389,8 @@ impl crate::Builder for CmakeBuilder {
 
         Ok(())
     }
+
+    fn name(&self) -> &str {
+        "CMake"
+    }
 }
diff --git a/aws-lc-sys/builder/main.rs b/aws-lc-sys/builder/main.rs
@@ -320,6 +320,7 @@ fn get_builder(prefix: &Option<String>, manifest_dir: &Path, out_dir: &Path) ->
 trait Builder {
     fn check_dependencies(&self) -> Result<(), String>;
     fn build(&self) -> Result<(), String>;
+    fn name(&self) -> &str;
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -348,7 +349,7 @@ impl CStdRequested {
 
 static mut PREGENERATED: bool = false;
 static mut AWS_LC_SYS_NO_PREFIX: bool = false;
-static mut AWS_LC_SYS_INTERNAL_BINDGEN: bool = false;
+static mut AWS_LC_SYS_PREGENERATING_BINDINGS: bool = false;
 static mut AWS_LC_SYS_EXTERNAL_BINDGEN: bool = false;
 static mut AWS_LC_SYS_NO_ASM: bool = false;
 static mut AWS_LC_SYS_CFLAGS: String = String::new();
@@ -359,8 +360,8 @@ static mut AWS_LC_SYS_C_STD: CStdRequested = CStdRequested::None;
 fn initialize() {
     unsafe {
         AWS_LC_SYS_NO_PREFIX = env_var_to_bool("AWS_LC_SYS_NO_PREFIX").unwrap_or(false);
-        AWS_LC_SYS_INTERNAL_BINDGEN =
-            env_var_to_bool("AWS_LC_SYS_INTERNAL_BINDGEN").unwrap_or(false);
+        AWS_LC_SYS_PREGENERATING_BINDINGS =
+            env_var_to_bool("AWS_LC_SYS_PREGENERATING_BINDINGS").unwrap_or(false);
         AWS_LC_SYS_EXTERNAL_BINDGEN =
             env_var_to_bool("AWS_LC_SYS_EXTERNAL_BINDGEN").unwrap_or(false);
         AWS_LC_SYS_NO_ASM = env_var_to_bool("AWS_LC_SYS_NO_ASM").unwrap_or(false);
@@ -369,7 +370,7 @@ fn initialize() {
         AWS_LC_SYS_C_STD = CStdRequested::from_env();
     }
 
-    if !is_external_bindgen() && (is_internal_bindgen() || !has_bindgen_feature()) {
+    if !is_external_bindgen() && (is_pregenerating_bindings() || !has_bindgen_feature()) {
         let target = target();
         let supported_platform = match target.as_str() {
             "aarch64-apple-darwin"
@@ -396,7 +397,7 @@ fn initialize() {
 
 fn is_bindgen_required() -> bool {
     is_no_prefix()
-        || is_internal_bindgen()
+        || is_pregenerating_bindings()
         || is_external_bindgen()
         || has_bindgen_feature()
         || !has_pregenerated()
@@ -413,8 +414,8 @@ fn is_no_prefix() -> bool {
     unsafe { AWS_LC_SYS_NO_PREFIX }
 }
 
-fn is_internal_bindgen() -> bool {
-    unsafe { AWS_LC_SYS_INTERNAL_BINDGEN }
+fn is_pregenerating_bindings() -> bool {
+    unsafe { AWS_LC_SYS_PREGENERATING_BINDINGS }
 }
 
 fn is_external_bindgen() -> bool {
@@ -497,12 +498,14 @@ fn main() {
     };
 
     let builder = get_builder(&prefix, &manifest_dir, &out_dir());
+    emit_warning(&format!("Building with: {}", builder.name()));
+    emit_warning(&format!("Symbol Prefix: {:?}", &prefix));
 
     builder.check_dependencies().unwrap();
 
     #[allow(unused_assignments)]
     let mut bindings_available = false;
-    if is_internal_bindgen() {
+    if is_pregenerating_bindings() {
         #[cfg(feature = "bindgen")]
         {
             emit_warning(&format!("Generating src bindings. Platform: {}", target()));

diff --git a/scripts/generate/_generate_bindings.sh b/scripts/generate/_generate_bindings.sh
@@ -41,10 +41,10 @@ if [[ -z "${GOPROXY:+x}" ]]; then
 fi
 
 cargo clean --target-dir "${TEMP_TARGET_DIR}"
-# Sets AWS_LC_SYS_INTERNAL_BINDGEN=1 which will cause the generation bindings for a specific platform. This feature
+# Sets AWS_LC_SYS_PREGENERATING_BINDINGS=1 which will cause the generation bindings for a specific platform. This feature
 # is only intended for internal use thus is not a cargo feature. Requires bindgen to be enabled. The internal_bindgen
 # config is enabled so that the final crates doesn't expect to find the dynamically generated bindings.rs
-env AWS_LC_SYS_INTERNAL_BINDGEN=1 AWS_LC_FIPS_SYS_INTERNAL_BINDGEN=1 cargo build --target-dir "${TEMP_TARGET_DIR}" --features bindgen
+env AWS_LC_SYS_PREGENERATING_BINDINGS=1 AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS=1 cargo build --target-dir "${TEMP_TARGET_DIR}" --features bindgen
 cargo clean --target-dir "${TEMP_TARGET_DIR}"
 
 popd &>/dev/null # ${CRATE_DIR}

diff --git a/scripts/generate/_verify_crate_api_diff.sh b/scripts/generate/_verify_crate_api_diff.sh
@@ -55,7 +55,7 @@ if [[ -z "${GOPROXY:+x}" ]]; then
   export GOPROXY=direct
 fi
 
-env AWS_LC_SYS_INTERNAL_BINDGEN=1 AWS_LC_FIPS_SYS_INTERNAL_BINDGEN=1 cargo build --target-dir "${TEMP_TARGET_DIR}" --features bindgen
+env AWS_LC_SYS_PREGENERATING_BINDINGS=1 AWS_LC_FIPS_SYS_PREGENERATING_BINDINGS=1 cargo build --target-dir "${TEMP_TARGET_DIR}" --features bindgen
 if ! cargo +stable public-api --target-dir "${TEMP_TARGET_DIR}" diff --deny changed --deny removed "${PUBLISHED_CRATE_VERSION}"; then
   echo
   echo "Version changing from: ${PUBLISHED_CRATE_VERSION} to ${CRATE_VERSION}"