Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated generation of Windows bindings #486

Merged
merged 36 commits into from
Aug 22, 2024

Conversation

justsmth
Copy link
Contributor

@justsmth justsmth commented Aug 5, 2024

Description of changes:

  • Automate the generation of bindings for common Windows platforms:
    • aarch64-pc-windows-msvc
    • x86_64-pc-windows-msvc
    • x86_64-pc-windows-gnu
    • i686-pc-windows-msvc
  • This eliminates the need for most Windows build environments to have LLVM/libclang configured.
  • First and second commits are manual changes. All other commits are automated.

Testing:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@justsmth justsmth requested a review from a team as a code owner August 5, 2024 18:57
@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.58%. Comparing base (c358484) to head (bbb0240).
Report is 61 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #486      +/-   ##
==========================================
- Coverage   95.80%   92.58%   -3.22%     
==========================================
  Files          61       67       +6     
  Lines        8143     8910     +767     
  Branches        0     8910    +8910     
==========================================
+ Hits         7801     8249     +448     
- Misses        342      400      +58     
- Partials        0      261     +261     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@skmcgrail skmcgrail self-requested a review August 5, 2024 19:06
@justsmth justsmth marked this pull request as draft August 5, 2024 19:10
@justsmth justsmth force-pushed the generate/aws-lc-sys-win-bindgen branch 3 times, most recently from 8392b44 to 9400496 Compare August 6, 2024 11:32
@justsmth
Copy link
Contributor Author

justsmth commented Aug 6, 2024

Adding the Windows bindings and removing the ssl bindings results in a slightly smaller crate size:
Before

aws-lc-rs on  main [$] via 🦀 v1.80.0 took 29s
at 08:45:55 ❯ du -h target/package/aws-lc-sys-0.21.0.crate
8.1M	target/package/aws-lc-sys-0.21.0.crate

After

aws-lc-rs on  generate/aws-lc-sys-win-bindgen [$] via 🦀 v1.80.0
at 08:44:31 ❯ du -h target/package/aws-lc-sys-0.21.0.crate
7.9M	target/package/aws-lc-sys-0.21.0.crate

@justsmth justsmth marked this pull request as ready for review August 6, 2024 22:14
@justsmth justsmth force-pushed the generate/aws-lc-sys-win-bindgen branch from 3149e8d to c3c27eb Compare August 13, 2024 12:47
@justsmth justsmth force-pushed the generate/aws-lc-sys-win-bindgen branch from c3c27eb to 3ee624e Compare August 21, 2024 14:46
justsmth and others added 17 commits August 21, 2024 11:33
diff --git a/.github/workflows/fips-bindings-generator.yml b/.github/workflows/fips-bindings-generator.yml
index f5cfd138b5..88d9fb9330 100644
--- a/.github/workflows/fips-bindings-generator.yml
+++ b/.github/workflows/fips-bindings-generator.yml
@@ -16,6 +16,10 @@ env:
   RUST_NIGHTLY_TOOLCHAIN: nightly
   RUST_SCRIPT_NIGHTLY_TOOLCHAIN: nightly-2024-05-22

+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref_name }}
+  cancel-in-progress: true
+
 jobs:
   collect-symbols-and-commit:
     if: github.repository == 'aws/aws-lc-rs'
diff --git a/.github/workflows/sys-bindings-generator.yml b/.github/workflows/sys-bindings-generator.yml
index 7c62c0581a..3fba893d80 100644
--- a/.github/workflows/sys-bindings-generator.yml
+++ b/.github/workflows/sys-bindings-generator.yml
@@ -16,6 +16,10 @@ env:
   RUST_NIGHTLY_TOOLCHAIN: nightly
   RUST_SCRIPT_NIGHTLY_TOOLCHAIN: nightly-2024-05-22

+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref_name }}
+  cancel-in-progress: true
+
 jobs:
   collect-symbols-and-commit:
     if: github.repository == 'aws/aws-lc-rs'
@@ -32,7 +36,7 @@ jobs:
       - uses: dtolnay/rust-toolchain@stable
       - uses: actions/setup-go@v4
         with:
-          go-version: '>=1.18'
+          go-version: '>=1.20'
       - name: Install current Bash on macOS
         if: runner.os == 'macOS'
         run: brew install bash coreutils
