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

-Werror builds for Travis #4279

Merged
merged 3 commits into from Aug 25, 2017
Merged

-Werror builds for Travis #4279

merged 3 commits into from Aug 25, 2017

Conversation

pks-t
Copy link
Member

@pks-t pks-t commented Jun 23, 2017

As promised in #4211, this PR enables building with "-Werror". I've taken a different approach here than last time by creating new macros ENABLE_WARNINGS/DISABLE_WARNINGS. When passing "-DENABLE_WERROR" to CMake, the ENABLE_WARNINGS macro is switched over to add "-Werror=" instead of "-W<warning" to our CFLAGS variable.

I've switched over Travis to use this new switch by default. I'll probably have to disable this for the old Precise machines as they still have the issue with curl_easy_opt warnings, which are out of our scope to fix.

@pks-t pks-t force-pushed the pks/error-builds branch 2 times, most recently from 279c78e to a3f0d0c Compare June 23, 2017 10:56
@pks-t
Copy link
Member Author

pks-t commented Jun 27, 2017

Please hold until #4282 is merged due to rebasing being a major pain

There are multiple sites where we enable or disable compiler warning via
"-W<warning>" or "-Wno-<warning>". As we want to extend this mechanism
later on to conditionally switch these over to "-Werror=<warning>", we
encapsulate the logic into its their own macros `ENABLE_WARNINGS` and
`DISABLE_WARNINGS`.

Note that we in fact have to use a macro here. Using a function would
not modify the CFLAGS inside of the callers scope, but in the function's
scope only.
Add a simple switch to enable building with "-Werror=<warning>" instead
of "-W<warning". Due to the encapsulated `ENABLE_WARNINGS` macro, this
is as simple as adding a new variable "ENABLE_WERROR`, which can be
passed on the command line via `-DENABLE_WERROR=ON`. The variable
defaults to NO to not bother developers in their day to day work.
One of our goals is to have our code free of any warnings. Due to the
recent switch to Ubuntu 14.04 on Travis, the last warning regarding some
preprocessor-magic in the curl-headers has been fixed and as such, the
goal of zero warnings is now reached for Travis CI. In order to avoid
introducing new warnings via pull requests, we can now enable building
with `-Werror` and turn compiler warnings into errors instead, causing
the CI jobs to fail.

This build does so by passing the newly introdcued `-DENABLE_WERROR`
flag to CMake for all Travis jobs.
@pks-t
Copy link
Member Author

pks-t commented Aug 25, 2017

Rebased for #4282. If CI has no complaints I'd consider this ready to be merged now.

@ethomson ethomson merged commit bcb7e92 into libgit2:master Aug 25, 2017
@pks-t pks-t deleted the pks/error-builds branch September 15, 2017 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants