Skip to content

/ servo

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

Jump to bottom

Generated code from try builds can affect merging other PRs #16632

Open
jdm opened this issue Apr 27, 2017 · 8 comments
Open

Generated code from try builds can affect merging other PRs #16632

jdm opened this issue Apr 27, 2017 · 8 comments
Labels
A-build I-bustage

Comments

@jdm
Copy link
Member

@jdm jdm commented Apr 27, 2017

In #16624 we see a build failure caused by types added in #16622, which was sent to try last night but hasn't merged yet.
@highfive
Copy link

@highfive highfive commented Apr 27, 2017

cc @aneeshusa
This was referenced Apr 27, 2017
Merged
Merged
@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 27, 2017

It looks like generated code in the script crate is not re-generated when it should have been?

One way to definitely fix this is to revert #611, but that would also cause a regression in CI cycle time so I’d prefer to explore other options first.
@jdm
Copy link
Member Author

@jdm jdm commented Apr 27, 2017

Since the cmake build that runs the codegen process is synchronous (afaik), I guess the culprit would be the makefile.
@jdm
Copy link
Member Author

@jdm jdm commented Apr 27, 2017 •
edited

I think I have it figured out - the binding generation depends on the set of files in the webidls/ directory. #16622 added files to this but didn't modify any others, so subsequent PRs do not see any reason to regenerate the bindings because none of the files listed there have been modified.
@jdm jdm mentioned this issue Apr 27, 2017
Merged
bors-servo added a commit to servo/saltfs that referenced this issue Apr 27, 2017
@bors-servo
Auto merge of #651 - servo:jdm-patch-9, r=bustage
Loading status checks…
06c5fca 
Revert a51202b.

When we don't clobber the target directory, we run into problems like servo/servo#16602 and servo/servo#16632 on our builders. Let's revert that until we solve it properly.

<!-- 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/651)
<!-- Reviewable:end -->
@jdm
Copy link
Member Author

@jdm jdm commented Apr 27, 2017

Due to the continued bustage, I reverted the change on the builders in servo/saltfs#651.
@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 28, 2017

@jdm Does servo/saltfs#651 fix this or should we keep it open?
@jdm
Copy link
Member Author

@jdm jdm commented Apr 28, 2017

It does actually affect local builds where you switch branches after doing a build that adds a webidl file.
@jdm jdm added A-build and removed A-infrastructure labels Apr 28, 2017
@UK992 UK992 mentioned this issue May 4, 2017
Merged
bors-servo added a commit that referenced this issue May 8, 2017
@bors-servo
Auto merge of #16722 - UK992:clobber, r=SimonSapin
Loading status checks…
5c62685 
Add clobber mechanism

Solution for #16602 and #16632
Only work when `AUTOCLOBBER` environment variable is defined.

CC @SimonSapin @jdm

<!-- 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/16722)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 9, 2017
@bors-servo
Auto merge of #16722 - UK992:clobber, r=SimonSapin
Loading status checks…
9c9bd17 
Add clobber mechanism

Solution for #16602 and #16632
Only work when `AUTOCLOBBER` environment variable is defined.

CC @SimonSapin @jdm

<!-- 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/16722)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 9, 2017
@UK992
servo: Merge #16722 - Add clobber mechanism (from UK992:clobber); r=S…
a601310 
…imonSapin

Solution for servo/servo#16602 and servo/servo#16632
Only work when `AUTOCLOBBER` environment variable is defined.

CC @SimonSapin @jdm

Source-Repo: https://github.com/servo/servo
Source-Revision: 9c9bd178713a1def9bec57e7f96e3365252c0f88

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : cd841cb12094f76b56791b19b9ab970fda70d377
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this issue May 10, 2017
@UK992
servo: Merge #16722 - Add clobber mechanism (from UK992:clobber); r=S…
7bdd072 
…imonSapin

Solution for servo/servo#16602 and servo/servo#16632
Only work when `AUTOCLOBBER` environment variable is defined.

CC @SimonSapin @jdm

Source-Repo: https://github.com/servo/servo
Source-Revision: 9c9bd178713a1def9bec57e7f96e3365252c0f88
@nox
Copy link
Member

@nox nox commented May 17, 2017

Why are try builds even ran from the same directories as actual builds?
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
@marco-c
servo: Merge #16722 - Add clobber mechanism (from UK992:clobber); r=S…
8d313d2 
…imonSapin

Solution for servo/servo#16602 and servo/servo#16632
Only work when `AUTOCLOBBER` environment variable is defined.

CC SimonSapin jdm

Source-Repo: https://github.com/servo/servo
Source-Revision: 9c9bd178713a1def9bec57e7f96e3365252c0f88

UltraBlame original commit: 34907be7058c10abbac0b56390aa144ac6d4386e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
@marco-c
servo: Merge #16722 - Add clobber mechanism (from UK992:clobber); r=S…
a902a4e 
…imonSapin

Solution for servo/servo#16602 and servo/servo#16632
Only work when `AUTOCLOBBER` environment variable is defined.

CC SimonSapin jdm

Source-Repo: https://github.com/servo/servo
Source-Revision: 9c9bd178713a1def9bec57e7f96e3365252c0f88

UltraBlame original commit: 34907be7058c10abbac0b56390aa144ac6d4386e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
@marco-c
servo: Merge #16722 - Add clobber mechanism (from UK992:clobber); r=S…
84ccb65 
…imonSapin

Solution for servo/servo#16602 and servo/servo#16632
Only work when `AUTOCLOBBER` environment variable is defined.

CC SimonSapin jdm

Source-Repo: https://github.com/servo/servo
Source-Revision: 9c9bd178713a1def9bec57e7f96e3365252c0f88

UltraBlame original commit: 34907be7058c10abbac0b56390aa144ac6d4386e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-build I-bustage
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@jdm @nox @SimonSapin @highfive
You can’t perform that action at this time.