Stop blocking nightlies when RLS/rustfmt/clippy is missing

[kennytm]

We currently will not release nightlies when RLS/rustfmt/clippy is missing to avoid accidentally removing them (913a3b3650231884a6bc8ad3cd98a44e8552b45a). But this impedes use of the compiler not involving these tools e.g. CI testing and bisection.

rustup 1.12.0 (573895abc 2018-07-07) has recently been released, which according to rust-lang/rust-central-station#61 (comment) should include a patch that when a component is unavailable will simply refuse to upgrade instead of removing it.

This means we could mark missing RLS/rustfmt/clippy as available = false instead of blocking the entire nightly.

[pietroalbini]

Doing that is going to be annoying if someone relies on a component being available on the latest nightly in CI, where the rustup patch doesn't help.

[kennytm]

@pietroalbini I'd say not able to update to the latest nightly is even more annoying. Those people who need the components to be always present on a fresh install could use the beta channel, after 1.29 is promoted.

Or could we have a "nightly + all components" channel for this?

[alexcrichton]

The main downside of this is that new users may have a subpar experience. Installation of the RLS isn't guaranteed to work (for example). A new type of channel might solve that, but nightly is pretty ingrained as the nightly channel nowadays.

For bisection though don't the per-commit builds work better anyway? And they're always available regardless of whether the RLS succeeds to build or not I believe

[Mark-Simulacrum]

Yes, we should be using the commit builds for bisection (I believe that cargo-bisect-rustc should fully support this use case). I think if we were to go ahead with something like this I'd rather see us explore an approach where we make rustup scan backwards when a target is added that isn't currently available, before we do something like this.

[kennytm]

@alexcrichton Per-commit doesn't work when the regression is too old e.g. rust-lang/rust#51907

The current situation is also subpar and confusing for existing users (e.g. irc-1, irc-2, irc-3, discord-4, rust-lang/rust#49929).

The latest breakage lasts from 2018-06-29 to 2018-07-06 (a whole week) and the fix (rust-lang/rust#51677) isn't applied because it is blocked by so many other details (snapshot cargo, license opt-out, duplicate artifacts). I don't think this is manageable when we deliver even more components via rustup. Delaying nightlies also mean we lengthen the time between an issue is closed on GitHub and the fix is delivered to the users.

(Also, new users can always get a working RLS/rustfmt on the stable/beta channel, and soon clippy too after 1.29 enters beta.)

I wonder if we could offer "install dev tools" as an option in rustup-init:

$ curl https://sh.rustup.rs -sSf | sh
info: downloading installer

[snip]

Current installation options:

   default host triple: x86_64-unknown-linux-gnu
     default toolchain: stable
     development tools: no
  modify PATH variable: yes

1) Proceed with installation (default)
2) Customize installation
3) Cancel installation
>2

I'm going to ask you the value of each these installation options.
You may simply press the Enter key to leave unchanged.

Default host triple?


Default toolchain? (stable/beta/nightly/none)
nightly

Install development tools (rls, rustfmt, and clippy)? (y/n)
y

Modify PATH variable? (y/n)



Current installation options:

   default host triple: x86_64-unknown-linux-gnu
     default toolchain: nightly
     development tools: yes 
  modify PATH variable: yes

1) Proceed with installation (default)
2) Customize installation
3) Cancel installation
>1

[will proceed to install the latest nightly with all 3 components provided]

[alexcrichton]

@kennytm all excellent points!

@nrc I'm curious, how would you feel about something like this? (releasing nightlies missing rls/clippy/rustfmt)

I suspect we'll need a feature like @kennytm mentioned with allowing rustup to automatically search for the last working nightly (or something like that), but it may not play well with many installation scripts today.

[Manishearth]

I'd say not able to update to the latest nightly is even more annoying.

I'm curious: why? I've found that there's not much desire to be on the extreme bleeding edge nightly, just a recent nightly. Sometimes folks want the latest when a new feature comes out, but usually folks just need something recent.