@@ -40,12 +44,50 @@ jobs:
         env:
           AWS_LC_SYS_NO_PREFIX: "1"
         run: |
-          cargo test -p aws-lc-sys --features bindgen,ssl
+          cargo test -p aws-lc-sys --features bindgen
       - name: Collect symbols
         run: |
           ./scripts/build/collect_symbols.sh -c aws-lc-sys
       - name: Commit & Push changes
         run: ./scripts/ci/ci_add_commit_rebase_push.sh "Symbols from ${{ matrix.os }}"
+  collect-windows-symbols-and-commit:
+    if: github.repository == 'aws/aws-lc-rs'
+    runs-on: windows-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        target:
+          - 'aarch64-pc-windows-msvc'
+          - 'x86_64-pc-windows-msvc'
+          - 'x86_64-pc-windows-gnu'
+          - 'i686-pc-windows-msvc'
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: 'recursive'
+          ref: ${{ github.ref_name }}
+      - uses: dtolnay/rust-toolchain@master
+        id: toolchain
+        with:
+          toolchain: stable
+          target: ${{ matrix.target }}
+      - if: contains(matrix.target, 'x86') || contains(matrix.target, 'i686')
+        uses: ilammy/setup-nasm@v1
+      - uses: actions/setup-go@v4
+        with:
+          go-version: '>=1.20'
+      - name: No-prefix build for ${{ matrix.target }}
+        env:
+          AWS_LC_SYS_NO_PREFIX: "1"
+        run: |
+          cargo ${{ (matrix.target == 'aarch64-pc-windows-msvc' && 'build') || 'test' }} -p aws-lc-sys --features bindgen --target  ${{ matrix.target }}
+      - name: Collect symbols
+        shell: bash
+        run: |
+          ./scripts/build/collect_symbols.sh -c aws-lc-sys -t  ${{ matrix.target }}
+      - name: Commit & Push changes
+        shell: bash
+        run: ./scripts/ci/ci_add_commit_rebase_push.sh "Symbols from  ${{ matrix.target }}"
   collect-cross-symbols-and-commit:
     if: github.repository == 'aws/aws-lc-rs'
     runs-on: ubuntu-latest
@@ -70,21 +112,21 @@ jobs:
       - uses: dtolnay/rust-toolchain@stable
       - uses: actions/setup-go@v4
         with:
-          go-version: '>=1.18'
+          go-version: '>=1.20'
       - name: Install cross
         run: cargo install cross --locked --git https://github.com/cross-rs/cross
       - name: No-prefix build for ${{ matrix.target }}
         env:
           AWS_LC_SYS_NO_PREFIX: "1"
         run: |
-          cross test -p aws-lc-sys --features bindgen,ssl --target ${{ matrix.target }}
+          cross test -p aws-lc-sys --features bindgen --target ${{ matrix.target }}
       - name: Collect symbols
         run: |
           ./scripts/build/collect_symbols.sh -c aws-lc-sys -t ${{ matrix.target }}
       - name: Commit & Push changes
         run: ./scripts/ci/ci_add_commit_rebase_push.sh "Symbols for ${{ matrix.target }}"
   generate-headers-and-commit:
-    needs: [ collect-cross-symbols-and-commit, collect-symbols-and-commit ]
+    needs: [ collect-cross-symbols-and-commit, collect-symbols-and-commit, collect-windows-symbols-and-commit ]
     if: github.repository == 'aws/aws-lc-rs'
     runs-on: ubuntu-latest
     steps:
@@ -98,7 +140,7 @@ jobs:
       - uses: dtolnay/rust-toolchain@stable
       - uses: actions/setup-go@v4
         with:
-          go-version: '>=1.18'
+          go-version: '>=1.20'
       - name: Generate Prefix Headers
         run: ./scripts/generate/_generate_prefix_headers.sh -c aws-lc-sys
       - name: Update sys-crate metadata
@@ -126,9 +168,44 @@ jobs:
         env:
           AWS_LC_SYS_INTERNAL_BINDGEN: "1"
         run: |
-          cargo test -p aws-lc-sys --features bindgen,ssl
+          cargo test -p aws-lc-sys --features bindgen
       - name: Commit & Push changes
         run: ./scripts/ci/ci_add_commit_rebase_push.sh "Generated bindings from ${{ matrix.os }}"
+  generate-windows-bindings-and-commit:
+    needs: generate-headers-and-commit
+    if: github.repository == 'aws/aws-lc-rs'
+    runs-on: windows-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        target:
+          - 'aarch64-pc-windows-msvc'
+          - 'x86_64-pc-windows-msvc'
+          - 'x86_64-pc-windows-gnu'
+          - 'i686-pc-windows-msvc'
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: 'recursive'
+          ref: ${{ github.ref_name }}
+      - uses: dtolnay/rust-toolchain@master
+        id: toolchain
+        with:
+          toolchain: stable
+          target: ${{ matrix.target }}
+      - if: contains(matrix.target, 'x86') || contains(matrix.target, 'i686')
+        uses: ilammy/setup-nasm@v1
+      - uses: actions/setup-go@v4
+        with:
+          go-version: '>=1.20'
+      - name: Generate bindings for ${{ matrix.target }}
+        env:
+          AWS_LC_SYS_INTERNAL_BINDGEN: "1"
+        run: |
+          cargo ${{ (matrix.target == 'aarch64-pc-windows-msvc' && 'build') || 'test' }} -p aws-lc-sys --features bindgen --target ${{ matrix.target }}
+      - name: Commit & Push changes
+        shell: bash
+        run: ./scripts/ci/ci_add_commit_rebase_push.sh "Generated bindings for ${{ matrix.target }}"
   generate-cross-bindings-and-commit:
     needs: generate-headers-and-commit
     if: github.repository == 'aws/aws-lc-rs'
@@ -149,7 +226,7 @@ jobs:
         env:
           AWS_LC_SYS_INTERNAL_BINDGEN: "1"
         run: |
-          cross test -p aws-lc-sys --features bindgen,ssl --target ${{ matrix.target }}
+          cross test -p aws-lc-sys --features bindgen --target ${{ matrix.target }}
       - name: Commit & Push changes
         run: ./scripts/ci/ci_add_commit_rebase_push.sh "Generated bindings for ${{ matrix.target }}"
   collect-src-and-commit:
diff --git a/aws-lc-sys/Cargo.toml b/aws-lc-sys/Cargo.toml
index c96d13a2ab..02b8183620 100644
--- a/aws-lc-sys/Cargo.toml
+++ b/aws-lc-sys/Cargo.toml
@@ -47,7 +47,7 @@ build = "builder/main.rs"

 [features]
 asan = []
-ssl = []
+ssl = ['bindgen']
 bindgen = ["dep:bindgen"] # Generate the bindings on the targetted platform as a fallback mechanism.

 [build-dependencies]
@@ -56,10 +56,10 @@ dunce = "1.0"
 fs_extra = "1.3"
 cc = { version = "1.0.83", features = ["parallel"] }

-[target.'cfg(any(all(any(target_arch = "x86_64", target_arch = "aarch64"), any(target_os = "linux", target_os = "macos"), any(target_env = "gnu", target_env = "musl", target_env = "")), all(target_arch = "x86", target_os = "linux", target_env = "gnu")))'.build-dependencies]
+[target.'cfg(any(all(any(target_arch="x86_64",target_arch="aarch64"),any(target_os="linux",target_os="macos",target_os="windows"),any(target_env="gnu",target_env="musl",target_env="msvc",target_env="")),all(target_arch="x86",target_os="windows",target_env="msvc"),all(target_arch="x86",target_os="linux",target_env="gnu")))'.build-dependencies]
 bindgen = { version = "0.69.2", optional = true }

-[target.'cfg(not(any(all(any(target_arch = "x86_64", target_arch = "aarch64"), any(target_os = "linux", target_os = "macos"), any(target_env = "gnu", target_env = "musl", target_env = "")), all(target_arch = "x86", target_os = "linux", target_env = "gnu"))))'.build-dependencies]
+[target.'cfg(not(any(all(any(target_arch="x86_64",target_arch="aarch64"),any(target_os="linux",target_os="macos",target_os="windows"),any(target_env="gnu",target_env="musl",target_env="msvc",target_env="")),all(target_arch="x86",target_os="windows",target_env="msvc"),all(target_arch="x86",target_os="linux",target_env="gnu"))))'.build-dependencies]
 bindgen = { version = "0.69.2" }

 [dependencies]
