Skip to content

feat: add cargo install fallback for dist prebuilt binary installation - #2352

Open
ChanTsune wants to merge 1 commit into
axodotdev:mainfrom
ChanTsune:feat/cargo-install-fallback
Open

feat: add cargo install fallback for dist prebuilt binary installation#2352
ChanTsune wants to merge 1 commit into
axodotdev:mainfrom
ChanTsune:feat/cargo-install-fallback

Conversation

@ChanTsune

Copy link
Copy Markdown

When a prebuilt binary installer is unavailable for the target architecture (e.g. riscv), the generated CI workflow now falls back to cargo install cargo-dist --locked --version <version> instead of failing outright.

resolve #2351

When a prebuilt binary installer is unavailable for the target
architecture (e.g. riscv), the generated CI workflow now falls back
to `cargo install cargo-dist --locked --version <version>` instead
of failing outright.
@mistydemeo

Copy link
Copy Markdown
Contributor

I'm not sure if this is the right approach. This will catch any curl | sh failure, even ones that are unrelated to the installer being missing for a given architecture. I think it would make more sense to vary which install command we inject based on whether a binary should be available or not.

@gounthar

Copy link
Copy Markdown

Hi, I came at this from the other end: I added riscv64 release targets to atuin, they merged, and then had to revert a day later (atuinsh/atuin#3745) because the release workflow can't install dist on a riscv64 runner. So this thread is the blocker for us too, and I'd rather help finish it than open something competing.

@mistydemeo, your objection makes sense to me. || cargo install catches a network blip or a bad checksum just as readily as a missing binary, and turns both into a silent twenty-minute source build.

The pieces for the narrower version look like they're already in place. for_triple receives the host triple of the job (github.rs:331 passes real_triple), and the set of hosts dist publishes for is small and known: x86_64 and aarch64 for linux and macOS, x86_64 for Windows. So the strategy can pick cargo install only when the host isn't one of those, and keep the installer with no fallback everywhere else. A curl failure on x86_64 then still fails loudly, which I think is what you want.

It also stays out of the way of the existing snapshots. Since the generated command only changes for hosts that have no binary today, the 60-odd .snap files in this PR would go back to unchanged, and it'd add one new case covering a custom runner on a target dist doesn't ship.

@ChanTsune, this is your issue and your PR, so I don't want to step on it. Happy to push that version to your branch if you'd like, or open a separate PR that credits you, or just leave it with you if you were already partway there. Whichever you prefer.

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.

Add cargo install fallback when prebuilt binary installer is unavailable

3 participants