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

Tracking issue: edition manifest changes #55967

Closed
2 tasks
nrc opened this issue Nov 15, 2018 · 6 comments
Closed
2 tasks

Tracking issue: edition manifest changes #55967

nrc opened this issue Nov 15, 2018 · 6 comments
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@nrc
Copy link
Member

nrc commented Nov 15, 2018

  • - rename rls and rustfmt to not include -preview
  • - add profiles

cc @alexcrichton

@nrc nrc added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. labels Nov 15, 2018
@nrc nrc added this to the Rust 2018 Release milestone Nov 15, 2018
@alexcrichton
Copy link
Member

So nowadays I'm sort of terrified about doing anything with rustup, so I've attempted to run this through its paces to make sure that if we deploy this change whether it will actually work. Unfortunately it looks like it doesn't work?

I played around with our dev artifacts, and I was specifically applying a few changes for an updated manifest:

  • All instances of rls-preview were replaced with rls (same for rustfmt
  • This section was added to the end:
[rename.rls-preview]
to = 'rls'
# ... same for rustfmt 

If I installed a previous compiler with rls-preview then updating the compiler to this new manifest simply removed the rls-preview. Additionally rustup component add rls-preview would fail on the new manifest saying "component not found".

I think both of these aspects means there's bugs in rustup's handling of [rename] so we won't be able to do this by the time of the edition :(

@nrc
Copy link
Member Author

nrc commented Nov 18, 2018

If I installed a previous compiler with rls-preview then updating the compiler to this new manifest simply removed the rls-preview

I think you would need to have rls rather than rls-preview available to download too (I'd expect an error about this, but perhaps it is swallowed because rustup thinks it is a temporarily missing component?)

Additionally rustup component add rls-preview would fail on the new manifest saying "component not found".

I think this was never implemented

@alexcrichton
Copy link
Member

In the new manifests though the rls-preview component didn't exist and only rls existed, but rustup didn't do the transition from rls-preview to rls automatically. I thought it would though?

@nrc
Copy link
Member Author

nrc commented Nov 19, 2018

rustup didn't do the transition from rls-preview to rls automatically

I'm not exactly sure what this means.

What happens is that if you have rls-preview installed, then rustup removes the rls-preview component and adds the rls component. But I think that if the rls component is not available, then it will only be able to remove the rls-preview one without adding the new one.

@alexcrichton
Copy link
Member

@nrc the behavior I was seeing was that the rls component was available and listed in the manifest as available, but rustup didn't install the renamed component it just uninstalled the previous one. I may have tested a wrong manifest though!

alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 19, 2018
This will be part of our strategy for shipping renamed versions of these
components for the Rust 2018 edition.

Closes rust-lang#55967
@alexcrichton
Copy link
Member

Updated strategy:

  • We will add a "rename" of rustfmt to rustfmt-preview (and clippy)
  • We're updating rustup to accept rustup component add rls. It doesn't accept that today, but it's specified in the manifest [rename] section
  • We're not landing profiles before the edition

Those two will get us over the hump for the edition. The end state is that rustup component add rls will work, although it'll probably print that it's installing the "rls-preview" component. We'll fix that UI bug once rust-lang/rustup#1546 is fixed

bors added a commit that referenced this issue Nov 20, 2018
Add temporary renames to manifests for rustfmt/clippy

This will be part of our strategy for shipping renamed versions of these
components for the Rust 2018 edition.

Closes #55967
nikomatsakis pushed a commit to nikomatsakis/rust that referenced this issue Nov 20, 2018
This will be part of our strategy for shipping renamed versions of these
components for the Rust 2018 edition.

Closes rust-lang#55967
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants