Make multirust-rs work with both v1 and v2 manifests #69
Conversation
This ensures than any mentioned component actually has a package to download. Just a defensive measure against backend mistakes.
This wires the cli to the new installer code for both v1 and v2 distribution manifests. v1 installations are upgraded to v2 automatically. I copied test cases from multirust.sh into tests/cli-v1.rs, tests/cli-v2.rs and tests/cli-misc.rs. Many of them unrelated to getting installation from the distribution server working are still unported and marked #[ignore]. There is a lot of setup necessary for these tests and they are quite slow. I've been testing with --release. The metadata format is sloppy right now. * rustlib/rust-installer-version - Still the same as rust-installer * rustlib/components - Components has its own component list that is the same as rust-installer, and I think needed for v1 manifest compatibility * rustlib/manifest-$component - The component manifest, per rust-installer * rustlib/multirust-config.toml - The multirust distribution configuration, which currently contains its own component/target list per the v2 manifests, and is versioned. * rustlib/multirust-channel-manifest.toml - A stored copy of the distribution manifest.
The format of the installed toolchains has changed. The big jump is to leave room for multirust.sh to leave room for multirust.sh to make its own diverging versions.
|
\o/ |
| let _g = LOCK.lock(); | ||
| let cwd = env::current_dir().unwrap(); | ||
| env::set_current_dir(path).unwrap(); | ||
| let _g = scopeguard::guard((), move |_| env::set_current_dir(&cwd).unwrap()); |
There was a problem hiding this comment.
I'd personally feel that defining a shim here with a Drop implementation would be easier than pulling in a whole crate to do this job, but either way's probably fine.
If this function stays it may also be best to return an RAII guard instead of taking a closure (helps indentation, etc)
There was a problem hiding this comment.
I don't think turning this into a guard will help readability - the indentation actually is good for readability because it indicates that within a particular block the directory is different. Without the extra block it's less obvious the environment has changed.
|
Nice! |
The not-yet-live v2 manifests aren't enabled without this on. This will let us merge back into master without worry that multirust will be broken when they do go live.
Like `multirust default 1.0.0`.
| return Err(Error::ObsoleteDistManifest); | ||
| } | ||
|
|
||
| let url = new_manifest.iter().find(|u| u.contains(&self.target_triple)); |
There was a problem hiding this comment.
We should be specifically looking for target_triple + ".tar.gz" here, otherwise it will just find whichever file happens to be listed first for that target.
There was a problem hiding this comment.
(This is what's causing the failures on appveyor)
|
Great! If you could just fix the two things causing the CI failures then I'm happy to merge this in - you can treat my other comments as just TODOs before this all gets merged back to master. |
|
I've added a few more commits: the The only thing I didn't do was change |
|
Thanks for the reviews. |
|
Travis is still failing, but I'll merge this in and try to sort it out on |
Make multirust-rs work with both v1 and v2 manifests
This wires the cli to the new installer code for both v1 and v2
distribution manifests. v1 installations are upgraded to v2
automatically.
I copied test cases from multirust.sh into tests/cli-v1.rs,
tests/cli-v2.rs and tests/cli-misc.rs. Many of them unrelated to
getting installation from the distribution server working are still
unported and marked #[ignore]. There is a lot of setup necessary for
these tests and they are quite slow. I've been testing with --release.
The metadata format is sloppy right now.
the same as rust-installer, and I think needed for v1 manifest
compatibility
configuration, which currently contains its own component/target
list per the v2 manifests, and is versioned.
distribution manifest.
This does a metadata version bump, but doesn't add the upgrade code yet.
Next steps are probably:
At that point we've got feature parity with master, but with an entirely new backend.
After that
$CARGO_HOMEwith upgrade.