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

Unbreak build without pch #3315

Closed
wants to merge 2 commits into from
Closed

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Jan 12, 2020

The change for WSL relied on #include <sys/utsname.h> being included everywhere. Also change release.nix so we build both builds in hydra/ofborg, preventing such regressions in the future.

Ericson2314 added 2 commits Jan 12, 2020
Do this to prevent such regressions in the future.
@edolstra
Copy link
Member

@edolstra edolstra commented Jan 13, 2020

Thanks, I've cherry-picked the first commit and did the regression test in a cheaper way (c86c71c).

@edolstra edolstra closed this Jan 13, 2020
@Ericson2314
Copy link
Member Author

@Ericson2314 Ericson2314 commented Jan 14, 2020

Err I understand wanting to keep the job names the same, but I think we really ought to CI everything we claim to to support, be it for production or ease of development.

@edolstra
Copy link
Member

@edolstra edolstra commented Jan 14, 2020

What's the problem? It is tested now.

@Ericson2314
Copy link
Member Author

@Ericson2314 Ericson2314 commented Jan 14, 2020

The opposite regression is still possible. Say someone develops without PCH for some reason, and breaks it. There would be no warning until someone else (e.g. you) goes back in a nix shell and finds their development environment broken.

Basically in my view if we aim to support both PRECOMPILED_HEADERS=0 and PRECOMPILED_HEADERS=1, then Hydra should build both.

@Ericson2314
Copy link
Member Author

@Ericson2314 Ericson2314 commented Jan 15, 2020

Oh forgive me, it was just the coverage checks doing this not all the CI builds.

@Ericson2314 Ericson2314 deleted the Ericson2314:uname-needs-include branch Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.