Project: simplification

nrc commented 5 years ago

We could simplify Rustup a lot. It was built with some very ambitious goals and it is clear that some of things will never happen, but the code is designed to accommodate them causing unnecessary complexity. It is also backwards compatible with very old versions of Rust and Rustup. There is lots of opportunity to reduce complexity. Some ideas:

remove telemetry (this has never been used and is not that useful in any case)
remove support for v1 manifests (we should still detect and error out with a message to delete things manually or whatever)
support fewer ways to install Rust
use standard logging rather than notifications
merge some of the crates (probably only rustup and rustup-dist)

cc @rust-lang/cargo for any suggestions of things we should/should not do

### Tasks
- [ ] https://github.com/rust-lang/rustup/issues/3790

dwijnand commented 5 years ago

I'd be happy to assist here. But I've never hacked on Rustup so I'd need a little babysitting.

h-michael commented 5 years ago

I think

we may delete code for compatible with multirust
may we need reqwest backend download?
if we need only curl support, we maybe merge download crate into rustup-utils

My suggestion may be besides the point, sorry.

alexcrichton commented 5 years ago

I'd personally recommend removing all but the curl backend that @h-michael mentioned, and then I'd also look to simplify filesystem transaction handling code. IIRC it's overly complicated and can be implemented far more efficiently than it is today

kinnison commented 5 years ago

This kind of simplification sounds sensible. If you're interested in non-core contributions toward this then I'm very interested in helping.

seanmonstar commented 5 years ago

I'd actually recommend removing the curl backend and using only reqwest. Is there some reason we can't have a Rust tool use more Rust and less C?

dwijnand commented 5 years ago

@nrc @alexcrichton could you give more information regarding "support fewer ways to install Rust" and "simplify filesystem transaction handling code" please? If it's likely to generate a back-and-forth, then perhaps in its own issue.

Meanwhile I've opened an issue dedicated to the crate merging: https://github.com/rust-lang/rustup.rs/issues/1644

dwijnand commented 5 years ago

Also, it's probably good to spin out the curl vs reqwest discussion into a thread and come to a conclusion.

nrc commented 5 years ago

support fewer ways to install Rust

iirc, there are a lot of ways to install Rust via Rustup but the only ones anyone ever uses are dist and the windows installer (which is somewhat separate any way). devs do sometimes install from local disk, but I think that might be able to be simplified by separating it and handling it differently (we can certainly make breaking changes in this case).

dwijnand commented 5 years ago

rustup toolchain install <toolchain>... is the common case.

For rustc devs they also use rustup toolchain link <toolchain> <path> (as documented also in https://rust-lang.github.io/rustc-guide/how-to-build-and-run.html#creating-a-rustup-toolchain), and I think we should keep.

Other than that rustup help toolchain does say:

but rustup can also install toolchains from the official archives, for alternate host platforms, and from local builds.

But I don't exactly see how or where that happens in the code (I'm looking at rustup_mode::update).

das-sein commented 5 years ago

@h-michael @alexcrichton @seanmonstar Created a PR for the removal of the cURL backend. Figure discussion could happen there with respect to @dwijnand 's request for a place to hold one.

SimonSapin commented 5 years ago

remove support for v1 manifests

Were v2 manifests generated for older releases? I think we shouldn’t break the ability for future rustup.rs to install, say, Rust 1.2.0. (I’ve used this to find old regressions.)

petrochenkov commented 5 years ago

cc @brson

brson commented 5 years ago

Were v2 manifests generated for older releases? I think we shouldn’t break the ability for future rustup.rs to install, say, Rust 1.2.0. (I’ve used this to find old regressions.)

@SimonSapin afaik, no

I'm in agreement with everything in the op, except for possibly removing v1 support.

The multi-backend support was a pet project just because I wanted to see if I could support all the HTTP/HTTPS implementations, particularly to support production usage of rustls. It's fine to strip down to just reqwest, assuming nobody is using the curl backend - I know at some point somebody needed to explicitly use the curl backend because reqwest didn't work. It's probably reasonable to remove it and see if anybody complains (though if they do complain there's no recovery - they'll need to downgrade until the subsequent release that re-adds curl).

multirust and very-old rustup compat code can probably be deleted at this kind, though if possible, emitting an error would prevent rustup from doing something destructive in the case those scenarios do come up.

Re multiple ways for rustup to install rust, the only ways I recall are from the xz-tarballs and gz-tarballs (even on windows). I don't recall rustup having windows installer support, though there is a rustup-win-installer subdirectory that I assume still doesn't do anything useful. That can be deleted.

The gz support can't be removed as long as rustup supports pre-xz toolchains.

I don't have any insight into whether people use rustup toolchain link or not, though I do know I was pretty close to using it the other day for testing custom rust toolchains, but got distracted before I did.

The notification system should be dropped. I agree it's unnecessarily complex.

brson commented 5 years ago

Telemetry is fine to remove, though I still think having telemetry somewhere is a good idea.

brson commented 5 years ago

I left a comment on the curl-removal PR about the prudence of both making reqwest the default and removing curl in the same release.

dwijnand commented 5 years ago

very-old rustup compat code

If you could give some more specifics or pointers that would be great.

though there is a rustup-win-installer subdirectory that I assume still doesn't do anything useful. That can be deleted

Oh!

Thanks, @brson!

mati865 commented 5 years ago

I don't have any insight into whether people use rustup toolchain link or not, though I do know I was pretty close to using it the other day for testing custom rust toolchains, but got distracted before I did.

It's convenient way to use Rust packaged by Linux distributions. Not like I'm using it often but it comes handy.

joshtriplett commented 5 years ago

I'm using rustup toolchain link system /usr for that exact purpose.

yodaldevoid commented 5 years ago

rustup toolchain link is also very useful for rustc development.

kinnison commented 5 years ago

So I think rustup toolchain link is important to keep; certainly I wouldn't advocate removing it.

I do wonder if there might be something even more nice we could do for supporting system-managed toolchains (e.g. converting components, targets, etc, into package names and reporting useful commands such as apt install rust-rls or somesuch should a proxy be run which isn't installed for the system toolchain.

Of course, this doesn't quite fall under the topic of simplification, so I'll shut up now :D

Xanewok commented 5 years ago

Agreed on rustup toolchain link; I'd even go as far and possibly better integrate the local workflow.

I know I wanted to rustup component add XYZ in the past. This could possibly run ./x.py dist XYZ under the hood or we could change the way these components are built inside Rust repo so that rustup can understand the resulting artifacts and be able to use them for rustup component add purposes.

pickfire commented 5 years ago

~~Can there be +nightly in rustup to keep it similar to cargo rather than using --toolchain=nightly?~~

EDIT: https://github.com/rust-lang/rustup.rs/issues/1570

rust-lang / rustup

Project: simplification #1611