Fix detect-targets glibc detection (#1421)

* Fix `detect-targets` glibc detection Fixed #1420 Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Fix `detect-targets` glibc detection for Fedora Fedora 37-39 has glibc installed in `/lib64` and `/usr/lib64` instead of `/lib` or `/usr/lib`. Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Improve caching in `detect-targets-alpine-test` Make sure it reuses cached artifacts instead of recompiling everything. Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Mv `test-detect-targets-musl.sh` into `.github/scripts/` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Refactor `ci.yml`: Extract new job `detect-targets-build` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * FIx `detect-targets-ubuntu-test` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Avoid building the entire workspacein `detect-targets-build` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Fix `detect-targets-*-test`: `chmod +x detect-targets` Also fixed testing on Alpine, to make sure it exits with status 1 on assertion failure. Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Fix typo Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Cache all crates in `detect-targets-build` Since `detect-targets` rarely changes and is quite small, it is also reasonable to cache it. Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Add job `detect-targets-more-glibc-test` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Add `detect-targets-nix-test` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Fix `detect-targets-nix-test` executing `detect-targets` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> * Fix workflow: Add all `detect-targets-*` jobs to be dep of `tests-pass` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com> --------- Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
cargo-bins · Sep 29, 2023 · b56a8c3 · b56a8c3
1 parent 86060fa
commit b56a8c3
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 30 deletions.
diff --git a/test-detect-targets-musl.sh → .github/scripts/test-detect-targets-musl.sh b/test-detect-targets-musl.sh → .github/scripts/test-detect-targets-musl.sh
@@ -1,6 +1,4 @@
-#!/bin/ash
-
-# shellcheck shell=dash
+#!/bin/bash
 
 set -exuo pipefail
 
@@ -13,7 +11,7 @@ apk add gcompat
 
 ls -lsha /lib
 
-GNU_TARGET=$(echo "$TARGET" | sed 's/musl/gnu/')
+GNU_TARGET=${TARGET//musl/gnu}
 
 [ "$(detect-targets)" = "$(printf '%s\n%s' "$GNU_TARGET" "$TARGET")" ]
 

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -139,28 +139,50 @@ jobs:
       CARGO_PROFILE_RELEASE_LTO: no
       CARGO_PROFILE_RELEASE_CODEGEN_UNITS: 4
 
-  detect-targets-alpine-test:
+  detect-targets-build:
     runs-on: ubuntu-latest
     env:
       CARGO_BUILD_TARGET: x86_64-unknown-linux-musl
-      TARGET: x86_64-unknown-linux-musl
     steps:
     - uses: actions/checkout@v4
-    - name: Install x86_64-unknown-linux-musl target
-      run: rustup target add $TARGET
+    - name: Install ${{ env.CARGO_BUILD_TARGET }} target
+      run: |
+        rustup target add $CARGO_BUILD_TARGET
+        pip3 install -r zigbuild-requirements.txt
     - uses: Swatinem/rust-cache@v2
+      with:
+        cache-all-crates: true
     - name: Build detect-targets
       run: |
-        pip3 install -r zigbuild-requirements.txt
-        cargo zigbuild --bin detect-targets --features cli-logging --target $TARGET
+        cargo zigbuild --features cli-logging --target $CARGO_BUILD_TARGET
+      # Set working directory here, otherwise `cargo-zigbuild` would download
+      # and build quite a few unused dependencies.
+      working-directory: crates/detect-targets
+    - uses: actions/upload-artifact@v3
+      with:
+        name: detect-targets
+        path: target/${{ env.CARGO_BUILD_TARGET }}/debug/detect-targets
+
+  detect-targets-alpine-test:
+    runs-on: ubuntu-latest
+    needs: detect-targets-build
+    steps:
+    - uses: actions/checkout@v4
+
+    - uses: actions/download-artifact@v3
+      with:
+        name: detect-targets
+    - run: chmod +x detect-targets
+
     - name: Run test in alpine
       run: |
         docker run --rm \
-          --mount src="$PWD/target/$TARGET/debug/detect-targets",dst=/usr/local/bin/detect-targets,type=bind \
-          --mount src="$PWD/test-detect-targets-musl.sh",dst=/usr/local/bin/test.sh,type=bind \
-          alpine test.sh "$TARGET"
+          --mount src="$PWD/detect-targets",dst=/usr/local/bin/detect-targets,type=bind \
+          --mount src="$PWD/.github/scripts/test-detect-targets-musl.sh",dst=/usr/local/bin/test.sh,type=bind \
+          alpine /bin/ash -c "apk update && apk add bash && test.sh x86_64-unknown-linux-musl"
 
   detect-targets-ubuntu-test:
+    needs: detect-targets-build
     strategy:
       fail-fast: false
       matrix:
@@ -169,16 +191,56 @@ jobs:
           - ubuntu-latest
     runs-on: ${{ matrix.os }}
     steps:
-    - uses: actions/checkout@v4
-    - uses: Swatinem/rust-cache@v2
+    - uses: actions/download-artifact@v3
       with:
-        key: ${{ matrix.os }}
-    - name: Build detect-targets
-      run: cargo build --bin detect-targets --features cli-logging
+        name: detect-targets
+    - run: chmod +x detect-targets
+
     - name: Run test in ubuntu
       run: |
         set -exuo pipefail
-        [ "$(./target/debug/detect-targets)" = "$(printf '%s\n%s' x86_64-unknown-linux-gnu x86_64-unknown-linux-musl)" ]
+        [ "$(./detect-targets)" = "$(printf 'x86_64-unknown-linux-gnu\nx86_64-unknown-linux-musl')" ]
+
+  detect-targets-more-glibc-test:
+    needs: detect-targets-build
+    strategy:
+      fail-fast: false
+      matrix:
+        container:
+          - archlinux
+          - fedora:37
+          - fedora:38
+          - fedora:39
+          - fedora
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/download-artifact@v3
+      with:
+        name: detect-targets
+    - run: chmod +x detect-targets
+
+    - name: Run test
+      run: |
+        set -exuo pipefail
+        [ "$(docker run --rm \
+          --mount src="$PWD/detect-targets",dst=/usr/local/bin/detect-targets,type=bind \
+          ${{ matrix.container }} detect-targets )" = "$(printf 'x86_64-unknown-linux-gnu\nx86_64-unknown-linux-musl')" ]
+
+  detect-targets-nix-test:
+    needs: detect-targets-build
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/download-artifact@v3
+      with:
+        name: detect-targets
+    - run: chmod +x detect-targets
+
+    - name: Run test
+      run: |
+        set -exuo pipefail
+        [ "$(docker run --rm \
+          --mount src="$PWD/detect-targets",dst=/detect-targets,type=bind \
+          nixos/nix /detect-targets )" = x86_64-unknown-linux-musl ]
 
   # Dummy job to have a stable name for the "all tests pass" requirement
   tests-pass:
@@ -188,8 +250,11 @@ jobs:
     - cross-check
     - lint
     - release-dry-run
+    - detect-targets-build
     - detect-targets-alpine-test
     - detect-targets-ubuntu-test
+    - detect-targets-more-glibc-test
+    - detect-targets-nix-test
     if: always() # always run even if dependencies fail
     runs-on: ubuntu-latest
     steps:

diff --git a/crates/detect-targets/src/detect/linux.rs b/crates/detect-targets/src/detect/linux.rs
@@ -38,16 +38,23 @@ pub(super) async fn detect_targets(target: String) -> Vec<String> {
                 .expect("unwrap: target always has a - for cpu_arch")
                 .0;
 
-            let cpu_arch_suffix = cpu_arch.replace('_', "-");
-
-            let handles: Vec<_> = [
-                format!("/lib/ld-linux-{cpu_arch_suffix}.so.2"),
-                format!("/lib/{cpu_arch}-linux-gnu/ld-linux-{cpu_arch_suffix}.so.2"),
-                format!("/usr/lib/{cpu_arch}-linux-gnu/ld-linux-{cpu_arch_suffix}.so.2"),
-            ]
-            .into_iter()
-            .map(|p| AutoAbortHandle(tokio::spawn(is_gnu_ld(p))))
-            .collect();
+            let handles: Vec<_> = {
+                let cpu_arch_suffix = cpu_arch.replace('_', "-");
+                let filename = format!("ld-linux-{cpu_arch_suffix}.so.2");
+                let dirname = format!("{cpu_arch}-linux-gnu");
+
+                [
+                    format!("/lib/{filename}"),
+                    format!("/lib64/{filename}"),
+                    format!("/lib/{dirname}/{filename}"),
+                    format!("/lib64/{dirname}/{filename}"),
+                    format!("/usr/lib/{dirname}/{filename}"),
+                    format!("/usr/lib64/{dirname}/{filename}"),
+                ]
+                .into_iter()
+                .map(|p| AutoAbortHandle(tokio::spawn(is_gnu_ld(p))))
+                .collect()
+            };
 
             let has_glibc = async move {
                 for mut handle in handles {
@@ -108,7 +115,7 @@ You are not meant to run this directly.
     if status.success() {
         // Executing glibc ldd or /lib/ld-linux-{cpu_arch}.so.1 will always
         // succeeds.
-        stdout.contains("GLIBC").then_some(Libc::Gnu)
+        (stdout.contains("GLIBC") || stdout.contains("GNU libc")).then_some(Libc::Gnu)
     } else if status.code() == Some(1) {
         // On Alpine, executing both the gcompat glibc and the ldd and
         // /lib/ld-musl-{cpu_arch}.so.1 will fail with exit status 1.