Skip to content

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

pks-t added 3 commits August 25, 2017 17:32
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 pks-t force-pushed the pks/error-builds branch from a3f0d0c to 414a338 Compare August 25, 2017 15:37
@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.

2 participants