The exception to this is compiler dev when you need bleeding edge nightlies to test stuff.

[kennytm]

@Manishearth Sometimes folks are blocked by a compiler issue, fixed, but not seeing anything released.

[Manishearth]

Hm, that's fair. I guess being able to set whether you want to update to channels without tools or not in rustup settings would be nice On Jul 10, 2018 11:01 AM, "kennytm" <notifications@github.com> wrote: @Manishearth <https://github.com/Manishearth> Sometimes folks are blocked by a compiler issue, fixed, but not seeing anything released. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/rust-lang/rust-central-station/issues/69#issuecomment-403913478>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSFQ9txTRn6zpDQslxf25NewQ7UDtks5uFOvvgaJpZM4VHzPR> .

[nrc]

@nrc I'm curious, how would you feel about something like this? (releasing nightlies missing rls/clippy/rustfmt)

I feel basically OK about it - there is stuff in Rustup to do it, we'd just need to check it works as expected and gives good error messages.

However, I do think that it is not the optimal experience, as mentioned above for new users who want to get the RLS and it's not there (the support for this use case in Rustup is currently very poor, but I don't think it would ever be great).

IMO the pressure for the latest tools is just as great as for the latest nightly. People shouldn't be blocked waiting on a compiler bug - if they are users, they should use stable and if they are Rust devs they can build their own compiler.

I would like to see how we go with the current scheme and if the breakage is intolerable - it's mostly not been too bad recently.

I would also like to leave any changes like this until after the edition - it's always a lot of work and I'm overloaded at the moment.

[Mark-Simulacrum]

@nrc and I just talked and the latest point we have is that when building nightlies, go with the following. @kennytm does this look reasonable and relatively straightforward to implement? Is there some clarification that's needed?

To be clear, I believe we want to do this now (and possibly revert after we've shipped the edition).

Use the commit which:

• Is after the last nightly
• Has the most tools built
• Is the most recent

[Manishearth]

This sounds pretty good. Though -- with "most tools" that means some nightlies won't have some tools, yeah? When you install a tool can we by default set a "required" flag for that tool locally? Such that if you rustup update to a nightly without the tool, we then have that prompt "hey we can update you to latest nightly FOO but it doesn't have the tool, would you like to update to nightly BAR instead, or remove the requirement on this tool?" and if the user selects the latter option the required flag is unset.

There are a lot of folks who use tools but don't care about having the recentmost nightly, and there are folks who need cutting edge nightlies (those using heavy churn features like async). This caters to them both.

[nrc]

The way rustup works is that if you have a tool installed and it is missing from the nightly, then it won't install the nightly and gives you a warning. You can then run it again with a --force flag to install without the tool.

However, this could do with some battle testing since it hasn't been used 'in real life' before.

[nrc]

And to be clear for the motivations, we want to:

• not block releasing a nightly on broken tools
• issue a nightly with tools if there has been one 'recently' even if the commit at the moment the nightly is rolled out does not have tools.

[Manishearth]

Yeah so I'm suggesting a more user friendly workflow for that, where instead of --force it will just prompt you and ask you to choose (and perhaps permanently remember the choice). I feel in a lot of these discussions we've been prioritizing the needs of the cutting-edge nightly users which I think is a minority of the community. We should try to ensure the probably-majority nightly-users-who-just-want-something-recentish group has a decent experience.

[RalfJung]

To add to the list of arguments for why this issue is worth solving, it is also strongly impeding miri development: miri CI uses the latest nightly (what else would it do), but that means to land a fix in miri that makes its toolstate go green again, we need to get a new nightly.

We've had a PR blocked for 5 days until the nightly 2018-07-30 was finally produced, and we currently have a PR blocked because there was no nightly since 2018-08-06. This is the third time since July 1st that there was no nightly for three continuous days.

I understand that some people and probably many/most users get a better experience when clippy/rls is guaranteed to be available, but for others like me who do not intend to use clippy/rls on nightly, the lack of nightly releases can seriously impact development (keeping PRs open far longer than they should be, which leads to a mess of unmerged branches accumulating that is bound to require extra work to land nicely).

I'd be perfectly fine switching from nightly to nightly-without-tools or so if we decide to make this a separate channel to subscribe to (that can share its artifacts with the nightly channel), to keep the "default nightly experience" one where tools are always available.

---

I also wonder if it might help to establish a culture of "if you break a critical tool, please try to fix it"? I am not sure if the problem usually is that landing an update is hard due to interdependencies, or that nobody is quite pushing hard enough. (I am not trying to assign blame here, just trying to see if there are ways we could fix the tools faster.)

[kennytm]

Use the commit which:

• Is after the last nightly
• Has the most tools built
• Is the most recent

SGTM :)