diff --git a/aws-lc-sys/builder/main.rs b/aws-lc-sys/builder/main.rs
index 6f93b1c5e7..b4c5540736 100644
--- a/aws-lc-sys/builder/main.rs
+++ b/aws-lc-sys/builder/main.rs
@@ -16,9 +16,15 @@ use cmake_builder::CmakeBuilder;
     not(any(
         all(
             any(target_arch = "x86_64", target_arch = "aarch64"),
-            any(target_os = "linux", target_os = "macos"),
-            any(target_env = "gnu", target_env = "musl", target_env = "")
+            any(target_os = "linux", target_os = "macos", target_os = "windows"),
+            any(
+                target_env = "gnu",
+                target_env = "musl",
+                target_env = "msvc",
+                target_env = ""
+            )
         ),
+        all(target_arch = "x86", target_os = "windows", target_env = "msvc"),
         all(target_arch = "x86", target_os = "linux", target_env = "gnu")
     ))
 ))]
@@ -174,9 +180,15 @@ fn execute_command(executable: &OsStr, args: &[&OsStr]) -> TestCommandResult {
     not(any(
         all(
             any(target_arch = "x86_64", target_arch = "aarch64"),
-            any(target_os = "linux", target_os = "macos"),
-            any(target_env = "gnu", target_env = "musl", target_env = "")
+            any(target_os = "linux", target_os = "macos", target_os = "windows"),
+            any(
+                target_env = "gnu",
+                target_env = "musl",
+                target_env = "msvc",
+                target_env = ""
+            )
         ),
+        all(target_arch = "x86", target_os = "windows", target_env = "msvc"),
         all(target_arch = "x86", target_os = "linux", target_env = "gnu")
     ))
 ))]
@@ -204,19 +216,8 @@ fn generate_src_bindings(manifest_dir: &Path, prefix: &Option<String>, src_bindi
             ..Default::default()
         },
     )
