Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix new misdetected platform incompatibility in test-32bit #1874

Merged
merged 3 commits into from
Mar 11, 2025

Conversation

EliahKagan
Copy link
Member

@EliahKagan EliahKagan commented Mar 4, 2025

This uses the rustup installation script instead of an action step, which fixes the following error in test-32bit, which started happening very recently (including on main if the job is rerun) and only happens in the i386 job:

error: toolchain 'stable-i686-unknown-linux-gnu' may not be able to run on this system
note: to build software for that platform, try `rustup target add i686-unknown-linux-gnu` instead
note: add the `--force-non-host` flag to install the toolchain anyway

The command dtolnay/rust-toolchain ran that gave that error was:

rustup toolchain install stable-i686-unknown-linux-gnu --profile minimal --no-self-update

As can be observed in:
https://github.com/EliahKagan/gitoxide/actions/runs/13602883937/job/38151997355

It is intentional that the test-32bit jobs use a toolchain of the same architecture as the target, in order to test that native builds on 32-bit platforms can be performed. This runs at, or almost at, native speeds. This is because these are run on runners whose 64-bit CPUs are capable of running these 32-bit binaries directly. No software-based emulation is used.

It also seems strange that this error is produced with i386 on x86-64, which all x86-64 CPUs are capable of, but not with arm32v7 on AArch64, which not all AArch64 CPUs are capable of. (Libraries may be needed to run toolchain executables such as rustc, so it makes sense that such a diagnostic might occur on both systems, but for it to block only the i386 toolchain installation feels odd.)

It seems there is no clear reliable way to pass --force-non-host through dtolnay/rust-toolchain when installing a toolchain. So this replaces that action with a curl ... | sh ... rustup install step and environment update step, similar to what we are already doing in the other container test job (pure-rust-build), but with the toolchain specified explicitly (for the same reason as before, that it has sometimes been misdetected due to assuming that the system platform matched the architecture of the running kernel).

This makes it easy to pass --force-non-host if we need to, but we actually do not need to do so, seemingly due to differences between this installation method, where the toolchain is installed at the same time rustup is installed, and the subtly different method dtolnay/rust-toolchain uses, where the installation script is run without installing any toolchain, and then a toolchain is installed in a separate rustup command (which is itself the command, shown above, that would need to have --force-non-host on i386).


This change allows both test-32bit jobs to work again. A separate unrelated failure is expected due to #1849, which also happens if that job is rerun on main and happens in all other PRs, except in #1870, which fixes it.

I considered adding these changes to #1870 so it would be a single PR that fixes all broken CI and would allow all CI checks to pass before being merged, but I decided against it because I think these two PRs are best reviewed separately (the approach taken in #1870 is one of several approaches, and you might decide you prefer a different one; piling more into that PR might create pressure to merge it even if it is not the desired fix).

@EliahKagan EliahKagan marked this pull request as ready for review March 4, 2025 09:22
@EliahKagan EliahKagan changed the title Fix new misdetected platform incompatibility in test-32bit Fix new misdetected platform incompatibility in test-32bit Mar 4, 2025
@Skgland
Copy link

Skgland commented Mar 4, 2025

which started happening very recently

This likely started with the release of the new rustup version two days ago https://blog.rust-lang.org/2025/03/02/Rustup-1.28.0.html

Which includes:

  • Installing a host-incompatible toolchain via rustup toolchain install or rustup default will now be rejected unless you explicitly add the --force-non-host flag.

This fixes the following error in test-32bit, which started
happening very recently and only happens in the i386 job:

    error: toolchain 'stable-i686-unknown-linux-gnu' may not be able to run on this system
    note: to build software for that platform, try `rustup target add i686-unknown-linux-gnu` instead
    note: add the `--force-non-host` flag to install the toolchain anyway

The command `dtolnay/rust-toolchain` ran that gave that error was:

    rustup toolchain install stable-i686-unknown-linux-gnu --profile minimal --no-self-update

As can be observed in:
https://github.com/EliahKagan/gitoxide/actions/runs/13602883937/job/38151997355

It is intentional that the test-32bit jobs use a toolchain of the
same architecture as the target, in order to test that native
builds on 32-bit platforms can be performed. This runs at, or
almost at, native speeds. This is because these are run on runners
whose 64-bit CPUs are capable of running these 32-bit binaries
directly. No software-based emulation is used.

It also seems strange that this error is produced with i386 on
x86-64, which all x86-64 CPUs are capable of, but not with arm32v7
on AArch64, which *not* all AArch64 CPUs are capable of. (Libraries
may be needed to run toolchain executables such as `rustc`, so it
makes sense that such a diagnostic might occur on both systems, but
for it to block only the i386 toolchain installation feels odd.)

It seems there is no clear reliable way to pass `--force-non-host`
through `dtolnay/rust-toolchain` when installing a toolchain. So
this replaces that action with a `curl ... | sh ...` rustup install
step and environment update step, similar to what we are already
doing in the other container test job (`pure-rust-build`), but with
the toolchain specified explicitly (for the same reason as before,
that it has sometimes been misdetected due to assuming that the
system platform matched the architecture of the running kernel).

This makes it easy to pass `--force-non-host` if we need to, but we
actually do not need to do so, seemingly due to differences between
this installation method, where the toolchain is installed at the
same time `rustup` is installed, and the subtly different method
`dtolnay/rust-toolchain` uses, where the installation script is run
without installing any toolchain, and then a toolchain is installed
in a separate `rustup` command (which is itself the command, shown
above, that would need to have `--force-non-host` on i386).
Since the default host triple was still detected as that of the
runner outside the container.

With the default host triple set as desired, the default toolchain
should be inferred correctly from that as well.
@EliahKagan
Copy link
Member Author

I've rebased and updated this to replace the change from 8ec6df9 (added in #1882) of making the job non-required with the full fix here.

@Byron Byron merged commit 5443bf2 into GitoxideLabs:main Mar 11, 2025
21 checks passed
@Byron
Copy link
Member

Byron commented Mar 11, 2025

Thanks so much, and again, for all your help!

After 7 days of not doing any of my usual maintenance, everything is a bit chaotic and I will have to save time by mostly just merging PRs without more than a glimpse. Please let me know if I miss something important, you can safely assume that it's not intentional.

@EliahKagan EliahKagan deleted the run-ci/non-host branch March 11, 2025 11:04
@EliahKagan
Copy link
Member Author

EliahKagan commented Mar 11, 2025

I am curious if the ability to override the default host triple (as done here), or to pass --force-non-host, or both (or to add arbitrary arguments to be passed to rustup-init or rustup?) are features that would make sense to propose for dtolnay/rust-tooolchain. If that action had such features, it would have helped here, and we could have kept using it for the affected job.

But I am not sure it should have that. Its purpose, as stated, is to install a toolchain. It also installs rustup if not already installed. Passing --force-non-host when installing a toolchain is within the scope of installing a toolchain. But it should not usually be done, and from the action's description:

It is designed for one-line concise usage and good defaults.

In particular, it turned out that this PR was a place it should not be done. Originally that seemed to be due only to an oddity of which commands required it. But the better solution was to make the toolchain not be host-incompatible by adjusting the default host triple, which is what we actually want in a situation like this where the reason we want the toolchain--rather than just the target--to be for a different architecture is that we are installing it in a container that is conceptually of a different architecture.

It may be that the use in the test-32bit jobs in this repository is sufficiently specialized to be outside the proper scope of dtolnay/rust-tooolchain.

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.

3 participants