Use rustup.rs instead of custom bootstrap #19395

SimonSapin · 2017-11-27T17:58:45Z

Fixes #11361, closes #18874, fixes #19365.

This change is

highfive · 2017-11-27T17:58:49Z

Heads up! This PR modifies the following files:

@wafflespeanut: python/servo/devenv_commands.py, python/servo/post_build_commands.py, python/servo/command_base.py, python/servo/testing_commands.py, python/servo/util.py and 2 more
@rillian: rust-stable-version

SimonSapin · 2017-11-27T18:04:10Z

Before this can land:

A new version of rustup that includes rust-lang/rustup#1295 needs to be released (rust-lang/rustup#1275 (comment))
That version needs to be install on CI builders

~~At first, contributors will probably have an older version of rustup and see something like this:~~

$ ./mach build error: Found argument '--install' which wasn't expected, or isn't valid in this context USAGE: rustup run <toolchain> <command>... For more information try --help Build FAILED in 0:00:00

This is pretty bad since --help in the output refers to rustup run --help, not ./mach build --help. And even rustup run --help wouldn’t be helpful since it doesn’t know about options added in later versions.

Mach cannot easily distinguish this case from a "normal" build failure, so the only way to detect this situation and print a better diagnostic we would need to run rustup --version and parse its output every time we’re about to run rustup run. This seems overkill, but perhaps I shouldn’t worry about that.

r? @jdm

SimonSapin · 2017-11-27T22:53:47Z

subprocess.check_output(["rustup", "--version"]) takes ~3ms on my laptop, so really not worth worrying about compared to existing overhead of Mach. I’ve added the detection mentioned above.

SimonSapin · 2017-11-27T23:10:48Z

With this PR running cargo build --manifest-path ports/servo/Cargo.toml will use the same compiler as ./mach build. However going back and forth between those will still recompile everything because mach sets the RUSTFLAGS environment variable.

To not have RUSTFLAGS by default we’d need rustc to revert back to using ld.gold by default: rust-lang/rust#29974, and us to replace -W unused-extern-crates with #![warn(unused_extern_crate)] in each crate.

jdm · 2017-11-27T23:43:26Z

Our Appvey and TravisCI are missing rustup; test-tidy is failing; and test-wpt is broken because ensure_bootstrapped was removed.

SimonSapin · 2017-11-28T08:16:33Z

Thanks. I pushed something to try to fix these immediate issues, but as I mentioned we’re still waiting on a new rustup release.

jdm · 2017-11-28T14:24:58Z

python/servo/bootstrap_commands.py

+                        ['git', 'show', '%s:%s' % (commit_hash, toolchain_file)])
+                    old_toolchains.append(toolchain.strip())
+
+        remove_anything = False


This doesn't match the remove_anything variable later.

This as reported by tidy and fixed.

jdm · 2017-11-28T14:24:58Z

appveyor.yml

+  - rustup-init.exe -y --default-host x86_64-pc-windows-msvc --default-toolchain none
+  - set PATH=%PATH%;C:\Users\appveyor\.cargo\bin
+  - rustc -V
+  - cargo -V


Should we modify the cache instructions above this block?

It looks like this caches the .servo directory including the extracted rustc and cargo downloaded by mach. Do we want to cache toolchains downloaded by rustup?

Yes, since it makes builds a bit faster.

Alright, done.

bors-servo · 2017-11-28T14:36:57Z

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

bors-servo · 2017-12-09T08:14:55Z

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

SimonSapin · 2017-12-09T08:58:22Z

Rustup release is blocked on rust-lang/rustup#1305

SimonSapin · 2017-12-12T21:23:11Z

Added a commit to unblock by not using the new rustup feature.

@bors-servo try

SimonSapin · 2018-01-10T14:32:14Z

@bors-servo try

bors-servo · 2018-01-10T14:39:47Z

