Magnus:
Looks good to me as well.
Tim
On 05/08/15 01:32, Erik Joelsson wrote:
Looks good.
/Erik
On 2015-05-08 09:57, Magnus Ihse Bursie wrote:
On 2015-04-20 09:02, Erik Joelsson wrote:
Looks good to me.
Thanks Erik.
Unfortunately, I never got round to pushing this. In the meantime,
the codebase evolved, and I had to add a couple of more disabled
warnings. I also modified the help text on a failed build slightly.
Here's the updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors/webrev.01
/Magnus
/Erik
On 2015-04-17 14:52, Magnus Ihse Bursie wrote:
With JDK-8074096, the number of warnings in the product was reduced
to a minimum. This enables the next step, which is turning on the
respective compiler flags that turns warnings into errors. In the
long run, this is the only way to keep the warnings from creeping
back.
Even with JDK-8074096, the product does not build 100% warning
free. This is due to some warnings that cannot be disabled, or (in
one case) where C and C++ code is mixed, and warnings for both
languages cannot be used. A system similar to the one introduced in
JDK-8074096 is therefore needed, in which individual libraries can
be exempted from this flag, until such warnings are fixed. A
library can thus disable warnings as errors with WARNINGS_AS_ERRORS
:= false, or (better) use a toolchain-specific version, e.g
WARNINGS_AS_ERRORS_gcc := false. This is intended as a temporary
measure, though. The long-term solution is reasonably to fix the
warnings and remove that argument.
Also, different versions of compilers can generate a different set
of warnings. It is therefore necessary to be able to turn off this
globally. Therefor a new flag for configure is introduced:
--disable-warnings-as-errors.
While the code compiles without errors on the build systems used
internally at Oracle, this might not be the case on other
combinations of operating system versions and toolchain versions.
To facilitate for unexpecting developers, a help message is added
if the build fails, that suggests using
--disable-warnings-as-errors. This solution was chosen as a
compromise between the "hard core" solution of turning on warnings
as errors by default for anyone, and the cowar... erm, conservative
solution of checking if the compiler versions exactly match what's
used inside Oracle (and therefore regularly tested), and only turn
it on in that case.
Similarly to JDK-8074096, I intend to file follow-up bugs for each
individual library that got a WARNINGS_AS_ERRORS_* := false.
Bug: https://bugs.openjdk.java.net/browse/JDK-8074859
WebRev for top:
http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-top/webrev.01
WebRev for jdk:
http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-jdk/webrev.01
Some comments:
* I needed to add a few more DISABLED_WARNINGS. For windows, this
is most likely due to the recent compiler change. For other
libraries, I'm not sure, but it might well be the result of recent
changes that has introduced new warnings. If so, it highlights the
need of this patch to keep the build warning free.
* For a few libraries and toolchains, there is *both*
WARNINGS_AS_ERRORS := false and a DISABLED_WARNINGS list. This is
the case if not all warnings are possible to disable.
* I have removed some incorrect uses of SHARED_LIBRARY_FLAGS. This
is included in our JDK LDFLAGS, so it should not be set separately,
and definitely not as CFLAGS. (This caused compiler warnings, which
now turned into errors.) However, a more suitable long-term
solution is probably to move the knowledge of how to create shared
libraries more specifically into SetupNativeCompilation, and not
set it as part of the JDK flags.
/Magnus