Add Flowey support for Arch Linux #2083
Conversation
stunes-ms
requested review from
Copilot and
romank-msft
and removed request for
Copilot
| let pkg = match linux_distribution { | ||
| FlowPlatformLinuxDistro::Fedora => "gcc-x86_64-linux-gnu", | ||
| FlowPlatformLinuxDistro::Ubuntu => "gcc-x86-64-linux-gnu", | ||
| FlowPlatformLinuxDistro::Arch => "gcc", |
There was a problem hiding this comment.
Is there no x86-64 version of the symlink? Like what if we were cross compiling for x64 from arm?
There was a problem hiding this comment.
Nope. Arch is explicitly an x86-only distro (https://wiki.archlinux.org/title/Frequently_asked_questions#What_architectures_does_Arch_support?). There is Arch Linux ARM, but that's a separate project.
There was a problem hiding this comment.
Oh right. Maybe we should have some unreachables or something for cross compilation cases then instead of pretending it's supported?
There was a problem hiding this comment.
Also what paths would we end up going down if someone did try to run this code on Arch ARM?
There was a problem hiding this comment.
Just pushed a change with one possible fix: bail here if we see that we're on Arch and the host is ARM.
Another possibility would have been in pipeline.rs linux_distro: if we find Arch and the host is ARM, bail out there. It's less code, probably, but it's also action-at-a-distance to some extent, maybe?
|
This is great, thank you for contributing! @jstarks brought this up in a different context, but the more complexity we have here then the more difficult it becomes to maintain quality in flowey. Do we need a flowey ci for ci that runs these flows across various architectures and distros? Or, we document that we only officially support whatever is used in the GH runners (e.g. ubuntu 24 lts), and everything else is best/community effort? |
|
I believe the latter is already the case, if not it's the unspoken case and we should document it. |
|
Or rather, the policy is "We support what we run in CI (Ubuntu) and our guide's recommended local dev environment (WSL with Ubuntu)." Ideally those will always be the same. |
|
As part of another workstream (reproducible builds) we are reconsidering the way that dependencies are installed. We will likely be switching to something Nix based. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Arch Linux to the Flowey build system. The main purpose is to extend the existing Linux distribution support to include Arch Linux alongside the already supported Fedora and Ubuntu distributions.
Key changes:
- Added Arch Linux as a recognized Linux distribution in the platform detection system
- Updated package management logic to handle pacman (Arch's package manager) commands
- Modified cross-compilation toolchain setup to use Arch-specific package names and binaries
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| flowey/flowey_core/src/node.rs | Added Arch variant to FlowPlatformLinuxDistro enum |
| flowey/flowey_core/src/pipeline.rs | Added detection logic for Arch Linux in /etc/os-release |
| flowey/flowey_lib_common/src/install_dist_pkg.rs | Added pacman package manager support for querying, updating, and installing packages |
| flowey/flowey_lib_common/src/download_azcopy.rs | Added libarchive package mapping for Arch Linux |
| flowey/flowey_lib_common/src/_util/extract.rs | Added libarchive package mapping for Arch Linux |
| flowey/flowey_lib_hvlite/src/build_openhcl_initrd.rs | Added python package name mapping (python vs python3) for Arch Linux |
| flowey/flowey_lib_hvlite/src/init_cross_build.rs | Updated cross-compilation toolchain setup with Arch-specific GCC package names |
| flowey/flowey_lib_hvlite/src/run_split_debug_info.rs | Updated objcopy tool setup with Arch-specific binutils package names |
| FlowArch::Aarch64 => { | ||
| anyhow::bail!("Arch Linux ARM is not supported") | ||
| } |
There was a problem hiding this comment.
Duplicate error handling logic for unsupported Arch Linux ARM appears multiple times. Consider extracting this into a helper function or constant to reduce code duplication.
Copilot uses AI. Check for mistakes.
| FlowArch::Aarch64 => { | ||
| anyhow::bail!("Arch Linux ARM is not supported") | ||
| } |
There was a problem hiding this comment.
This is the same error message and handling logic as in the previous file. Consider consolidating this repeated pattern into a shared utility function.
Copilot uses AI. Check for mistakes.
| FlowArch::Aarch64 => { | ||
| anyhow::bail!("Arch Linux ARM is not supported") | ||
| } |
There was a problem hiding this comment.
This is the third occurrence of the same error handling pattern. The repeated logic should be extracted into a common helper function.
Copilot uses AI. Check for mistakes.
stunes-ms
added
the
release-ci-required
stunes-ms
force-pushed
the
user/mikestunes/arch
branch
from
77137a8 to
24d27cc
Compare
stunes-ms
force-pushed
the
user/mikestunes/arch
branch
from
24d27cc to
77c3aa4
Compare
I use Arch btw
This was mostly straightforward, thanks to previous work to add support for additional Linux distros. This change mostly changes a few places from building package/binary names from arch strings, to just giving the whole package/binary name explicitly.