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

'llvm-tools-preview' component is now named 'llvm-tools' #3578

Merged
merged 1 commit into from
Jan 28, 2024

Conversation

mbrossard
Copy link
Contributor

The llvm-tools-preview component is now name llvm-tools. It seems this change happened in version 1.67 since rustup +1.67 component add llvm-tools succeeds but rustup +1.66 component add llvm-tools fails.

@ehuss
Copy link
Contributor

ehuss commented Dec 12, 2023

I'm a little uncertain if this is the right change to make. The llvm-tools component is not considered stable, and should keep the preview tag. I think it is actually a bug that it works without the -preview part. I can't remember or find the history of why -preview gets ignored (I think it has something to do with how renames were implemented). It looks like #1728 tried to at least fix it so that miri appeared as -preview, but even that seemed to not work.

@mbrossard
Copy link
Contributor Author

It seems https://static.rust-lang.org/dist/2023-01-26/channel-rust-1.67.toml contains:

[renames.llvm-tools]
to = "llvm-tools-preview"

Whereas https://static.rust-lang.org/dist/2022-12-15/channel-rust-1.66.toml did not have it.

It looks related to rust-lang/rust@a3dd94e
(or related changes in https://github.com/rust-lang/rust/commits/master/src/tools/build-manifest/src/main.rs around Oct 27th 2022).

@rbtcollins
Copy link
Contributor

This is the correct change as of 1.67. You can see in the manifest - https://static.rust-lang.org/dist/channel-rust-stable.toml - that llvm-tools is renamed to llvm-tools-preview: so the package real name is unaltered, but users should now enter llvm-tools.

However, rustup is not connected to that versioning scheme, but its a problem that becomes rarer over time.

@rbtcollins rbtcollins added this pull request to the merge queue Jan 28, 2024
Merged via the queue into rust-lang:master with commit f0d76d0 Jan 28, 2024
14 of 16 checks passed
@smoelius
Copy link
Contributor

This is the correct change as of 1.67. You can see in the manifest - https://static.rust-lang.org/dist/channel-rust-stable.toml - that llvm-tools is renamed to llvm-tools-preview: so the package real name is unaltered, but users should now enter llvm-tools.

@ehuss Is this your conclusion as well? I had the impression from rust-lang/rust#85658 (comment) that you had the opposite conslusion.

@rami3l rami3l mentioned this pull request Jan 29, 2024
1 task
@ehuss
Copy link
Contributor

ehuss commented Jan 29, 2024

Correct, there hasn't been a decision on this, yet, so I think it may be premature to merge this.

Although, the cat is out of the bag, so I'm not sure if we can put it back. I'll start a topic with the release team and see if they have any thoughts.

@smoelius
Copy link
Contributor

@ehuss Please forgive me for bothering you, but were you able to determine whether llvm-tools is now the official term, as opposed to llvm-tools-preview?

@ehuss
Copy link
Contributor

ehuss commented Mar 12, 2024

The discussion was at https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/llvm-tools.20stabilization.

I think the consensus is that we shouldn't remove llvm-tools and break people using it. However, it probably should not be considered stabilized. There are no guarantees about which tools are included, no documentation about which tools are included, where they are located (and some thinking that they are currently in the wrong directory), or how to find them.

I'll copy one of the last comments here:

I agree that simply saying "llvm-tools may break at any time" is uncomfortable, but I think the fairest statement is that "llvm-tools are provided as-is from upstream LLVM". In most cases, they aren't going to break all the time or even occasionally. At the same time, we're not going to take on maintaining a tool LLVM has dropped just to fulfil a stability promise. I think the vast majority of users would totally understand this position and not have an issue with that.

This feels very similar to the Target Tier Policy where we do not promise perpetual support for any particular target, just a level of support in a given release. Breakage in upstream LLVM has even necessitated demoting targets from Tier 2 to Tier 3 because they're simply too broken to be at that level.

I think further tracking of stabilization would be at rust-lang/rust#85658. I posted #3707 to add a note indicating the current status.

@smoelius
Copy link
Contributor

Thank you very much for the detailed response, which was far more than I'd hoped for!

gdamjan added a commit to gdamjan/rust-embedded-book that referenced this pull request May 17, 2024
gdamjan added a commit to gdamjan/rust-embedded-discovery that referenced this pull request May 17, 2024
gdamjan added a commit to gdamjan/rust-embedded.github.io that referenced this pull request May 17, 2024
gdamjan added a commit to gdamjan/rust-embedded-embedonomicon that referenced this pull request May 18, 2024
rust-lang/rustup#3578

updated `.github/workflows/ci.yaml` to use rust 1.78 instead of 1.62,
since the llvm-tools change happened in 1.67. also updated ubuntu 20.04
to 22.04
gdamjan added a commit to gdamjan/rust-embedded-embedonomicon that referenced this pull request May 18, 2024
rust-lang/rustup#3578

updated `.github/workflows/ci.yaml` to use rust 1.78 instead of 1.62,
since the llvm-tools change happened in 1.67.
also updated ci ubuntu 20.04 to 22.04
gdamjan added a commit to gdamjan/rust-embedded-embedonomicon that referenced this pull request May 18, 2024
victor-prokhorov pushed a commit to victor-prokhorov/discovery that referenced this pull request Jul 8, 2024
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.

4 participants