Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upDisable LLVM assertions by default, on supported platforms #15564
+26
−17
Conversation
highfive
commented
Feb 15, 2017
|
Heads up! This PR modifies the following files:
|
|
@aneeshusa has said previously that using a custom .servobuild for CI is more difficult than it sounds. |
|
What about an environment variable? We could have |
|
@metajack suggets a command-line flag, I’ll try that. |
SimonSapin
force-pushed the
no-gods-no-masters-no-assertions
branch
from
d51528e
to
4f19922
|
That’s annoying because the default config (and the path to the rustc directory) is set before @bors-servo try |
bors-servo
added a commit
that referenced
this pull request
Feb 16, 2017
Do not merge yet: Disable LLVM assertions by default <!-- Please describe your changes on the following line: --> #15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- 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/15564) <!-- Reviewable:end -->
|
|
SimonSapin
force-pushed the
no-gods-no-masters-no-assertions
branch
from
4f19922
to
d38ec26
SimonSapin
force-pushed the
no-gods-no-masters-no-assertions
branch
from
d38ec26
to
c1fa033
SimonSapin
changed the title
@bors-servo try |
bors-servo
added a commit
that referenced
this pull request
Feb 16, 2017
Do not merge yet: Disable LLVM assertions by default, on supported platforms <!-- Please describe your changes on the following line: --> #15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- 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/15564) <!-- Reviewable:end -->
SimonSapin
force-pushed the
no-gods-no-masters-no-assertions
branch
from
c1fa033
to
5070e45
Oh, wait, we do. It’s called |
But keep them on linux-dev CI.
SimonSapin
force-pushed the
no-gods-no-masters-no-assertions
branch
from
5070e45
to
bd8ec03
|
@bors-servo try |
bors-servo
added a commit
that referenced
this pull request
Feb 16, 2017
Do not merge yet: Disable LLVM assertions by default, on supported platforms <!-- Please describe your changes on the following line: --> #15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- 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/15564) <!-- Reviewable:end -->
|
Before this PR, r? @metajack |
SimonSapin
changed the title
|
|
|
@bors-servo r+ I don't like env variables, but I guess this is fine... |
|
|
bors-servo
added a commit
that referenced
this pull request
Feb 17, 2017
Disable LLVM assertions by default, on supported platforms <!-- Please describe your changes on the following line: --> #15559 (comment) > With an empty incremental compilation cache (or, presumably, with incremental compilation disabled), LLVM assertions add 16% to the compilation time in debug mode, 53% (!) in release mode. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- 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/15564) <!-- Reviewable:end -->
|
|
bors-servo
added a commit
to servo/saltfs
that referenced
this pull request
Jun 3, 2017
…larsbergstrom Support env vars in buildbot steps This makes it possible to specify environment variables for CI builds directly in `buildbot_steps.yml`, whether permanently, such as configuring LLVM assertions in servo/servo#15564, or temporarily, e.g. testing out `SCCACHE` for servo/servo#17106 before putting it into saltfs or trying out Android env vars. This is also helpful for windows, which AFAIK doesn't have an equivalent to the `env` command. r? @larsbergstrom @edunham <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/687) <!-- Reviewable:end -->
bors-servo
added a commit
to servo/saltfs
that referenced
this pull request
Jun 3, 2017
…larsbergstrom Support env vars in buildbot steps This makes it possible to specify environment variables for CI builds directly in `buildbot_steps.yml`, whether permanently, such as configuring LLVM assertions in servo/servo#15564, or temporarily, e.g. testing out `SCCACHE` for servo/servo#17106 before putting it into saltfs or trying out Android env vars. This is also helpful for windows, which AFAIK doesn't have an equivalent to the `env` command. r? @larsbergstrom @edunham <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/687) <!-- Reviewable:end -->
bors-servo
added a commit
to servo/saltfs
that referenced
this pull request
Jun 3, 2017
…larsbergstrom Support env vars in buildbot steps This makes it possible to specify environment variables for CI builds directly in `buildbot_steps.yml`, whether permanently, such as configuring LLVM assertions in servo/servo#15564, or temporarily, e.g. testing out `SCCACHE` for servo/servo#17106 before putting it into saltfs or trying out Android env vars. This is also helpful for windows, which AFAIK doesn't have an equivalent to the `env` command. r? @larsbergstrom @edunham <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/687) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
SimonSapin commentedFeb 15, 2017
•
edited
#15559 (comment)
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is