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

[WIP] Use rustup to manage toolchains #17927

Closed
wants to merge 1 commit into from
Closed

Conversation

@shinglyu
Copy link
Member

shinglyu commented Aug 1, 2017

[DO NOT MERGE]
This is an early prototype of using rustup to replace our rustc/cargo download logic. I'd like to collect some feedback before I start working on other platforms.

I'd also love to hear your opinion on how to approach the following issues:

  • Windows/OSX build: any special rustup tricks?
  • Android build: Should we use rustup at all?
  • Cross compiling: What kind of use cases are there?

ping @metajack


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #11361 .
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive
Copy link

highfive commented Aug 1, 2017

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/command_base.py, python/servo/bootstrap_commands.py
@SimonSapin
Copy link
Member

SimonSapin commented Aug 1, 2017

@jdm jdm assigned metajack and unassigned wafflespeanut Aug 1, 2017
@shinglyu
Copy link
Member Author

shinglyu commented Aug 2, 2017

@SimonSapin: Thanks for the first two comments! As for the nightly vs stable issue, is geckolib the only case we use stable rust? If so would it be safe to remove (at least part) of the stable flags and use rust-toolchain to control this instead?

@shinglyu
Copy link
Member Author

shinglyu commented Aug 2, 2017

Also if not using master-alt will hurt the performance badly, I can land the rustup feature prefed-off. Because the only people who need this eagerly is the people working on embedding, so I guess pref-on rustup for them is not a big deal.

@SimonSapin
Copy link
Member

SimonSapin commented Aug 2, 2017

Yes, geckolib is the only case where we use stable Rust. "Stable Rust version" in the context of Servo really means "minimum version that Firefox wants to support". (Search for "Mozilla-central can use" in https://wiki.mozilla.org/Rust_Update_Policy_for_Firefox.)

And, indeed, LLVM assertions are already disabled in official stable channel builds, so "alt" toolchains are not relevant there.


I think it’s fine to require that rustup is available for building Servo. Eventually I’d like to entirely remove our code for downloading tarballs and setting $PATH environment variables, and leave that job to rustup. I’m not sure what’s the best way to handle alt toolchains until they’re natively supported by rustup, though.

@metajack
Copy link
Contributor

metajack commented Aug 2, 2017

@shinglyu I believe the plan is that rustup will have access to alt toolchains soon.

@shinglyu
Copy link
Member Author

shinglyu commented Aug 3, 2017

OK! I'll keep an eye on the alt-toolchains support for rustup

@paulrouget
Copy link
Contributor

paulrouget commented Aug 31, 2017

I believe the plan is that rustup will have access to alt toolchains soon.

Is there any issue/PR we can look at?

@SimonSapin
Copy link
Member

SimonSapin commented Aug 31, 2017

SimonSapin added a commit that referenced this pull request Aug 31, 2017
… this time using a `rust-toolchain` file compatible with rustup:
https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section,
and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers,
then rustup will use the correct version based on `rust-toolchain`.

CC #11361

Unlike #17927,
this does not make mach use rustup directly.
SimonSapin added a commit that referenced this pull request Aug 31, 2017
… this time using a `rust-toolchain` file compatible with rustup:
https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section,
and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers,
then rustup will use the correct version based on `rust-toolchain`.

CC #11361

Unlike #17927,
this does not make mach use rustup directly.
bors-servo added a commit that referenced this pull request Aug 31, 2017
Switch back to pinning Rust by Nightly date instead of commit hash…

… this time using a `rust-toolchain` file compatible with rustup: https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section, and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers, then rustup will use the correct version based on `rust-toolchain`.

CC #11361

Unlike #17927, this does not make mach use rustup directly. That should wait until rust-lang/rustup#1099 is fixed.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18325)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 31, 2017

The latest upstream changes (presumably #18325) made this pull request unmergeable. Please resolve the merge conflicts.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Sep 1, 2017
…tead of commit hash… (from servo:rustup-toolchain); r=nox

… this time using a `rust-toolchain` file compatible with rustup: https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section, and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers, then rustup will use the correct version based on `rust-toolchain`.

CC servo/servo#11361

Unlike servo/servo#17927, this does not make mach use rustup directly. That should wait until rust-lang/rustup#1099 is fixed.

Source-Repo: https://github.com/servo/servo
Source-Revision: c4800a6c83e6fdabaf7c4eff70a24487d16f18ff

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : e3d41159bc4670ea990630e5342fe9b663a79fe5
xeonchen pushed a commit to xeonchen/gecko-cinnabar that referenced this pull request Sep 1, 2017
…tead of commit hash… (from servo:rustup-toolchain); r=nox

… this time using a `rust-toolchain` file compatible with rustup: https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section, and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers, then rustup will use the correct version based on `rust-toolchain`.

CC servo/servo#11361

Unlike servo/servo#17927, this does not make mach use rustup directly. That should wait until rust-lang/rustup#1099 is fixed.

Source-Repo: https://github.com/servo/servo
Source-Revision: c4800a6c83e6fdabaf7c4eff70a24487d16f18ff
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this pull request Sep 2, 2017
…tead of commit hash… (from servo:rustup-toolchain); r=nox

… this time using a `rust-toolchain` file compatible with rustup: https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section, and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers, then rustup will use the correct version based on `rust-toolchain`.

CC servo/servo#11361

Unlike servo/servo#17927, this does not make mach use rustup directly. That should wait until rust-lang/rustup#1099 is fixed.

Source-Repo: https://github.com/servo/servo
Source-Revision: c4800a6c83e6fdabaf7c4eff70a24487d16f18ff
@SimonSapin
Copy link
Member

SimonSapin commented Jan 10, 2018

Done in #19395.

@SimonSapin SimonSapin closed this Jan 10, 2018
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
…tead of commit hash… (from servo:rustup-toolchain); r=nox

… this time using a `rust-toolchain` file compatible with rustup: https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section, and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers, then rustup will use the correct version based on `rust-toolchain`.

CC servo/servo#11361

Unlike servo/servo#17927, this does not make mach use rustup directly. That should wait until rust-lang/rustup#1099 is fixed.

Source-Repo: https://github.com/servo/servo
Source-Revision: c4800a6c83e6fdabaf7c4eff70a24487d16f18ff

UltraBlame original commit: 5df6f8fe6c50631fe00f47b69d16b8d555d90503
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
…tead of commit hash… (from servo:rustup-toolchain); r=nox

… this time using a `rust-toolchain` file compatible with rustup: https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section, and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers, then rustup will use the correct version based on `rust-toolchain`.

CC servo/servo#11361

Unlike servo/servo#17927, this does not make mach use rustup directly. That should wait until rust-lang/rustup#1099 is fixed.

Source-Repo: https://github.com/servo/servo
Source-Revision: c4800a6c83e6fdabaf7c4eff70a24487d16f18ff

UltraBlame original commit: 5df6f8fe6c50631fe00f47b69d16b8d555d90503
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
…tead of commit hash… (from servo:rustup-toolchain); r=nox

… this time using a `rust-toolchain` file compatible with rustup: https://github.com/rust-lang-nursery/rustup.rs/#the-toolchain-file

And upgrade to rustc 1.21.0-nightly (c11f689d2 2017-08-29)

----

Now if both `system-rust` and `system-cargo` are set to `true` in `.servobuild`’s `[tools]` section, and the corresponding `rustc` and `cargo` binaries are in fact rustup’s wrappers, then rustup will use the correct version based on `rust-toolchain`.

CC servo/servo#11361

Unlike servo/servo#17927, this does not make mach use rustup directly. That should wait until rust-lang/rustup#1099 is fixed.

Source-Repo: https://github.com/servo/servo
Source-Revision: c4800a6c83e6fdabaf7c4eff70a24487d16f18ff

UltraBlame original commit: 5df6f8fe6c50631fe00f47b69d16b8d555d90503
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
You can’t perform that action at this time.