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

rustbuild: Add option for enabling partial LLVM rebuilds #40329

Merged
merged 3 commits into from
Mar 13, 2017

Conversation

petrochenkov
Copy link
Contributor

@alexcrichton , you probably didn't notice my late comment on #40236, but here's an implementation of that suggestion, it supersedes c652a4f.

r? @alexcrichton

@alexcrichton
Copy link
Member

Oh sorry I did indeed miss that, my bad!

FWIW I'm down for basically any behavior that's reasonable for local development. Local developers are much more forgiving with behavior like "looks weird? eh I'll blow it away". CI is much more unforgiving :(

That being said we're in a much better place than we were when all this infrastructure was created long ago. Almost all CI builders start from scratch unconditionally (e.g. no llvm build dir existing). We then rely on sccache to quickly compile LLVM (as it should all be cached).

Unfortunately, though, sccache doesn't quite work on MSVC with LLVM just yet (effort to get ninja working hasn't completed yet). So to counter this the LLVM build directory is cached across MSVC builds on appveyor. For us though AppVeyor handles the invalidation by not downloading the cache when src/rustllvm/llvm-auto-clean-trigger changes.

So the tl;dr; for CI is that:

  • All builds except MSVC have completely clean build directories, LLVM is always compiled from scratch
  • On MSVC, the build dir is cached across builds, and cleaned when src/rustllvm/llvm-auto-clean-trigger changes.

For now that means that we still require src/rustllvm/llvm-auto-clean-trigger to get changed whenever we touch LLVM, but in the future once sccache works well with LLVM on MSVC we can just remove all this "trigger" infrastructure.

(if you're willing to help out with sccache on ninja it'd be much appreciated as well!)

@alexcrichton
Copy link
Member

This PR seems fine by me, but want to default this option to false? You could then modify src/ci/run.sh to pass a ./configure option to turn it on by default on CI.

@petrochenkov
Copy link
Contributor Author

Updated.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 8, 2017

📌 Commit 5697240 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 9, 2017

🔒 Merge conflict

@alexcrichton
Copy link
Member

@bors: retry

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 10, 2017

📌 Commit c4b9284 has been approved by alexcrichton

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 10, 2017

📌 Commit 8c5bdd6 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 11, 2017

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

@petrochenkov
Copy link
Contributor Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Mar 11, 2017

📌 Commit 4cdbe5c has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 12, 2017

⌛ Testing commit 4cdbe5c with merge 4123c76...

@bors
Copy link
Contributor

bors commented Mar 12, 2017

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

@petrochenkov
Copy link
Contributor Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Mar 12, 2017

📌 Commit 4cda4d6 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 12, 2017

⌛ Testing commit 4cda4d6 with merge 49ab594...

@bors
Copy link
Contributor

bors commented Mar 12, 2017

💔 Test failed - status-travis

@petrochenkov
Copy link
Contributor Author

Not sure what happened, too much logging from sccache, the part of the log after 4MB is lost.

@bors retry

@bors
Copy link
Contributor

bors commented Mar 12, 2017

⌛ Testing commit 4cda4d6 with merge 4ab92b8...

@bors
Copy link
Contributor

bors commented Mar 12, 2017

💔 Test failed - status-appveyor

@petrochenkov
Copy link
Contributor Author

sccache failure:

error: failed to execute compile
caused by: error reading compile response from server
caused by: IoError(Error { repr: Os { code: 10054, message: "An existing connection was forcibly closed by the remote host." } })
caused by: An existing connection was forcibly closed by the remote host. (os error 10054)

@bors retry

@bors
Copy link
Contributor

bors commented Mar 12, 2017

⌛ Testing commit 4cda4d6 with merge 2936719...

@bors
Copy link
Contributor

bors commented Mar 12, 2017

💔 Test failed - status-appveyor

@petrochenkov
Copy link
Contributor Author

sccache failure

@bors retry

@bors
Copy link
Contributor

bors commented Mar 13, 2017

⌛ Testing commit 4cda4d6 with merge 0a1bce1...

@bors
Copy link
Contributor

bors commented Mar 13, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Mar 13, 2017

⌛ Testing commit 4cda4d6 with merge fd182c4...

bors added a commit that referenced this pull request Mar 13, 2017
rustbuild: Add option for enabling partial LLVM rebuilds

@alexcrichton , you probably didn't notice my [late comment](#40236 (comment)) on #40236, but here's an implementation of that suggestion, it supersedes c652a4f.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Mar 13, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing fd182c4 to master...

@bors bors merged commit 4cda4d6 into rust-lang:master Mar 13, 2017
@petrochenkov petrochenkov deleted the llreuse branch March 16, 2017 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants