rust-lang / rustup

The Rust toolchain installer
https://rust-lang.github.io/rustup/
Apache License 2.0
6.03k stars 877 forks source link

rustup breaks with cargo-make due to environment variables #3029

Open sffc opened 1 year ago

sffc commented 1 year ago

Problem

It appears that a recent rustup update today caused our GitHub Actions CI to start failing to install a pinned nightly version.

Passing (7hrs ago): https://github.com/unicode-org/icu4x/actions/runs/2650696918

Failing (1hr ago): https://github.com/unicode-org/icu4x/actions/runs/2652484278

GitHub Actions config file: https://github.com/unicode-org/icu4x/blob/main/.github/workflows/build-test.yml

CC @Manishearth

Steps

View the above CI reports. See how the jobs start failing with messages such as:

[cargo-make] INFO - Execute Command: "rustup" "run" "nightly-2022-04-05" "cargo" "wasm-build-release" "--package" "icu_capi_cdylib" "--features" "provider_test,smaller_test"

and

[memory] > error: no such subcommand: `+nightly-2022-04-05`

Possible Solution(s)

No response

Notes

No response

Rustup version

latest

Installed toolchains

nightly-2022-04-05
Manishearth commented 1 year ago

Another error that we're seeing is:

error: "/home/runner/.rustup/toolchains/1.61-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/Cargo.lock" does not exist, unable to build with the standard library, try:
        rustup component add rust-src --toolchain nightly-2022-04-05-x86_64-unknown-linux-gnu

Which I think is our fault, and I have a fix in https://github.com/rust-lang/rustup/issues/3029 . However it's unclear how the rustup update broke that (maybe the culprit isn't the rustup update)


Apologies for the low-context issue, I figured given that rustup releases are rare it was more valuable for us to report this bug ASAP and work on figuring out more details afterwards. The +nightly thing being broken still seems sus.

sffc commented 1 year ago

Note that the following error only appears on Windows:

[memory] > error: no such subcommand: `+nightly-2022-04-05`
Manishearth commented 1 year ago
error: "/home/runner/.rustup/toolchains/1.61-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/Cargo.lock" does not exist, unable to build with the standard library, try:
        rustup component add rust-src --toolchain nightly-2022-04-05-x86_64-unknown-linux-gnu

Okay, looking at this further this is really weird. In our "Load cortex target for no_std build" CI task section in this failing job we call:

        rustup component add --toolchain nightly-2022-04-05 rust-src
        rustup target add thumbv7m-none-eabi --toolchain nightly-2022-04-05
        rustup target add thumbv8m.main-none-eabihf --toolchain nightly-2022-04-05
        rustup target add x86_64-unknown-linux-gnu --toolchain nightly-2022-04-05

Looking closely at the error, I see a couple things wrong:

Both of these things seem to be Rustup bugs. The +nightly-foo failing on Windows seems like another rustup issue but unrelated.

kinnison commented 1 year ago

If you are doing cross-builds from within invocations of cargo are you perhaps not being sure to clean your environment of things like RUSTC RUSTDOC RUSTFLAGS or other environment variables which might affect builds? This might be the equivalent of #3031 if so.

If I'm barking up the wrong tree, please tell me and we can investigate further.

Kryptos-FR commented 1 year ago

Maybe a different issue but since the release of the new rustup version, it fails on Windows CI jobs on GitHub.

Minimal repro

name: Build (Windows)

on:
  workflow_dispatch:
jobs:
  Windows:
    runs-on: windows-2019
    steps:
      - name: Set up rust
        env:
          RUSTUP_USE_CURL: 1
        run: |
          rustup toolchain install nightly
          rustup +nightly target add i686-pc-windows-gnu
          rustup default nightly-i686-pc-windows-gnu

Errors in log

info: syncing channel updates for 'nightly-x86_64-pc-windows-msvc'
info: latest update on 2022-07-12, rust version 1.64.0-nightly (38b72154d 2022-07-11)
info: downloading component 'cargo'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: installing component 'cargo'
info: installing component 'rust-std'
info: installing component 'rustc'
  nightly-x86_64-pc-windows-msvc installed - rustc 1.64.0-nightly (38b72154d 2022-07-11)
info: checking for self-updates
info: downloading self-update
error: could not copy file from 'C:\Users\runneradmin\.cargo\bin\rustup-init.exe' to 'C:\Users\runneradmin\.cargo\bin\rustup.exe': Access is denied. (os error 5)
ResourceUnavailable: D:\a\_temp\aaf6a12a-e61a-436f-8bde-96f840d0d276.ps1:3
Line |
   3 | rustup +nightly target add i686-pc-windows-gnu
     |  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Program 'rustup.exe' failed to run: An error occurred trying to start process
     | 'C:\Users\runneradmin\.cargo\bin\rustup.exe' with working directory 'D:\a\shards\shards'. Access is
     | denied.At D:\a\_temp\aaf6a12a-e61a-436f-8bde-96f840d0d276.ps1:3 char:1 + rustup +nightly target add
     | i686-pc-windows-gnu + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~.
Error: Process completed with exit code 1.

Workaround

rustup toolchain install --no-self-update nightly

Looks like it is the self update causing havoc.

Note that it only fails on Windows workers. Linux and macOS didn't have that failure.

kinnison commented 1 year ago

That is a problem with Windows locking the file harder. You can add rustup set auto-self-update disable in your CI if you want to prevent this kind of thing.

Emilgardis commented 1 year ago

the windows error is very sporadic, see https://github.com/Emilgardis/rustup-windows-issue/runs/7298752016?check_suite_focus=true

to circumvent, do what @kinnison suggest, example with actions-rs/toolchain (and only doing it on windows since it works well on the unix runners):

- run: rustup set auto-self-update disable
  if: contains(runner.os, 'windows')
  shell: bash
- name: Install Rust toolchain
  uses: actions-rs/toolchain@v1
  with:
    toolchain: stable
    default: true
    profile: minimal
Manishearth commented 1 year ago

If you are doing cross-builds from within invocations of cargo are you perhaps not being sure to clean your environment of things like RUSTC RUSTDOC RUSTFLAGS or other environment variables which might affect builds? This might be the equivalent of #3031 if so.

Hmm, we're using cargo-make which might be doing that? But cargo make is used a lot by folks, if that's breaking that's probably a problem.

But I don't think cargo-make sets toolchains that way.

Manishearth commented 1 year ago

https://github.com/unicode-org/icu4x/pull/2170 this confirms it, for whatever reason rustup has decided to use the stable compiler there

Manishearth commented 1 year ago

@kinnison @pietroalbini Given that there are no easy ways to "select" a rustup version at this time, would it be a good idea to roll back the release until we know the scope of this problem and can fix it? As far as ICU4X is concerned we can disable some of these tests if we have to, but I'm worried about the wider impact here, it doesn't seem like we're the only ones with this problem, and it's rather hard to debug since I haven't been able to reproduce it locally (I suspect I will be able to on a fresh system)

Manishearth commented 1 year ago

What we know so far:

https://github.com/rust-lang/rustup/issues/2958 is a part of this

The problem is that when you call cargo make, cargo sets the RUSTC/etc environment variables based on the current toolchain, and then calls cargo-make .... cargo make then calls cargo +nightly build (i.e., rustup run nightly cargo build) or whatever

this ends up with the following invocation: RUSTC=[..]/stable/[..]/rustc cargo +nightly build. Which greatly confuses cargo and leads to very mixed and confusing error messages, since it's trying to use the nightly toolchain, but stable binaries.

Part of the solution here is probably that cargo-make should be clearing rust-relevant environment variables (cc @sagiegurari) since they get passed in by cargo. Especially when cargo-make is doing its own toolchain selection.

But I'm not sure if that's all of it.

Manishearth commented 1 year ago

from @kinnison

... anything which runs under cargo and expects to invoke cargo on something other than the host toolchain, they should clean up the environment properly before invoking it

Manishearth commented 1 year ago

Thinking about it more the +nightly-foo issue on windows may also be due to the same kind of bug, if the RUSTC variable ends up with rustc getting called directly instead of te rustup shims

sagiegurari commented 1 year ago

