-
Notifications
You must be signed in to change notification settings - Fork 68
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
[WIP] Rewrite the rust-installer in Rust #62
Conversation
Awesome, thanks so much for this @cuviper! Everything looks quite solid to me. I really like how we've still got a test suite in this repository, and I like the strategy here as well. I think we'll want to keep the test suite no matter what, and it's not necessarily critical to rewrite it just yet, I was mostly interested in the tarball-creation logic which just tends to be easier if it's cross-platform. Note that we probably don't have to worry about updating the Cargo repository's rust-installer submodule, I've been meaning to phase that out now that it's all included in rust-lang/rust. In that sense I think the next steps would be:
Does that sounds reasonable? Or did you have different plans in mind perhaps? |
On the distro side, we still build cargo from its own source rpm, so I would at least hope that "make install" continues to work -- but that definitely doesn't need to use a full rust-installer. This is a tangent though, so if there's more to discuss here, let's spin off a cargo issue.
I had thought to just make And FWIW, a basic tool already exists in the new |
Ah my only thinking here is that this'd mean the libs would be a dependency of rustbuild itself, and rustbuild is the gate into compiling everything in the rust-lang/rust repo. If we add all the deps (semi-weighty here) it may make rust compilations more difficult to get working on all platforms all the time. Basically I'd imagine that less-than-100% (maybe not even half?) of builds in rust-lang/rust don't actually end up using I think, however, it's always possible to add more deps to rustbuild, it'll just be a long road uphill unfortunately.
Nah it seems fine to me! Either a custom tool or the one here should work great. |
Ah, that makes total sense. External tool it is! |
r? @brson (would like to review before merging) |
lgtm The remove_dir_all module should probably be replaced with the remove_dir_all crate. |
I saw the |
…richton Update to the Rusty rust-installer This updates the rust-installer submodule to the new version written in Rust (rust-lang/rust-installer#62), now moved to `src/tools/rust-installer` and invoked in `dist.rs` as a cargo-based tool command. All of the former shell-script invocations now invoke the tool, otherwise keeping the same arguments as before. As a small bonus, `rustc-src` now also uses the same tarball generator, so it gains a smaller `.tar.xz` too. Fixes rust-lang#41569. r? @alexcrichton
…richton Update to the Rusty rust-installer This updates the rust-installer submodule to the new version written in Rust (rust-lang/rust-installer#62), now moved to `src/tools/rust-installer` and invoked in `dist.rs` as a cargo-based tool command. All of the former shell-script invocations now invoke the tool, otherwise keeping the same arguments as before. As a small bonus, `rustc-src` now also uses the same tarball generator, so it gains a smaller `.tar.xz` too. Fixes rust-lang#41569. r? @alexcrichton
…richton Update to the Rusty rust-installer This updates the rust-installer submodule to the new version written in Rust (rust-lang/rust-installer#62), now moved to `src/tools/rust-installer` and invoked in `dist.rs` as a cargo-based tool command. All of the former shell-script invocations now invoke the tool, otherwise keeping the same arguments as before. As a small bonus, `rustc-src` now also uses the same tarball generator, so it gains a smaller `.tar.xz` too. Fixes rust-lang#41569. r? @alexcrichton
…richton Update to the Rusty rust-installer This updates the rust-installer submodule to the new version written in Rust (rust-lang/rust-installer#62), now moved to `src/tools/rust-installer` and invoked in `dist.rs` as a cargo-based tool command. All of the former shell-script invocations now invoke the tool, otherwise keeping the same arguments as before. As a small bonus, `rustc-src` now also uses the same tarball generator, so it gains a smaller `.tar.xz` too. Fixes rust-lang#41569. r? @alexcrichton
Update to the Rusty rust-installer This updates the rust-installer submodule to the new version written in Rust (rust-lang/rust-installer#62), now moved to `src/tools/rust-installer` and invoked in `dist.rs` as a cargo-based tool command. All of the former shell-script invocations now invoke the tool, otherwise keeping the same arguments as before. As a small bonus, `rustc-src` now also uses the same tarball generator, so it gains a smaller `.tar.xz` too. Fixes #41569. r? @alexcrichton
For rust-lang/rust#41569, this branch rewrites almost all of rust-installer in Rust. The existing shell scripts are cut down to shims invoking
cargo run
, but otherwise they still accept the exact same options as before. Only the actual install script and the testing script are still written in shell, and all of those tests do pass as-is.It's not clear to me if this will be wanted as an evolution of rust-installer, or dropped in directly to the Rust sources, but having a PR here should let people start reviewing it either way. My next step is to see if this will serve as a drop-in replacement in Rust, Cargo, etc., and then we can see about porting rustbuild
dist.rs
to use this directly as a library.