Re: How to add options to the javac build in the JDK 8 repo?

[David Holmes]

On 11/09/2013 3:19 PM, Joe Darcy wrote:
Hi David,
On 9/10/2013 8:31 PM, David Holmes wrote:
Hi Joe,

I assume these lint settings will cause the build to fail if someone
introduces a new warning.

That is the point actually :-)

I'm concerned that build breakage could easily be introduced. If that
happens is there a simple command-line make flag we can specify to
turn this off?

New code should be warning free for the lint categories which have been
cleared, either naturally or through judicious application of
@SuppressWarnings as needed. (For some other categories of warnings,
like deprecation, javac fixes are in-progress to improve the handling of
@SuppressWarnings.)

If you mean for experimentation, how can the settings be overridden,
then something like

     make JAVAC_WARNINGS="-Xlint:-all"

should work.

I was concerned about "accidentally" pushing something that didn't go 
through a proper build cycle. But that should be a rarity and of course 
if the Java code compiles cleanly once then it will compile cleanly 
always (unlike some other code :)).

David
-----

Going forward, I would be very leery of pushing changesets which
*reduced* the set of lint options turned on during the jdk build.

Cheers,

-Joe


Thanks,
David

On 11/09/2013 1:02 PM, Joseph Darcy wrote:
Hello,

Update on the patch status: the short version, when run through jprt,
lint issues in platform-specific and closed code reduce the set of lint
categories which can now be enabled. I've filed a few bugs to clean this
up:

     JDK-8024600 macosx code prevents use of -Xlint:auxiliaryclass,empty
in jdk build
     JDK-8024601 Windows networking code prevents use of
-Xlint:auxiliaryclass in jdk build
     JDK-8024603 Turn on javac lint checking for auxiliaryclass and
empty in jdk build

In the mean, time to allow some javac lint protection, I'd like to push
a patch for the issues the full jdk build is clean on; proposed patch
for review below.

Thanks,

-Joe

diff -r 909aced59bef makefiles/Setup.gmk
--- a/makefiles/Setup.gmk    Tue Sep 10 10:42:55 2013 +0100
+++ b/makefiles/Setup.gmk    Tue Sep 10 16:28:11 2013 -0700
@@ -1,5 +1,5 @@
  #
-# Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights
reserved.
+# Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights
reserved.
  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  #
  # This code is free software; you can redistribute it and/or modify it
@@ -25,6 +25,10 @@

DISABLE_WARNINGS:=-Xlint:all,-deprecation,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally


+# To build with all warnings enabled, do the following:
+# make JAVAC_WARNINGS="-Xlint:all -Xmaxwarns 10000"
+JAVAC_WARNINGS:=-Xlint:-unchecked,-deprecation,-overrides,classfile,dep-ann,divzero,varargs

-Werror
+
  # The generate old bytecode javac setup uses the new compiler to
compile for the
  # boot jdk to generate tools that need to be run with the boot jdk.
  # Thus we force the target bytecode to 7.
@@ -41,7 +45,7 @@
       JVM:=$(JAVA),\
       JAVAC:=$(NEW_JAVAC),\
       FLAGS:=-bootclasspath $(JDK_OUTPUTDIR)/classes -source 8
-target 8 \
-        -encoding ascii -XDignore.symbol.file=true
$(DISABLE_WARNINGS) \
+        -encoding ascii -XDignore.symbol.file=true $(JAVAC_WARNINGS) \
          $(GENERATE_JDKBYTECODE_EXTRA_FLAGS),\
       SERVER_DIR:=$(SJAVAC_SERVER_DIR),\
       SERVER_JVM:=$(SJAVAC_SERVER_JAVA)))

On 9/5/2013 12:03 PM, Stuart Marks wrote:
On 9/5/13 10:53 AM, Joe Darcy wrote:
On 9/5/2013 2:01 AM, Magnus Ihse Bursie wrote:
First of all, I think what you're doing is a great job, and I'm
all in
favor of enabling more warnings!

Thanks; I'm all for more warnings too :-)

I think you meant, you're for *fewer* warnings. :-)

 
DISABLE_WARNINGS:=-Xlint:all,-deprecation,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally



+JAVAC_WARNINGS:=-Xlint:-unchecked,-deprecation,-overrides,auxiliaryclass,classfile,dep-ann,divzero,empty,try,varargs


-Werror

Whither DISABLE_WARNINGS? It's used in a couple other build macros in
Setup.gmk. It's also used in a build macro in CompileJavaClasses.gmk.
Should they all be changed to use JAVAC_WARNINGS?

There may be valid reasons to use different warnings flags for
different build rules, but which variables are used where should be
made clear through better naming and comments.

It might be worthwhile to throw in a comment that describes how
JAVAC_WARNINGS can be overridden to enable all warnings. This is
useful for working on warnings cleanup or for generating statistics.
Something like:

# To build with all warnings enabled, do the following:
# make JAVAC_WARNINGS="-Xlint:all -Xmaxwarns 10000"

s'marks