Skip to content

ci: Attempt to skip a full rustc compile on dist* #61212

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

Merged
merged 4 commits into from
May 30, 2019

Conversation

alexcrichton
Copy link
Member

Currently when we're preparing cross-compiled compilers it can take
quite some time because we have to build the compiler itself three
different times. The first is the normal bootstrap, the second is a
second build for the build platform, and the third is the actual target
architecture compiler. The second compiler was historically built
exclusively for procedural macros, and long ago we didn't actually need
it.

This commit tries out avoiding that second compiled compiler, meaning we
only compile rustc for the build platform only once. Some local testing
shows that this is promising, but bors is of course the ultimate test!

Currently when we're preparing cross-compiled compilers it can take
quite some time because we have to build the compiler itself three
different times. The first is the normal bootstrap, the second is a
second build for the build platform, and the third is the actual target
architecture compiler. The second compiler was historically built
exclusively for procedural macros, and long ago we didn't actually need
it.

This commit tries out avoiding that second compiled compiler, meaning we
only compile rustc for the build platform only once. Some local testing
shows that this is promising, but bors is of course the ultimate test!
@rust-highfive
Copy link
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 26, 2019
@alexcrichton
Copy link
Member Author

r? @pietroalbini

@pietroalbini
Copy link
Member

@bors r+ p=1

This should also give us a big performance boost on the current Travis and AppVeyor setup.

@bors
Copy link
Collaborator

bors commented May 26, 2019

📌 Commit ad52c77 has been approved by pietroalbini

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2019
@pietroalbini
Copy link
Member

@bors rollup=never

@bors

This comment has been minimized.

@pietroalbini

This comment has been minimized.

@bors

This comment has been minimized.

@bors

This comment has been minimized.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 26, 2019
@rust-highfive

This comment has been minimized.

@alexcrichton

This comment has been minimized.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2019
@alexcrichton
Copy link
Member Author

@bors: r-

Hm looking at the logs there's something else to remove, need to keep digging.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 26, 2019
@rust-highfive

This comment has been minimized.

This commit furthers the previous one to ensure that we don't build an
extra stage of the compiler in CI. A test has been added to rustbuild to
ensure that this doesn't regress, and then in debugging this test it was
hunted down that the `dist::Std` target was the one erroneously pulling
in the wrong compiler.

The `dist::Std` step was updated to instead account for the "full
bootstrap" or not flag, ensuring that the correct compiler for compiling
the final standard library was used. This was another use of the
`force_use_stage1` function which was in theory supposed to be pretty
central, so existing users were all evaluated and a new function,
`Builder::compiler_for`, was introduced. All existing users of
`force_use_stage1` have been updated to use `compiler_for`, where the
semantics of `compiler_for` are similar to that of `compiler` except
that it doesn't guarantee the presence of a sysroot for the arguments
passed (as they may be modified).

Perhaps one day we can unify `compiler` and `compiler_for`, but the
usage of `Builder::compiler` is so ubiquitous it would take quite some
time to evaluate whether each one needs the sysroot or not, so it's
hoped that can be done in parallel.
@alexcrichton
Copy link
Member Author

Ok the last two commits I believe actually fix the issue,

re-r? @pietroalbini

@pietroalbini pietroalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 29, 2019
@pietroalbini
Copy link
Member

This looks good! Let's try again.

@bors r+ p=1 rollup=never
(priority since it will reduce the build time a lot)

@bors
Copy link
Collaborator

bors commented May 29, 2019

📌 Commit 7b362bb has been approved by pietroalbini

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2019
@Centril
Copy link
Contributor

Centril commented May 29, 2019

@bors p=19

@bors
Copy link
Collaborator

bors commented May 29, 2019

⌛ Testing commit 7b362bb with merge 85774b4b8d5b7928b40e338268ba7caa9a1d46ba...

@bors
Copy link
Collaborator

bors commented May 29, 2019