So during promote-release, we would

1. Download the manifest of the previous release as usual. Obtain pkg.rustc.git_commit_hash (assume it was 1234abcd).
2. List all commits after that version with git rev-list --first-parent 1234abcd...
3. Set "the commit with most tools" to None.
4. For each commit, from first to last (thus new to old),
   1. Query S3 to check if the components are present

      for tool in rustc rust-std cargo rls rustfmt clippy; do
          aws s3 ls s3://rust-lang-ci2/rustc-builds/$commit/$tool-$branch-x86_64-unknown-linux-gnu.tar.xz.sha256 | grep sha256
      done

   2. If all 6 components are present, set this commit as "the commit with most tools", break.
   3. If any of rustc, rust-std and cargo is absent, continue.
   4. If there are strictly more components than "the commit with most tools", update the variable to Some(current_commit).
   • (maybe we could do bisection instead of linear search here)
5. If "the commit with most tools" is Some(rev), proceed to download artifacts, otherwise just skip.

instead of --force it will just prompt you and ask you to choose (and perhaps permanently remember the choice)

rustup is not an interactive tool (unlike rustup-init). Adding an "ask you to choose" is a very bad idea IMO.

[RalfJung]

List all commits after that version

I suppose this means strictly after that version, i.e., it never considers the commit that was used for the last nightly?

If there are strictly more components than "the commit with most tools"

Just to clarify, if "commit with most tools" is None, that counts as -inf tools, or so?

IOW, if all commits in that range have no tool, then it uses the latest?

---

This could still lead to odd behavior in some strange cases: Imagine we start with all tools working. Commit 1 preserves that state, commit 2 breaks one tool, commit 3 breaks another tool; all of these happen at the same day.
Assuming the tools do not get fixed, the next three nightlies would now be for commits 1, 2, 3 -- so, the nightly at day X+3 would actually be from day X+0!

Should the commit selection in the beginning be restricted to things merged in that given day?

[kennytm]

I suppose this means strictly after that version, i.e., it never considers the commit that was used for the last nightly?

Yes, in git a...b excludes the commit a.

Just to clarify, if "commit with most tools" is None, that counts as -inf tools, or so?

IOW, if all commits in that range have no tool, then it uses the latest?

Effectively None has zero components. A valid commit require at least 3 components (rustc, cargo, std). If all commits in the range has no components at all, no nightly will be promoted. If all commits only have the 3 essential components but no tools, yes the latest one will be used.

Should the commit selection in the beginning be restricted to things merged in that given day?

Good idea. Now just needs to figure out the git command to do this :D.

[RalfJung]

Good idea. Now just needs to figure out the git command to do this :D.

Something like this?

git rev-list --since 2018-08-08 --first-parent 25e32903ef5bc8b9e4ea22f6d7c3017ceabd707c...

[Manishearth]

@RalfJung I just want to highlight that miri, RLS, clippy, etc are all pretty special cases in that they hook deeply into the compiler. I am very wary of primarily solving this problem for them, because there's a ready workaround -- use rustup-toolchain-install-master. Clippy plans on doing this (rust-lang/rust-clippy#2941)

However I think the --force plan fixes your use case.

rustup is not an interactive tool (unlike rustup-init). Adding an "ask you to choose" is a very bad idea IMO.

@kennytm Doesn't have to be interactive, as long as we provide a neat flow:

• If you add a tool, it is marked as required
• When you attempt to update a nightly, it will update to the latest nightly with that tool (if possible), and then warn you "hey i couldn't update to latest nightly FOO because it didn't have the tool, here's nightly BAR. If you want to get the latest nightly use rustup update channelname --force. Bear in mind this will mark TOOLNAME as unnecessary for future rustup update calls."
• When you call it with --force, it will again mention that TOOLNAME has been marked as unnecessary, and also mention that you can mark it as necessary again by recalling rustup component add TOOLNAME

[kennytm]

@Manishearth I interpreted "instead of --force" means there wouldn't be a --force flag 😛.

I assume "mark TOOLNAME as unnecessary" means:

1. The TOOLNAME component will be removed using the conventional rustup component remove $TOOLNAME mechanism
2. ~/.rustup/settings.toml will be updated to include unnecessary = ["TOOLNAME"], as a marker to allow the messages in your third point.
3. Because the TOOLNAME has been removed, running rustup update will not reinstall TOOLNAME again, even if it is now available.

---

I'm thinking if this is possible:

When a component is marked unnecessary by --force, it is "stubbed out" instead of removed. rustup update (with or without --force) will still check if TOOLNAME is available and install it if exists.

[Manishearth]

Oh no I absolutely was suggesting we have an interactive workflow, but if we can't then I'm okay with a manually spelled out workflow.

The TOOLNAME component will be removed using the conventional rustup component remove $TOOLNAME mechanism

No, it means that the TOOLNAME still exists, however it is possible to update to a nightly without it. But if it exists in the nightly it will work.

This is the "stubbed out" solution you talk of.

[kennytm]

Interactive is bad because existing environment would have already expected rustup is non-interactive. If a CI runs rustup update and rustup suddenly stops and asks "This will disable TOOLNAME, proceed? [y/N]", the builder will just be stuck for 60 minutes until time out.

So the manually spelled out workflow is likely the only viable option. I believe we both agree on how this manual update --force workflow should be implemented.

BTW,

• there should also be a rustup component mark-unnecessary TOOLNAME to manually set a component unnecessary?

• in a new install, if TOOLNAME is missing, and I run rustup component add TOOLNAME, this would fail as usual? Could we do rustup component add TOOLNAME --force to get a "stubbed out" component?

[Manishearth]

makes sense

there should also be a rustup component mark-unnecessary TOOLNAME to manually set a component unnecessary?

probably

Could we do rustup component add TOOLNAME --force to get a "stubbed out" component?

yes

---

Also we should ensure that the default state (when the flag isn't present at all) is required=true (if the component is installed, of course) so that there's a smooth transition.

[Manishearth]

Oh, also, attempting to use a non-required tool on a toolchain where it is installed but not present should perhaps help you roll back a bit or something?

We kinda want this workflow for the normal case too, in our current proposal there's no way to say "hey i want a nightly with clippy and RLS" if your currently installed nightly doesn't have it.

[Mark-Simulacrum]

I think it makes sense to implement linear scan backwards for "I want this tool" -- shouldn't be too difficult I imagine and would only need to be done once.

[nrc]

To add to the list of arguments for why this issue is worth solving, it is also strongly impeding miri development: miri CI uses the latest nightly (what else would it do), but that means to land a fix in miri that makes its toolstate go green again, we need to get a new nightly.

@RalfJung you can use https://github.com/kennytm/rustup-toolchain-install-master to get a 'nightly' from the latest commit (without building from source), whether or not it was actually released. That means you can always continue development.

I know it is frustrating to not be able to land PRs. To be able to do so, you might consider moving miri out of tree (like Rustfmt, Clippy, etc) so you can land PRs more quickly (this of course has the downside that you need to land PRs to update the submodule, but I have found that trade-off worthwhile).

[RalfJung]

miri is out of tree, which kind of this the entire problem. ;)