@sffc @Manishearth from what i remember, cargo-make does not set RUSTC env var. maybe i don't remember correctly.

however, cargo-make does set the CARGO env var before calling commands that require a specific toolchain. its actually related to sagiegurari/cargo-make#658 which was reported by @sffc 😄

if a change is needed feel free to open an issue and specify whats wrong with the behavior so i'll know what needs fixing.

Manishearth commented 1 year ago

@sagiegurari no, the problem is that when you run cargo make, what happens is that cargo is run (under the default toolchain), which then calls cargo-make with the environment variables set, and cargo-make doesn't unset them before calling things further down

Manishearth commented 1 year ago

That may not be the only problem however. cargo-make setting CARGO when needed also seems correct (maybe it should be setting more things?)

sagiegurari commented 1 year ago

question, rustup manages the rust installations. doesn't it override any existing env var to satisfy the command line request?

i could unset stuff but later cargo will set more env vars which in turn mess up rustup again and its all internal rust implementation.

so it feels like something that rustup needs to manage maybe? does it make sense?

unsetting those 3 env vars is easy and can be worked around now in the makefile env block of any project using the unset capability. so that could be used to test it solves the issue and if so i got no problem pushing it to the master. but, it might break again in the future if cargo changes something.

Manishearth commented 1 year ago

@sagiegurari I don't think that's the problem. This is in part cargo behavior, and how cargo calls custom subcommands.

In general I think the following can be taken as an authoritative statement on what the rustup team's intent is here:

from @kinnison

... anything which runs under cargo and expects to invoke cargo on something other than the host toolchain, they should clean up the environment properly before invoking it

So I don't think "things changing later" is a real problem if the team commits to (and documents) this.

unsetting those 3 env vars is easy and can be worked around now in the makefile env block of any project using the unset capability.

Going to try this!

Manishearth commented 1 year ago

I suspect on cargo-make's side, the answer is that it should unset RUSTC and RUSTDOC, and it should either unset CARGO or set it to a toolchain, if specified.

simbleau commented 1 year ago

I think this is relevant https://github.com/rust-lang/rustup/issues/3025

As far as CI goes, trying to update the toolchain on nightly is broken at least (Ubuntu 22.04).

I think this can help narrow the problem: https://github.com/simbleau/website/commit/bac36b225cb209e97a86646835b5925ce6f58e4b

This caused my pipeline to pass.

And locally, I couldn't install the newest nightly until I did a rustup uninstall and reinstalled nightly, it worked.

Somewhere, somehow, someone pushed a bad version, and the only solution is to uninstall and reinstall.

Manishearth commented 1 year ago

(note that rustup updates are done via rustup self update)

kinnison commented 1 year ago

We need, at some point, to start to resolve this problem in a way which permits us to give the performance boost we were trying to achieve; without breaking your CI

Manishearth commented 1 year ago

I mean, we're also happy to make changes on our end, if cargo-make can be updated to handle this and stuff it's also fine.

Such CI tools are popular but I don't think there are actually all too many of them.

Emilgardis commented 1 year ago

if a cargo subcommand is currently doing from it's own process cargo +<toolchain> <command> is the way to resolve when it's fixed: rustup run <toolchain> cargo <command> instead?

sagiegurari commented 1 year ago

if cargo-make can be updated to handle this and stuff it's also fine

@Manishearth I can unset these 3 env vars by default at the start of cargo-make. that's easy. that means cargo setting them would be ignored in a way.

Manishearth commented 1 year ago

@kinnison btw, mind changing this issue title to something like "new rustup version breaks with error: no such subcommand:+nightly-2022-04-05`" (unfortunately I don't have edit access to this repo anymore)

sffc commented 1 year ago

(title edited)

rami3l commented 2 months ago

A gentle ping: cargo-make has made https://github.com/sagiegurari/cargo-make/pull/1060 now, does this issue still exist since?

wmmc88 commented 2 months ago

From the cargo-make side, this issue was worked around and resolved, but im unsure if rustup want to leave this open to address @kinnison comments:

https://github.com/rust-lang/rustup/issues/3029#issuecomment-1229170650