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

[Joe Darcy]

Hello again,

I hereby submit this patch for code review (subject to passing a 
cross-platform jprt run):

--- a/makefiles/Setup.gmk    Wed Aug 14 11:26:32 2013 -0700
+++ b/makefiles/Setup.gmk    Wed Aug 14 11:45:16 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
@@ -24,6 +24,7 @@
  #

  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

  # 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.
@@ -41,7 +42,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)))

Thanks,

-Joe

On 8/14/2013 3:08 AM, Erik Joelsson wrote:
> 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
>>