I will look into maybe using https://github.com/kennytm/rustup-toolchain-install-master for miri's CI.

[alexcrichton]

I've opened rust-lang/rust-central-station#79 to start the ball rolling on this

[kennytm]

FYI https://crates.io/crates/rustup-find

[shepmaster]

should include a patch that when a component is unavailable will simply refuse to upgrade instead of removing it.

I don't know this is true...

$ rustup update
info: syncing channel updates for 'stable-x86_64-apple-darwin'
info: syncing channel updates for 'nightly-x86_64-apple-darwin'
info: latest update on 2018-08-25, rust version 1.30.0-nightly (d41f21f11 2018-08-24)
warning: component 'rls-preview' is not available anymore on target 'x86_64-apple-darwin'
info: downloading component 'rustc'
 58.8 MiB /  58.8 MiB (100 %)   1.7 MiB/s ETA:   0 s
info: downloading component 'rust-std'
 46.4 MiB /  46.4 MiB (100 %)   2.2 MiB/s ETA:   0 s
info: downloading component 'cargo'
  3.3 MiB /   3.3 MiB (100 %)   1.9 MiB/s ETA:   0 s
info: downloading component 'rust-docs'
  8.3 MiB /   8.3 MiB (100 %)   1.9 MiB/s ETA:   0 s
info: downloading component 'rust-std' for 'wasm32-unknown-emscripten'
 13.1 MiB /  13.1 MiB (100 %)   2.0 MiB/s ETA:   0 s
info: downloading component 'rust-std' for 'wasm32-unknown-unknown'
 10.6 MiB /  10.6 MiB (100 %)   2.1 MiB/s ETA:   0 s
info: downloading component 'rust-src'
  3.2 MiB /   3.2 MiB (100 %)   1.7 MiB/s ETA:   0 s
info: downloading component 'rust-analysis'
info: downloading component 'rustfmt-preview'
  1.7 MiB /   1.7 MiB (100 %)   1.3 MiB/s ETA:   0 s
info: removing component 'rustc'
info: removing component 'rust-std'
info: removing component 'cargo'
info: removing component 'rust-docs'
info: removing component 'rust-std' for 'wasm32-unknown-emscripten'
info: removing component 'rust-std' for 'wasm32-unknown-unknown'
info: removing component 'rust-src'
info: removing component 'rust-analysis'
info: removing component 'rustfmt-preview'
info: removing component 'rls-preview'
info: installing component 'rustc'
info: installing component 'rust-std'
info: installing component 'cargo'
info: installing component 'rust-docs'
info: installing component 'rust-std' for 'wasm32-unknown-emscripten'
info: installing component 'rust-std' for 'wasm32-unknown-unknown'
info: installing component 'rust-src'
info: installing component 'rust-analysis'
info: installing component 'rustfmt-preview'
info: checking for self-updates

  stable-x86_64-apple-darwin unchanged - rustc 1.28.0 (9634041f0 2018-07-30)
   nightly-x86_64-apple-darwin updated - rustc 1.30.0-nightly (d41f21f11 2018-08-24)

$ rls
error: toolchain 'nightly-x86_64-apple-darwin' does not have the binary `rls`

[kennytm]

Until rust-lang/rustup#1486 is fixed, we should probably revert rust-lang/rust-central-station#79 after RLS is available again.

We could wait for another breakage and see if rust-lang/rust#53715 fixed it.

[nrc]

I don't know this is true...

It should be now

Until rust-lang/rustup#1486 is fixed

It should be fixed now

[pietroalbini]

Everything about this issue should now be fixed!