Hello Joe,
On 2013-08-13 23:31, Joe Darcy wrote:
A follow-up, the build incantation to embed a comma seems to be
"(COMMA)". Therefore, I will upgrade this to a code review request:
Actually, you need to use $(COMMA) for it to work. Unfortunately FLAGS
isn't a valid parameter to the SetupJavaCompilation macro and will be
silently ignored. For a list of valid parameters, see
common/makefiles/JavaCompilation.gmk. ADD_JAVAC_FLAGS is probably the
one you are looking for. Using this would result in two -Xlint
parameters on the javac line however.
Each SetupJavaCompilation call refers to a "SETUP" which is a compiler
configuration. GENERATE_JDKBYTECODE is defined in
jdk/makefiles/Setup.gmk and includes an -Xlint line disabling most
warnings. I tried replacing that with your line for
GENERATE_JDKBYTECODE. Unfortunately I had to disable fallthrough, try
and finally to get the build to pass. It looked like only one warning
per kind so should be easy to fix.
Here is the diff:
diff -r 18ce880b5fb4 makefiles/Setup.gmk
--- a/makefiles/Setup.gmk
+++ b/makefiles/Setup.gmk
@@ -25,6 +25,8 @@
DISABLE_WARNINGS:=-Xlint:all,-deprecation,-unchecked,-rawtypes,-cast,-serial,-dep-ann,-static,-fallthrough,-try,-varargs,-empty,-finally
+JAVAC_WARNINGS:=-Xlint:-unchecked,-deprecation,auxiliaryclass,classfile,divzero,empty,-fallthrough,-finally,-try,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 +43,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),\
/Erik
diff -r a4b0be7341ef makefiles/CompileJavaClasses.gmk
--- a/makefiles/CompileJavaClasses.gmk Tue Aug 13 19:10:54 2013 +0100
+++ b/makefiles/CompileJavaClasses.gmk Tue Aug 13 14:24:21 2013 -0700
@@ -291,6 +291,7 @@
$(eval $(call SetupJavaCompilation,BUILD_JDK,\
SETUP:=GENERATE_JDKBYTECODE,\
+
FLAGS:=-Xlint:-unchecked(COMMA)-deprecation(COMMA)auxiliaryclass(COMMA)classfile(COMMA)divzero(COMMA)empty(COMMA)fallthrough(COMMA)finally(COMMA)try(COMMA)varargs
-Werror,\
SRC:=$(JDK_TOPDIR)/src/share/classes \
$(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS_API_DIR)/classes \
$(MACOSX_SRC_DIRS) \
Thanks,
-Joe
On 08/13/2013 11:09 AM, Joe Darcy wrote:
Hello build gurus,
In in effort to have javac help us maintain good code quality in the
jdk 8 repo, I'd like to turn on selected lint warnings and make them
errors. In particular, the set of javac options I want to try out is
-Xlint:-unchecked,-deprecation,auxiliaryclass,classfile,divzero,empty,fallthrough,finally,try,varargs
-Werror
That is, treat as an error any of the auxiliaryclass,classfile, ...
lint categories. The jdk repo's code should already be clean on these
categories and I'd like to get it clean on other categories before
JDK 8 ships.
In any case, how does one get these javac options added to the build?
I've tried
--- a/makefiles/CompileJavaClasses.gmk Tue Aug 13 10:12:03 2013 -0700
+++ b/makefiles/CompileJavaClasses.gmk Tue Aug 13 11:07:01 2013 -0700
@@ -291,6 +291,7 @@
$(eval $(call SetupJavaCompilation,BUILD_JDK,\
SETUP:=GENERATE_JDKBYTECODE,\
+
FLAGS:=-Xlint:-unchecked,-deprecation,auxiliaryclass,classfile,divzero,empty,fallthrough,finally,try,varargs
-Werror,\
SRC:=$(JDK_TOPDIR)/src/share/classes \
$(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS_API_DIR)/classes \
$(MACOSX_SRC_DIRS) \
(as well as trying ADD_JAVAC_FLAGS instead of FLAGS), but get an
error like
CompileJavaClasses.gmk:292: *** Internal makefile error: Too many
arguments to SetupJavaCompilation, please update
JavaCompilation.gmk. Stop.
Thanks,
-Joe
