Skip to content

Commit 817a5eb

Browse files
committed
fix: correct build condition logic and musl library extension
Critical fixes: 1. Fix Linux native cargo build condition (build-rust-library:74) - Changed from complex AND logic to simple 'use-cross == false' check - Previous logic would never execute on Linux when use-cross='false' - Now correctly falls back to native cargo when explicitly disabled 2. Fix musl library extension mismatch (build-rust-library:95-96) - Changed from .a (static) to .so (shared) for musl targets - Matches actual Cargo.toml: crate-type = ["cdylib", "staticlib"] - Prevents "Library not found" errors on musl targets 3. Remove redundant use-cross setting (rust-ci.yml:100) - Action already defaults to 'auto' which handles platform detection - Explicit setting was duplicating built-in logic - Added comment explaining the default behavior Minor fixes: 4. Change PowerShell 'return' to 'exit 0' (get-rust-library:154) - Consistent with bash scripts' exit behavior - More explicit about success exit code 5. Update README.md documentation - Clarify build selection logic with numbered steps - Document that musl produces .so not .a files - Add library output format information
1 parent f481f30 commit 817a5eb

4 files changed

Lines changed: 15 additions & 6 deletions

File tree

.github/actions/README.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,13 @@ The composite actions follow a layered approach:
100100
**Build Selection Logic**:
101101
1. If `use-zigbuild='true'` → uses cargo-zigbuild
102102
2. Else if Linux + `use-cross='true'|'auto'` → uses cross
103-
3. Else → uses native cargo
103+
3. Else if `use-cross='false'` → uses native cargo
104+
4. Note: Default is `'auto'` which uses cross on Linux, native elsewhere
105+
106+
**Library Output**:
107+
- All platforms produce shared libraries (.so/.dylib/.dll)
108+
- musl targets produce `.so` (cdylib), not `.a` static libraries
109+
- Cargo.toml specifies `crate-type = ["cdylib", "staticlib"]`
104110

105111
---
106112

.github/actions/build-rust-library/action.yml

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,13 @@ runs:
6565
run: cargo zigbuild --release --target ${{ inputs.target }}
6666

6767
- name: Build with cargo (native)
68-
# Fallback: macOS, Windows, or Linux when not using cross/zigbuild
68+
# Fallback: macOS, Windows, or Linux when explicitly skipping cross/zigbuild
6969
# Note: Uses bash shell even on Windows (via Git Bash) for consistency
7070
# with other build steps and cross-platform script compatibility
7171
if: |
7272
(runner.os == 'macOS' || runner.os == 'Windows' || runner.os == 'Linux') &&
7373
inputs.use-zigbuild != 'true' &&
74-
(inputs.use-cross != 'true' && inputs.use-cross != 'auto')
74+
inputs.use-cross == 'false'
7575
shell: bash
7676
run: cargo build --release --target ${{ inputs.target }}
7777

@@ -82,6 +82,8 @@ runs:
8282
set -euo pipefail
8383
8484
# Determine library name based on target
85+
# Note: Cargo.toml specifies crate-type = ["cdylib", "staticlib"]
86+
# We use the shared library (.so/.dylib/.dll) for all platforms
8587
case "${{ inputs.target }}" in
8688
*-apple-darwin)
8789
LIB_NAME="libtokenizers.dylib"
@@ -90,7 +92,8 @@ runs:
9092
LIB_NAME="libtokenizers.so"
9193
;;
9294
*-unknown-linux-musl)
93-
LIB_NAME="libtokenizers.a"
95+
# musl targets also produce .so (cdylib), not .a
96+
LIB_NAME="libtokenizers.so"
9497
;;
9598
*-pc-windows-msvc)
9699
LIB_NAME="tokenizers.dll"

.github/actions/get-rust-library/action.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ runs:
151151
Write-Host "Checksum verification failed, building locally instead"
152152
cargo build --release
153153
"TOKENIZERS_LIB_PATH=$(Get-Location)\target\release\tokenizers.dll" | Out-File -FilePath $env:GITHUB_ENV -Append
154-
return
154+
exit 0
155155
}
156156
} else {
157157
Write-Host "Warning: No checksum file found, skipping verification"

.github/workflows/rust-ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,4 @@ jobs:
9797
uses: ./.github/actions/build-rust-library
9898
with:
9999
target: ${{ matrix.target }}
100-
use-cross: ${{ matrix.os == 'ubuntu-latest' && 'true' || 'false' }}
100+
# use-cross defaults to 'auto' which automatically uses cross on Linux

0 commit comments

Comments
 (0)