-
Notifications
You must be signed in to change notification settings - Fork 273
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
Use sanitizers in travis #968
Conversation
79623b5
to
3e52d0b
Compare
22a2ee5
to
0d692b0
Compare
0d692b0
to
ecba3c0
Compare
This works now, although it does cause some of the builds to time out. Not sure how to proceed. |
@reuk, I'd suggest to enable it only for one of the builds. Each build taking 60min doesn't look viable. |
I think the problem is that the failing builds use both the undefined-behaviour and address sanitizers, while the succeeding builds use only the address sanitizer. Because each sanitizer slows down execution a bit, the builds with more sanitizers take longer than 50 minutes, at which point they time-out. However, it's important that we run all the tests with sanitizers on, as sanitizers only find errors in code which is actually run. Even if all sanitizers are enabled just for a single build, that build will still time-out. Therefore, I suggest either:
|
Still waiting for guidance on this. I think it's really important to use the sanitizers in Travis, but I'm not sure about the best way to proceed. |
I'd say build stages should be the way to go, in fact irrespective of the sanitizers. If that solves the timeout issue then that would of course be desirable, else one of the other options you proposed will need to be applied. Would you be able to invest time to implement build stages support? |
I have this afternoon, but I don't think this will be practical to fix today because Travis is so busy, due to the bug squashing party. |
This is superseded by #1624. |
Fixes #832
I don't have a way of testing this locally, please don't review until the CI is showing all green.