-        .write_to_file(src_bindings_path.join(format!("{}.rs", target_platform_prefix("crypto"))))
-        .expect("write bindings");
-
-    bindgen::generate_bindings(
-        manifest_dir,
-        &BindingOptions {
-            build_prefix: prefix.clone(),
-            include_ssl: true,
-            ..Default::default()
-        },
-    )
-        .write_to_file(src_bindings_path.join(format!("{}.rs", target_platform_prefix("crypto_ssl"))))
-        .expect("write bindings");
+    .write_to_file(src_bindings_path.join(format!("{}.rs", target_platform_prefix("crypto"))))
+    .expect("write bindings");
 }

 fn emit_rustc_cfg(cfg: &str) {
@@ -332,13 +333,17 @@ fn initialize() {
     if !is_external_bindgen() && (is_internal_bindgen() || !has_bindgen_feature()) {
         let target = target();
         let supported_platform = match target.as_str() {
-            "i686-unknown-linux-gnu"
-            | "x86_64-unknown-linux-gnu"
+            "aarch64-apple-darwin"
+            | "aarch64-pc-windows-msvc"
             | "aarch64-unknown-linux-gnu"
-            | "x86_64-unknown-linux-musl"
             | "aarch64-unknown-linux-musl"
+            | "i686-pc-windows-msvc"
+            | "i686-unknown-linux-gnu"
             | "x86_64-apple-darwin"
-            | "aarch64-apple-darwin" => Some(target),
+            | "x86_64-pc-windows-gnu"
+            | "x86_64-pc-windows-msvc"
+            | "x86_64-unknown-linux-gnu"
+            | "x86_64-unknown-linux-musl" => Some(target),
             _ => None,
         };
         if let Some(platform) = supported_platform {
@@ -394,13 +399,17 @@ fn prepare_cargo_cfg() {
     // Also remove `#![allow(unexpected_cfgs)]` from src/lib.rs
     /*
     println!("cargo::rustc-check-cfg=cfg(use_bindgen_generated)");
-    println!("cargo::rustc-check-cfg=cfg(i686_unknown_linux_gnu)");
-    println!("cargo::rustc-check-cfg=cfg(x86_64_unknown_linux_gnu)");
-    println!("cargo::rustc-check-cfg=cfg(aarch64_unknown_linux_gnu)");
-    println!("cargo::rustc-check-cfg=cfg(x86_64_unknown_linux_musl)");
-    println!("cargo::rustc-check-cfg=cfg(aarch64_unknown_linux_musl)");
-    println!("cargo::rustc-check-cfg=cfg(x86_64_apple_darwin)");
     println!("cargo::rustc-check-cfg=cfg(aarch64_apple_darwin)");
+    println!("cargo::rustc-check-cfg=cfg(aarch64_pc_windows_msvc)");
+    println!("cargo::rustc-check-cfg=cfg(aarch64_unknown_linux_gnu)");
+    println!("cargo::rustc-check-cfg=cfg(aarch64_unknown_linux_musl)");
+    println!("cargo::rustc-check-cfg=cfg(i686_pc_windows_msvc)");
+    println!("cargo::rustc-check-cfg=cfg(i686_unknown_linux_gnu)");
+    println!("cargo::rustc-check-cfg=cfg(x86_64_apple_darwin)");
+    println!("cargo::rustc-check-cfg=cfg(x86_64_pc-windows-gnu)");
+    println!("cargo::rustc-check-cfg=cfg(x86_64_pc_windows_msvc)");
+    println!("cargo::rustc-check-cfg=cfg(x86_64_unknown_linux_gnu)");
+    println!("cargo::rustc-check-cfg=cfg(x86_64_unknown_linux_musl)");
      */
 }

@@ -442,9 +451,15 @@ fn main() {
             not(any(
                 all(
                     any(target_arch = "x86_64", target_arch = "aarch64"),
-                    any(target_os = "linux", target_os = "macos"),
-                    any(target_env = "gnu", target_env = "musl", target_env = "")
+                    any(target_os = "linux", target_os = "macos", target_os = "windows"),
+                    any(
+                        target_env = "gnu",
+                        target_env = "musl",
+                        target_env = "msvc",
+                        target_env = ""
+                    )
                 ),
+                all(target_arch = "x86", target_os = "windows", target_env = "msvc"),
                 all(target_arch = "x86", target_os = "linux", target_env = "gnu")
             ))
         ))]
diff --git a/aws-lc-sys/src/lib.rs b/aws-lc-sys/src/lib.rs
index 032fa6559b..08959fa1b6 100644
--- a/aws-lc-sys/src/lib.rs
+++ b/aws-lc-sys/src/lib.rs
@@ -19,26 +19,21 @@ macro_rules! platform_binding {
         paste! {
             #[cfg(all($platform, not(feature = "ssl"), not(use_bindgen_generated)))]
             use_bindings!([< $platform _crypto >]);
-
-            #[cfg(all($platform, feature = "ssl", not(use_bindgen_generated)))]
-            use_bindings!([< $platform _crypto_ssl >]);
         }
     };
 }

-platform_binding!(i686_unknown_linux_gnu);
-
-platform_binding!(x86_64_unknown_linux_gnu);
-
-platform_binding!(aarch64_unknown_linux_gnu);
-
-platform_binding!(x86_64_unknown_linux_musl);
-
-platform_binding!(aarch64_unknown_linux_musl);
-
-platform_binding!(x86_64_apple_darwin);
-
 platform_binding!(aarch64_apple_darwin);
+platform_binding!(aarch64_pc_windows_msvc);
+platform_binding!(aarch64_unknown_linux_gnu);
+platform_binding!(aarch64_unknown_linux_musl);
+platform_binding!(i686_pc_windows_msvc);
+platform_binding!(i686_unknown_linux_gnu);
+platform_binding!(x86_64_apple_darwin);
+platform_binding!(x86_64_pc_windows_gnu);
+platform_binding!(x86_64_pc_windows_msvc);
+platform_binding!(x86_64_unknown_linux_gnu);
+platform_binding!(x86_64_unknown_linux_musl);

 #[cfg(use_bindgen_generated)]
 #[allow(
diff --git a/scripts/build/collect_symbols.sh b/scripts/build/collect_symbols.sh
index 43defea174..6fcb7a10b0 100755
--- a/scripts/build/collect_symbols.sh
+++ b/scripts/build/collect_symbols.sh
@@ -57,23 +57,23 @@ if [[ ! -d "${AWS_LC_DIR}" ]]; then
 fi

 function filter_symbols() {
-  grep -v -E "^bignum_" | grep -v "curve25519_x25519" | grep -v "edwards25519_"
+  grep -E '^\w*$' | grep -v -E "^bignum_" | grep -v "curve25519_x25519" | grep -v "edwards25519_"
 }

 function filter_nm_symbols() {
-  grep -v -E '^_Z' | grep -v 'BORINGSSL_bcm_' | grep -v 'BORINGSSL_integrity_test'
+  grep -v -E '^_Z' | grep -v -E '^\?' | grep -v 'BORINGSSL_bcm_' | grep -v 'BORINGSSL_integrity_test'
 }

-function filter_macho_symbols() {
+function filter_windows_symbols() {
+   grep -v -E '^_*v?f?s?n?printf' | grep -v -E '^_*v?s?f?scanf' | grep -v RtlSecureZeroMemory | grep -v gai_strerrorA
+}
+
+function remove_leading_underscore() {
   grep -E '^_' | sed -e 's/^_\(.*\)/\1/'
 }

 function find_libcrypto() {
-  find "${REPO_ROOT}/target" -type f \( -name "lib*crypto.a" -o -name "lib*crypto.so" -o -name "lib*crypto.dylib" \) | grep "${CRATE_NAME}"
-}
-
-function find_libssl() {
-  find "${REPO_ROOT}/target" -type f \( -name "lib*ssl.a" -o -name "lib*ssl.so" -o -name "lib*ssl.dylib" \) | grep "${CRATE_NAME}"
+  find "${REPO_ROOT}/target" -type f \( -name "*crypto.lib" -o -name "lib*crypto.a" -o -name "lib*crypto.so" -o -name "lib*crypto.dylib" \) | grep "${CRATE_NAME}"
 }

 LIBCRYPTO_PATH="$(find_libcrypto)"
@@ -82,22 +82,37 @@ if [[ "${?}" -ne 0 ]]; then
   exit 1
 fi

-LIBSSL_PATH="$(find_libssl)"
-if [[ "${?}" -ne 0 ]]; then
-  echo "Unable to find libssl"
-  exit 1
-fi
-
 mkdir -p "$(dirname "${SYMBOLS_FILE}")"
 echo Writing symbols to: ${SYMBOLS_FILE}

-if [[ "${LIBCRYPTO_PATH}" = *.dylib ]]; then
-  nm --extern-only --defined-only -j  "${LIBCRYPTO_PATH}" "${LIBSSL_PATH}" | grep -v "${REPO_ROOT}" | sort | uniq | filter_macho_symbols | filter_nm_symbols |  filter_symbols >"${SYMBOLS_FILE}"
-elif [[ "${LIBCRYPTO_PATH}" = *.so ]]; then
-  nm --extern-only --defined-only --format=just-symbols  "${LIBCRYPTO_PATH}" "${LIBSSL_PATH}" | grep -v "${REPO_ROOT}" | sort | uniq | filter_nm_symbols | filter_symbols >"${SYMBOLS_FILE}"
+if [[ "${PLATFORM}" = *-msvc ]]; then
+  if [[ "${PLATFORM}" = aarch64-* ]]; then
+    MSVC_ARCH=arm64
+  elif [[ "${PLATFORM}" = i686-* ]]; then
+    MSVC_ARCH=x86
+  else
+    MSVC_ARCH=x64
+  fi
+  PFx86=$(printenv "ProgramFiles(x86)")
+  VS_INSTALL_PATH="$("$(echo "${PFx86//\\/\/}//Microsoft Visual Studio/Installer/vswhere.exe")" | grep 'resolvedInstallationPath:' | sed -e 's/[^:]*: \(.*\)$/\1/')"
+
+  DUMPBIN="$(ls -1 "${VS_INSTALL_PATH//\\/\/}"/VC/Tools/MSVC/*/bin/Hostx64/${MSVC_ARCH}/dumpbin.exe | tail -n 1)"
+  PATH="$(dirname "${DUMPBIN/C:/\/c}")":"${PATH}"
+  if [[ "${MSVC_ARCH}" = x64 ]]; then
+    dumpbin //EXPORTS //SYMBOLS  "${LIBCRYPTO_PATH}" | grep External | grep -v UNDEF | sed -e 's/.*External\s*|\s*\(.*\)$/\1/' | filter_windows_symbols | grep -E '^\w*$' | sort | uniq >"${SYMBOLS_FILE}"
+  elif [[ "${MSVC_ARCH}" = x86 ]]; then
+    dumpbin //EXPORTS //SYMBOLS  "${LIBCRYPTO_PATH}" | grep External | grep -v UNDEF | sed -e 's/.*External\s*|\s*\(.*\)$/\1/' | remove_leading_underscore | filter_windows_symbols | grep -E '^\w*$' | sort | uniq >"${SYMBOLS_FILE}"
+  else
+    dumpbin //EXPORTS //SYMBOLS  "${LIBCRYPTO_PATH}" | grep External | grep -v UNDEF | sed -e 's/.*External\s*|\s*\(.*\)$/\1/' | filter_windows_symbols | sort | uniq | filter_symbols>"${SYMBOLS_FILE}"
+  fi
+  echo "dumpbin pipes: ${PIPESTATUS[@]}"
+elif [[ "${LIBCRYPTO_PATH}" = *.dylib ]]; then
+  nm --extern-only --defined-only -j  "${LIBCRYPTO_PATH}" | grep -v "${REPO_ROOT}" | sort | uniq | remove_leading_underscore | filter_nm_symbols |  filter_symbols >"${SYMBOLS_FILE}"
+elif [[ "${LIBCRYPTO_PATH}" = *.so || "${LIBCRYPTO_PATH}" = *.lib ]]; then
+  nm --extern-only --defined-only --format=just-symbols  "${LIBCRYPTO_PATH}" | sort | uniq | filter_nm_symbols | filter_symbols >"${SYMBOLS_FILE}"
 else
   pushd "${AWS_LC_DIR}"
-  go run -mod readonly "${AWS_LC_DIR}"/util/read_symbols.go "${LIBCRYPTO_PATH}" "${LIBSSL_PATH}" | filter_symbols >"${SYMBOLS_FILE}"
+  go run -mod readonly "${AWS_LC_DIR}"/util/read_symbols.go "${LIBCRYPTO_PATH}" | filter_symbols >"${SYMBOLS_FILE}"
   popd
 fi
@justsmth justsmth force-pushed the generate/aws-lc-sys-win-bindgen branch from 3ee624e to bbb0240 Compare August 21, 2024 15:34
@@ -1,6 +1,6 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR ISC
// Thu Aug 1 23:56:17 UTC 2024
// Wed Aug 21 14:33:51 UTC 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for the date? It would shrink a lot of the diffs if these files didn't all have the same one line change.

Copy link
Contributor Author

@justsmth justsmth Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a typical diff. We normally only regenerate the bindings when the version number changes (which it will soon).

When the version number changes, nearly every definition included in the bindings is modified due to the version number being part of the symbol for linkage. In this PR, this date difference simply confirms that these bindings were also re-generated (although otherwise unchanged) during the workflow run.

- name: Collect symbols
run: |
./scripts/build/collect_symbols.sh -c aws-lc-sys
- name: Commit & Push changes
run: ./scripts/ci/ci_add_commit_rebase_push.sh "Symbols from ${{ matrix.os }}"
collect-windows-symbols-and-commit:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it useful to run this in the PR so we can see them in the diff, or would it make more sense to automatically run this after the PR has been merged in and this push the generated files straight to the main branch?

Copy link
Contributor Author

@justsmth justsmth Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order for our CI tests to confirm these changes properly, it needs the artifacts generated and included on the PR branch. For review purposes, I often provide links directly to the "manual commits" in the description so that it's more clear what changes require the most scrutiny.

This is an unusual PR b/c it's making substantial changes to our automated bindings generation mechanism. Typically we only run this mechanism with 2-3 line "version bump" commit immediately prior to a new release.

@justsmth justsmth merged commit 40ac459 into aws:main Aug 22, 2024
189 of 196 checks passed
@justsmth justsmth deleted the generate/aws-lc-sys-win-bindgen branch August 22, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants