Skip to content

Commit

Permalink
Fix bug where we would try to use cross on ARM hosts for all targets
Browse files Browse the repository at this point in the history
If the target is ARM and the host is ARM, this should work with just `cargo`. I'm not sure if this
works for all ARM host/target combos, or if the CPU architecture must be an exact match.

But it doesn't really matter too much, since at least for now there's no ARM builds for
`cross` (cross-rs/cross#1612). So even if cross-compilation doesn't work
natively, we couldn't use `cross` in this case either.

I also rewrote the `set-cross-compile.sh` in Python. By "I", I mean I had Claude do it and then I
tweaked it.
  • Loading branch information
autarch committed Jan 21, 2025
1 parent 5b5d0b5 commit 52839d9
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 26 deletions.
5 changes: 5 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 1.0.1

- Fixed a bug where this action would attempt to use `cross` when compiling for an ARM Linux target
on an ARM Linux host.

## 1.0.0 - 2025-01-11

The addition of caching is a significant behavior change for this action, so the version has been
Expand Down
2 changes: 1 addition & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ runs:
- name: Determine whether we need to cross-compile
id: determine-cross-compile
shell: bash
run: set-cross-compile.sh ${{ inputs.target }}
run: set-cross-compile.py ${{ inputs.target }}
- name: Install toolchain
uses: dtolnay/rust-toolchain@master
with:
Expand Down
97 changes: 97 additions & 0 deletions set-cross-compile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/usr/bin/env python3

# Written mostly by Claude.ai based on my original bash script.

import sys
import platform
import re
import os
from subprocess import run, PIPE, CalledProcessError


def main() -> int:
"""
Main function to determine cross-compilation requirements.
Returns:
Exit code (0 for success)
"""
if len(sys.argv) < 2:
print("Error: Target architecture argument is required", file=sys.stderr)
return 1

target = sys.argv[1]
needs_cross = check_needs_cross(target)
write_github_output(needs_cross)

return 0


def check_needs_cross(target: str) -> bool:
"""
Determine if cross-compilation is needed based on system and target.
Args:
target: Target architecture string
Returns:
Boolean indicating if cross-compilation is needed
"""
system_info = get_uname_info().lower()

# Check if we're on macOS or Windows
if any(os in system_info for os in ["darwin", "msys", "windows"]):
return False

target = target.lower()

# Check for x86_64 Linux targets on x86_64 Linux host
if (
re.search(r"x86_64.+linux-(?:gnu|musl)", target)
and "x86_64" in system_info
and "linux" in system_info
):
return False

# Check if both host and target are ARM Linux. I'm assuming here that for things like
# "arm-linux-androideabi" or "armv7-unknown-linux-ohos" we'd still need cross.
if (
re.search(r"(?:aarch64|arm).+linux-(?:gnu|musl)", target)
and ("arm" in system_info or "aarch64" in system_info)
and "linux" in system_info
):
return False

return True


def get_uname_info() -> str:
"""
Get system information using uname command.
Returns:
String containing system information
"""
try:
result = run(["uname", "-a"], check=True, text=True, stdout=PIPE)
return result.stdout
except (CalledProcessError, FileNotFoundError):
# Fallback to platform.platform() if uname is not available
return platform.platform()


def write_github_output(needs_cross: bool) -> None:
"""
Write the needs-cross output to GITHUB_OUTPUT environment variable file.
Args:
needs_cross: Boolean indicating if cross-compilation is needed
"""
github_output = os.getenv("GITHUB_OUTPUT")
if github_output:
with open(github_output, "a") as f:
f.write(f"needs-cross={str(needs_cross).lower()}\n")


if __name__ == "__main__":
sys.exit(main())
25 changes: 0 additions & 25 deletions set-cross-compile.sh

This file was deleted.

0 comments on commit 52839d9

Please sign in to comment.