⌛ Trying commit 272fef3 with merge f26eb55...


        Auto merge of #19395 - servo:rustup.rs, r=<try>

Use rustup.rs instead of custom bootstrap Fixes #11361, closes #18874, fixes #19365.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19395)

bors-servo · 2018-01-10T16:42:02Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True


        Revert "Use rustdoc --document-private-item instead of deprecated flags"

This reverts commit d96f0ff.


        Use rustup.rs instead of custom bootstrap

Fixes #11361, closes #18874


        Add rustup.rs instructions in README


        Print a link to instructions if rustup is not found.


        Print a better error message when rustup is too old


        Cache rustup toolchains on AppVeyor and Travis-CI


        Fix "rustup is not installed" message


        Use rustup "proxies" instead of `rustup run`

To make sure we’re indeed running rustup’s proxy rather than some other `cargo` for example, run the `rustup` executable with a different `argv[0]`.


        Revert "Use rustup "proxies" instead of `rustup run`"

This reverts commit ad22368.


        rustup.rs: Use MSVC rather than GNU toolchains on Windows

SimonSapin · 2018-01-10T17:06:29Z

@bors-servo r=jdm

bors-servo · 2018-01-10T17:06:30Z

📌 Commit 0681e68 has been approved by jdm

bors-servo · 2018-01-10T18:01:00Z

⌛ Testing commit 0681e68 with merge 2be49ac...


        Auto merge of #19395 - servo:rustup.rs, r=jdm

Use rustup.rs instead of custom bootstrap Fixes #11361, closes #18874, fixes #19365.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19395)

bors-servo · 2018-01-10T19:05:13Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm
Pushing 2be49ac to master...


        Re-enable selectors doctests

Closes #16710, which was actually fixed by 6dff251 in #19395. This exercises that fix.


        Re-enable selectors doctests

Closes #16710, which was actually fixed by 6dff251 in #19395. This exercises that fix. Green AppVeyor build: https://ci.appveyor.com/project/servo/servo/build/1.0.24124

highfive assigned glennw Nov 27, 2017

highfive added the S-awaiting-review label Nov 27, 2017

This was referenced Nov 27, 2017

New release rust-lang/rustup#1275

Closed

Use rustup to get nightly rust #11361

Closed

rillian mentioned this pull request Nov 27, 2017

Add geckolib-rust-toolchain to rillian's watch list. servo/highfive#197

Merged

jdm assigned jdm and unassigned glennw Nov 27, 2017

SimonSapin force-pushed the rustup.rs branch 2 times, most recently from 99377c9 to ae1e125 Nov 28, 2017

SimonSapin mentioned this pull request Nov 28, 2017

Install rustup.rs on builders servo/saltfs#757

Closed

jdm requested changes Nov 28, 2017

View changes

jdm added S-needs-code-changes S-blocked-on-external and removed S-awaiting-review labels Nov 28, 2017

highfive added the S-needs-rebase label Nov 28, 2017

SimonSapin force-pushed the rustup.rs branch from ae1e125 to ce3a140 Nov 28, 2017

highfive added S-awaiting-review and removed S-needs-code-changes labels Nov 28, 2017

highfive added the S-needs-rebase label Dec 9, 2017

SimonSapin force-pushed the rustup.rs branch from 282562c to 034e834 Dec 9, 2017

jdm approved these changes Jan 10, 2018

View changes

SimonSapin added 10 commits Jan 9, 2018

Revert "Use rustdoc --document-private-item instead of deprecated flags"

6dff251

This reverts commit d96f0ff.

Use rustup.rs instead of custom bootstrap

365a139

Fixes #11361, closes #18874

Add rustup.rs instructions in README

f2c398f

Print a link to instructions if rustup is not found.

2f941d8

Print a better error message when rustup is too old

a3339f9

Cache rustup toolchains on AppVeyor and Travis-CI

d0b3525

Fix "rustup is not installed" message

e0f8f09

Use rustup "proxies" instead of `rustup run`

2618e4e

To make sure we’re indeed running rustup’s proxy rather than some other `cargo` for example, run the `rustup` executable with a different `argv[0]`.

Revert "Use rustup "proxies" instead of `rustup run`"

ded0bc9

This reverts commit ad22368.

rustup.rs: Use MSVC rather than GNU toolchains on Windows

Loading status checks…

0681e68

SimonSapin force-pushed the rustup.rs branch from 272fef3 to 0681e68 Jan 10, 2018

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 10, 2018

highfive removed the S-awaiting-merge label Jan 10, 2018

SimonSapin deleted the rustup.rs branch Jan 10, 2018

SimonSapin mentioned this pull request Jan 10, 2018

[WIP] Use rustup to manage toolchains #17927

Closed

1 of 5 tasks complete

SimonSapin added a commit that referenced this pull request Jan 12, 2018

Re-enable selectors doctests

Loading status checks…

d86f533

Closes #16710, which was actually fixed by 6dff251 in #19395. This exercises that fix.

SimonSapin added a commit that referenced this pull request Jan 12, 2018

Re-enable selectors doctests

0929f72

Closes #16710, which was actually fixed by 6dff251 in #19395. This exercises that fix. Green AppVeyor build: https://ci.appveyor.com/project/servo/servo/build/1.0.24124

SimonSapin mentioned this pull request Jan 31, 2018

Stop setting $CARGO_HOME to its default value servo/saltfs#789

Closed

servo / servo

Use rustup.rs instead of custom bootstrap #19395

Use rustup.rs instead of custom bootstrap #19395

SimonSapin commented Nov 27, 2017 •

edited

highfive commented Nov 27, 2017

SimonSapin commented Nov 27, 2017 •

edited

SimonSapin commented Nov 27, 2017

SimonSapin commented Nov 27, 2017

jdm commented Nov 27, 2017

SimonSapin commented Nov 28, 2017 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors-servo commented Nov 28, 2017

bors-servo commented Dec 9, 2017

SimonSapin commented Dec 9, 2017

SimonSapin commented Dec 12, 2017

SimonSapin commented Jan 10, 2018

bors-servo commented Jan 10, 2018

bors-servo commented Jan 10, 2018

SimonSapin commented Jan 10, 2018

bors-servo commented Jan 10, 2018

bors-servo commented Jan 10, 2018

bors-servo commented Jan 10, 2018

servo / servo

Join GitHub today

Use rustup.rs instead of custom bootstrap #19395

Use rustup.rs instead of custom bootstrap #19395

Conversation

SimonSapin commented Nov 27, 2017 • edited

highfive commented Nov 27, 2017

SimonSapin commented Nov 27, 2017 • edited

SimonSapin commented Nov 27, 2017

SimonSapin commented Nov 27, 2017

jdm commented Nov 27, 2017

SimonSapin commented Nov 28, 2017 • edited

This comment has been minimized.

jdm Nov 28, 2017

This comment has been minimized.

SimonSapin Nov 28, 2017

This comment has been minimized.

jdm Nov 28, 2017

This comment has been minimized.

SimonSapin Nov 28, 2017

This comment has been minimized.

jdm Nov 28, 2017

This comment has been minimized.

SimonSapin Nov 28, 2017

bors-servo commented Nov 28, 2017

bors-servo commented Dec 9, 2017

SimonSapin commented Dec 9, 2017

SimonSapin commented Dec 12, 2017

SimonSapin commented Jan 10, 2018

bors-servo commented Jan 10, 2018

bors-servo commented Jan 10, 2018

SimonSapin commented Jan 10, 2018

bors-servo commented Jan 10, 2018

bors-servo commented Jan 10, 2018

bors-servo commented Jan 10, 2018

SimonSapin commented Nov 27, 2017 •

edited

SimonSapin commented Nov 27, 2017 •

edited

SimonSapin commented Nov 28, 2017 •

edited