Do not merge yet: Enable incremental compilation by default, except on Windows where it’s buggy #15565
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Heads up! This PR modifies the following files:
|
highfive
added
the
S-awaiting-review
|
I’m really not sure we should do this just yet, but let’s see if it breaks CI: @bors-servo try |
bors-servo
pushed a commit
that referenced
this pull request
Feb 15, 2017
Do not merge yet: Enable incremental compilation by default <!-- Please describe your changes on the following line: --> --- <!-- 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 - [ ] These changes fix #__ (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/15565) <!-- Reviewable:end -->
|
💔 Test failed - windows-gnu-dev |
|
Sigh. |
SimonSapin
force-pushed
the
incrementalism
branch
from
24a16ea
to
fa00447
Compare
SimonSapin
changed the title
|
@bors-servo try |
bors-servo
pushed a commit
that referenced
this pull request
Feb 17, 2017
Do not merge yet: Enable incremental compilation by default, except on Windows where it’s buggy <!-- Please describe your changes on the following line: --> --- <!-- 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 - [ ] These changes fix #__ (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/15565) <!-- Reviewable:end -->
|
💔 Test failed - mac-dev-unit |
SimonSapin
force-pushed
the
incrementalism
branch
from
fa00447
to
ae9ae6d
Compare
|
Disabling for the stable compiler (geckolib). @bors-servo try |
bors-servo
pushed a commit
that referenced
this pull request
Feb 17, 2017
Do not merge yet: Enable incremental compilation by default, except on Windows where it’s buggy <!-- Please describe your changes on the following line: --> --- <!-- 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 - [ ] These changes fix #__ (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/15565) <!-- Reviewable:end -->
|
💔 Test failed - windows-gnu-dev |
SimonSapin
force-pushed
the
incrementalism
branch
from
ae9ae6d
to
e585717
Compare
|
@bors-servo try |
:/ |
|
☔ The latest upstream changes (presumably #15911) made this pull request unmergeable. Please resolve the merge conflicts. |
* On Windows where it’s buggy: rust-lang/rust#39644 * On our CI cross-compiling to ARM Linux or Android, these builders don’t have enough RAM: #15565 (comment)
SimonSapin
force-pushed
the
incrementalism
branch
from
f0d3c7d
to
3e9b579
Compare
|
That failure was due to updating Cargo. See #15852. Rebased on rust update at #15911. @bors-servo try |
bors-servo
pushed a commit
that referenced
this pull request
Mar 11, 2017
Do not merge yet: Enable incremental compilation by default, except on Windows where it’s buggy <!-- Please describe your changes on the following line: --> --- <!-- 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 - [ ] These changes fix #__ (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/15565) <!-- Reviewable:end -->
|
☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev |
|
This change appears to break TravisCI: |
jdm
added
S-fails-travis
S-needs-code-changes
highfive
added
S-awaiting-review
|
Looks like the relevant part is “No space left on device”. Incremental compilation does is a lot of space, probably more than is worth preserving through Travis-CI’s "cache" feature. Pushed a commit that should disable it on Travis. |
|
Looks good to merge to me, but I'm holding off due to the "Do not merge yet" in the title. |
|
Right, I think we’ve resolved the various issues with this but I’m not convinced we should make incremental compilation the default. (And the answer to that could be different for CI and for developers.) I’m planning to write to dev-servo to discuss the trade-offs, but there’s no hurry. |
jdm
added
S-needs-decision
|
☔ The latest upstream changes (presumably #16487) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Not making a decision is in its own way a decision. |
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is