Add support from installing toolchains from try builds. #83
Conversation
| [ | ||
| ("rustc", "rustc-nightly-x86_64-unknown-linux-gnu.tar.gz"), | ||
| ("rust-std-x86_64-unknown-linux-gnu", "rust-std-nightly-x86_64-unknown-linux-gnu.tar.gz"), | ||
| ("cargo", "cargo-nightly-x86_64-unknown-linux-gnu.tar.gz"), |
There was a problem hiding this comment.
Using the xz tarballs which I believe rustup also supports will probably be a little faster, though I doubt it matters at cargobomb's current length of time and quantity of downloads.
There was a problem hiding this comment.
I've pinned rustup to 1.3.0 which doesn't support xz yet. I've filed #85 to upgrade once there is a new release.
tomprince
force-pushed
the
custom-toolchain
branch
from
fac25a3 to
c0ed0af
Compare
| tx = dist::component::TarGzPackage::new(response, &cfg)? | ||
| .install(&target, component, None, tx)?; | ||
| } | ||
| tx.commit(); |
There was a problem hiding this comment.
This is doing some weird stuff with the rustup internal metadata, that I'm not sure how I feel about. It looks like it is creating a toolchain named after the sha by commandeering the rustup APIs. The result is not something rustup can do itself, so it's basically creating a corrupted rustup installation, and ... maybe it works.
The way I expected this to work, and that I think would be easier, is to download the 'combined' installer, that contains both rustc and cargo, untar it somewhere in the cargobomb directory structure, then call rustup toolchain link on it.
I am worried that this is creating some technical debt.
There was a problem hiding this comment.
It looks like there are a number of code paths (apparently dead, though) that do installation about like I did. It also doesn't appear to care whether the toolchain directory is symlink, but just its name.
| tx = dist::component::TarGzPackage::new(response, &cfg)? | ||
| .install(&target, component, None, tx)?; | ||
| } | ||
| tx.commit(); |
There was a problem hiding this comment.
Oh, untarring is not sufficient, one also has to run install.sh to put it into the proper form.
| tx = dist::component::TarGzPackage::new(response, &cfg)? | ||
| .install(&target, component, None, tx)?; | ||
| } | ||
| tx.commit(); |
There was a problem hiding this comment.
Heh, this could actually do exactly what it's doing to install the toolchain to some non-rustup location (instead of calling install.sh), then call 'toolchain link' to turn it into a rustup toolchain.
| [ | ||
| ("rustc", "rustc-nightly-x86_64-unknown-linux-gnu.tar.gz"), | ||
| ("rust-std-x86_64-unknown-linux-gnu", "rust-std-nightly-x86_64-unknown-linux-gnu.tar.gz"), | ||
| ("cargo", "cargo-nightly-x86_64-unknown-linux-gnu.tar.gz"), |
There was a problem hiding this comment.
The single rust-nightly tarball would also work here as it contains all these components. Not sure if one approach is better.
| tx = dist::component::TarGzPackage::new(response, &cfg)? | ||
| .install(&target, component, None, tx)?; | ||
| } | ||
| tx.commit(); |
There was a problem hiding this comment.
I can't think of anything that will obviously not work with this formulation, as long as deleting these toolchains works correctly, but it does seem subject to breakage if rustup changes. I think it would be slightly more correct to install these toolchains to a cargobomb directory, not a rustup directory, and link them into rustup.
There was a problem hiding this comment.
I think that it would be better to teach rustup about this kind of toolchain (as suggested in rust-lang/rustup#1099).
tomprince
force-pushed
the
custom-toolchain
branch
from
c0ed0af to
c85640f
Compare
tomprince
force-pushed
the
custom-toolchain
branch
from
d523a14 to
b193211
Compare
|
I'm going to land this, as it has been used in production. |
|
(Although a post-merge review wouldn't be amiss.) |
This is initial implementation of #64.