💔 Test failed - checks-travis

@rust-highfive
Copy link
Contributor

The job dist-x86_64-apple of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
xmoto
zxing-cpp
Updating Homebrew...
==> Downloading https://homebrew.bintray.com/bottles/xz-5.2.4.high_sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/e7/e7be50f4ee00e35887f3957263334eb3baba59e8c061919060f9259351be6880?__gda__=exp=1559145307~hmac=16f77dee1e67ec6c9ef3e1df0b08fa0587b89943a38de3b7431282d51eeca7e5&response-content-disposition=attachment%3Bfilename%3D%22xz-5.2.4.high_sierra.bottle.tar.gz%22&response-content-type=application%2Fgzip&requestInfo=U2FsdGVkX1-8DkM2q-OecfuBFYOwbLz3IwtsIyr5RaCX4_uByIbrUssEw2K0G40XOKaWAHQkeGQ7yLuvH68UR1314Kdk6LpyMPeh6afa1wRuUc8kSXCWLnMAPOv1TRCrb2CNuMLwiHbwF8ntvOEBRg&response-X-Checksum-Sha1=32dc0b28e61f32b40c20e2993418aa8cb6e746d5&response-X-Checksum-Sha2=e7be50f4ee00e35887f3957263334eb3baba59e8c061919060f9259351be6880
🍺  /usr/local/Cellar/xz/5.2.4: 92 files, 1MB
==> `brew cleanup` has not been run in 30 days, running now...
Removing: /Users/travis/Library/Caches/Homebrew/boost-1.66.0.high_sierra.bottle.tar.gz... (84.6MB)
Removing: /Users/travis/Library/Caches/Homebrew/carthage-0.28.0.high_sierra.bottle.tar.gz... (8.3MB)
---
Pruned 0 symbolic links and 5 directories from /usr/local
==> Installing dependencies for swig: pcre
==> Installing swig dependency: pcre
==> Downloading https://homebrew.bintray.com/bottles/pcre-8.43.high_sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/03/0389911a93a88efd4a69b52dea8ecb872fdb55bcfff45d2f7313be5f79730861?__gda__=exp=1559145323~hmac=8ccb64526e49641dd0ade180030fcca1b2d5dba5bf1de6b2a44e8ebd7b7a29ed&response-content-disposition=attachment%3Bfilename%3D%22pcre-8.43.high_sierra.bottle.tar.gz%22&response-content-type=application%2Fgzip&requestInfo=U2FsdGVkX1_ZYHKT-rABvKtE9Y8z6bNrEiPUdXf5iJjUjWVX0M91QX9L4GngsREZcMd64y46ttMZLQaO8vfMVduONHkuk8KUvT_y_9iolzsrr-Xwu42s1TtCZOgYVAsGwBGE5ntE3rgDxM9PGSky1A&response-X-Checksum-Sha1=c67d4b99bb245f0ea56b34118dd6325b06a7250c&response-X-Checksum-Sha2=0389911a93a88efd4a69b52dea8ecb872fdb55bcfff45d2f7313be5f79730861
🍺  /usr/local/Cellar/pcre/8.43: 204 files, 5.5MB
==> Installing swig
==> Downloading https://homebrew.bintray.com/bottles/swig-4.0.0.high_sierra.bottle.tar.gz
==> Downloading https://homebrew.bintray.com/bottles/swig-4.0.0.high_sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/ae/aed79cb436b3a0ac5812c4085e3121ffd62866397b8c7eaa06815ed8ec1e22b7?__gda__=exp=1559145325~hmac=244df401059da2c662c42329f4f0449145cf5e1892f2f4d955e46f010f956e9e&response-content-disposition=attachment%3Bfilename%3D%22swig-4.0.0.high_sierra.bottle.tar.gz%22&response-content-type=application%2Fgzip&requestInfo=U2FsdGVkX1_tiIss4whfnn_Ut65sPyFY5wTD_e3QpF_msQyxVQE9RSMYSZZtg6r4FlKoOCmVzMkCZ4UwjdSWS9NeyGCy3w6NA87TdrcjSmM1HLNse22-1YAq1qa3ETnM5L_YkLN5jtNejuJzl3NWNw&response-X-Checksum-Sha1=a9c428aee4337d91061a69c02d7ae508b627d03f&response-X-Checksum-Sha2=aed79cb436b3a0ac5812c4085e3121ffd62866397b8c7eaa06815ed8ec1e22b7
🍺  /usr/local/Cellar/swig/4.0.0: 722 files, 5.4MB
travis_time:end:02881e60:start=1559144244403738000,finish=1559144645872069000,duration=401468331000
travis_fold:end:install
travis_fold:start:before_script.1

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 29, 2019
@mati865
Copy link
Contributor

mati865 commented May 29, 2019

Timeout:

The job exceeded the maximum time limit for jobs, and has been terminated.

@pietroalbini
Copy link
Member

Ugh, it almost finished :/

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 29, 2019
@bors
Copy link
Collaborator

bors commented May 30, 2019

⌛ Testing commit 7b362bb with merge 19e0ddb...

bors added a commit that referenced this pull request May 30, 2019
ci: Attempt to skip a full rustc compile on dist*

Currently when we're preparing cross-compiled compilers it can take
quite some time because we have to build the compiler itself three
different times. The first is the normal bootstrap, the second is a
second build for the build platform, and the third is the actual target
architecture compiler. The second compiler was historically built
exclusively for procedural macros, and long ago we didn't actually need
it.

This commit tries out avoiding that second compiled compiler, meaning we
only compile rustc for the build platform only once. Some local testing
shows that this is promising, but bors is of course the ultimate test!
@bors
Copy link
Collaborator

bors commented May 30, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: pietroalbini
Pushing 19e0ddb to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 30, 2019
@bors bors merged commit 7b362bb into rust-lang:master May 30, 2019
alexcrichton added a commit to alexcrichton/rust that referenced this pull request May 30, 2019
Since rust-lang#61212 we've been timing out on OSX, and this looks to be because
we're building tools like Cargo and the RLS twice instead of once. This
turns out to be a slight bug in our configuration. CI builders using the
`RUST_CHECK_TARGET` directive actually execute `make all` just before
their acual target. In `make all` we're building a stage2 cargo, and
then in `make dist` we're building a stage1 cargo.

Other builders use `SCRIPT` which provides explicit control over what
`x.py` script, for example, is used to execute the build. This moves
almost all targets to using `SCRIPT` to ensure that we're explicitly
specifying what's being built where. Additionally this updates the logic
of `RUST_CHECK_TARGET` to remove the pre-flight tidy as well as the
pre-flight `make all`. The system LLVM builder (run on PRs) now
explicitly runs tidy first and then runs the rest of the test suite.
bors added a commit that referenced this pull request May 30, 2019
ci: Favor SCRIPT instead of RUST_CHECK_TARGET

Since #61212 we've been timing out on OSX, and this looks to be because
we're building tools like Cargo and the RLS twice instead of once. This
turns out to be a slight bug in our configuration. CI builders using the
`RUST_CHECK_TARGET` directive actually execute `make all` just before
their acual target. In `make all` we're building a stage2 cargo, and
then in `make dist` we're building a stage1 cargo.

Other builders use `SCRIPT` which provides explicit control over what
`x.py` script, for example, is used to execute the build. This moves
almost all targets to using `SCRIPT` to ensure that we're explicitly
specifying what's being built where. Additionally this updates the logic
of `RUST_CHECK_TARGET` to remove the pre-flight tidy as well as the
pre-flight `make all`. The system LLVM builder (run on PRs) now
explicitly runs tidy first and then runs the rest of the test suite.
@alexcrichton alexcrichton deleted the skip-rustc branch July 8, 2019 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants