RFR: 8010785: JDK 8 build on Linux fails with new build mechanism

Tue Jun 11 07:47:42 UTC 2013

On 2013-06-11 09:22, David Holmes wrote:
> Erik,
>
> On 7/06/2013 7:50 PM, Erik Joelsson wrote:
>>
>>
>> On 2013-06-07 04:08, David Holmes wrote:
>>> On 4/06/2013 10:38 PM, David Holmes wrote:
>>>> On 4/06/2013 9:21 PM, Erik Joelsson wrote:
>>>>>
>>>>>
>>>>> On 2013-06-04 06:56, David Holmes wrote:
>>>>>>
>>>>>> I think we should add a (configure time?) check to watch for the
>>>>>> illegal: BUILD_CRYPTO=no,  OPENJDK=true.
>>>>>>
>>>>> Not sure where that would fit. Configure is only setting BUILD_CRYPTO
>>>>> false in the closed logic, so this would be to catch someone either
>>>>> setting it on command line to configure or make, or a licensee adding
>>>>> --enable-openjdk-only to configure. To catch both we would need the
>>>>> check in the open. (And if we start looking at BUILD_CRYPTO in open
>>>>> configure, we might as well set it to yes there for a default and 
>>>>> skip
>>>>> ever having it empty.)
>>>>
>>>> Can we do something in the makefile then? Straight after the comment
>
> You never addressed this. Was there some reason not to put a check in 
> the Makefile?
>
Right, I started to look at it but gave it some more thought. 
BUILD_CRYPTO=no will only be set when building with closed present and 
not setting --enable-openjdk-only. The logic in the closed configure for 
this variable will be skipped if --enable-openjdk-only is given. So even 
if a licensee with a security purged source tree is trying to do this, 
the failure will be missing source code when compiling java classes and 
the variable BUILD_CRYPTO will still be unset, making a makefile check 
pointless.

For the pure open case, why would anyone get the idea to set 
BUILD_CRYPTO=no on the command line? I'm sure we have a number of 
variables that would screw up the build if set to random values on the 
command line and we can't really be expected to check all of them. For 
this I would think the comment explaining about the variable in 
CreateJars.gmk should be enough to discourage anyone to go about and set it.

So to sum it up, I couldn't come up with a situation where both of these 
variables would be set to conflicting values, that was likely enough to 
warrant an explicit check.

/Erik

> Otherwise looks okay.
>
> Thanks,
> David
>
>
>>>>
>>>> 475 # The source for the crypto jars is not available for all 
>>>> licensees.
>>>> The BUILD_CRYPTO
>>>> 476 # variable is set to false if these jars can't be built to skip 
>>>> that
>>>> step of the build.
>>>> 477 # Note that for OPENJDK, the build will fail if BUILD_CRYPTO=false
>>>> since then there is no
>>>> 478 # other way to get the jars then to build them.
>>>
>>> BTW in the above comment need to change "false" to "no".
>>>
>> Gahh, again, fixed in hopefully the last webrev on this issue. Think
>> this will be a new record for me.
>>
>> http://cr.openjdk.java.net/~erikj/8010785/webrev.jdk.05/
>>
>> Thanks for your patience.
>>
>> /Erik
>>
>>> David
>>>
>>>> ifeq ($(BUILD_CRYPTO), no)
>>>>    ifdef OPENJDK
>>>>      $(error Crypto libraries must be built in an OpenJDK build)
>>>>    endif
>>>> endif
>>>>
>>>> David
>>>>
>>>>> /Erik
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>> On 2013-06-03 06:22, David Holmes wrote:
>>>>>>>> Hi Erik,
>>>>>>>>
>>>>>>>> In CreateJars.gmk I don't understand why you move the update to 
>>>>>>>> the
>>>>>>>> JARS variable inside the BUILD_CRYPTO conditional when the jar
>>>>>>>> file is
>>>>>>>> a pre-req for a target defined outside of that conditional. 
>>>>>>>> What are
>>>>>>>> the allowed combinations:
>>>>>>>>
>>>>>>>> BUILD_CRYPTO=yes, OPENJDK=true  == OK (normal OpenJDK build)
>>>>>>>> BUILD_CRYPTO=yes, OPENJDK=false == OK? (builds but doesn't use 
>>>>>>>> it?)
>>>>>>>> BUILD_CRYPTO=no,  OPENJDK=true  == ILLEGAL? (missing re-req in
>>>>>>>> rule?)
>>>>>>>> BUILD_CRYPTO=no, OPENJDK=false  == OK (normal Oracle JDK build)
>>>>>>>>
>>>>>>>> This also seems to indicate that the earlier comment block:
>>>>>>>>
>>>>>>>> 469
>>>>>>>> ########################################################################################## 
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>  470 # For all security jars, always build the jar, but for 
>>>>>>>> closed,
>>>>>>>> install the prebuilt signed
>>>>>>>>  471 # version instead of the newly built jar. Unsigned jars are
>>>>>>>> treated as intermediate targets
>>>>>>>>  472 # and explicitly added to the JARS list. For open, signing
>>>>>>>> is not
>>>>>>>> needed. See SignJars.gmk
>>>>>>>>  473 # for more information.
>>>>>>>>
>>>>>>>> needs updating to account for this new condition. ("security" 
>>>>>>>> covers
>>>>>>>> these crypto jars).
>>>>>>>>
>>>>>>> This is true and I've updated the comment to point it out.
>>>>>>> BUILD_CRYPTO=false and OPENJDK=true is not a legal combination.
>>>>>>>> ---
>>>>>>>>
>>>>>>>> In Setup.gmk, wouldn't this:
>>>>>>>>
>>>>>>>> 38 ifndef OPENJDK
>>>>>>>>   39   # Some licensees do not get the Security Source 
>>>>>>>> bundles.  We
>>>>>>>> will
>>>>>>>>   40   # fall back on the prebuilt jce.jar so that we can do a 
>>>>>>>> best
>>>>>>>>   41   # attempt at building.
>>>>>>>>   42   ifeq ($(wildcard
>>>>>>>> $(JDK_TOPDIR)/src/share/classes/javax/crypto/Cipher.java),)
>>>>>>>>   43     JCE_PATH :=
>>>>>>>> $(PATH_SEP)$(JDK_TOPDIR)/make/closed/tools/crypto/jce/jce.jar
>>>>>>>>   44   endif
>>>>>>>>   45 endif
>>>>>>>>
>>>>>>>> be better handled by a configure check that the sources exist -
>>>>>>>> as is
>>>>>>>> done for other potentially not-present components? Further I think
>>>>>>>> this kind of check belongs in a closed build file as it doesn't
>>>>>>>> relate
>>>>>>>> to building the openjdk sources.
>>>>>>>>
>>>>>>> Also true. The condition is actually similar enough to that of
>>>>>>> BUILD_CRYPTO to be treated as the same. I moved this to a closed
>>>>>>> file.
>>>>>>>
>>>>>>> /Erik
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>> On 31/05/2013 8:14 PM, Erik Joelsson wrote:
>>>>>>>>> Finally getting back to this. Updated webrevs:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~erikj/8010785/webrev.jdk.02/
>>>>>>>>> http://cr.openjdk.java.net/~erikj/8010785/webrev.root.02/
>>>>>>>>>
>>>>>>>>> The javascript part is no longer needed since it has been 
>>>>>>>>> removed.
>>>>>>>>>
>>>>>>>>> /Erik
>>>>>>>>>
>>>>>>>>> On 2013-04-11 12:53, Erik Joelsson wrote:
>>>>>>>>>> Open part of this review.
>>>>>>>>>>
>>>>>>>>>> The licensee bundles aren't buildable with the new build for
>>>>>>>>>> several
>>>>>>>>>> reasons. I've tried to fix all the issues that I've found and 
>>>>>>>>>> have
>>>>>>>>>> now
>>>>>>>>>> successfully built them on linux, windows and solaris. Here is a
>>>>>>>>>> list
>>>>>>>>>> of the changes that I had to do to OpenJDK:
>>>>>>>>>>
>>>>>>>>>> * Filter out javascript src when the rhino source isn't 
>>>>>>>>>> available.
>>>>>>>>>> Also do not copy rhino resource files when not available. 
>>>>>>>>>> This is
>>>>>>>>>> controlled by a new variable, INCLUDE_JAVASCRIPT, which we 
>>>>>>>>>> control
>>>>>>>>>> from closed configure and shouldn't affect the OpenJDK build. I
>>>>>>>>>> also
>>>>>>>>>> moved the copying of the resources to the correct makefile,
>>>>>>>>>> CopyIntoClasses.gmk.
>>>>>>>>>>
>>>>>>>>>> * If javax/crypto isn't available, jce.jar needs to be added to
>>>>>>>>>> the
>>>>>>>>>> bootclasspath of the main java compilation. Also, a number of
>>>>>>>>>> security
>>>>>>>>>> jar files shouldn't be built at all. (Normally these are built
>>>>>>>>>> just to
>>>>>>>>>> exercise the logic, but not used.) The kerberos library is also
>>>>>>>>>> excluded by this. Introduced the variable BUILD_CRYPTO, also
>>>>>>>>>> set by
>>>>>>>>>> closed configure to control this. I used the logic ifneq
>>>>>>>>>> ($(BUILD_CRYPTO),no) to not change the behavior if the variable
>>>>>>>>>> isn't
>>>>>>>>>> set, which it won't be in the open case.
>>>>>>>>>>
>>>>>>>>>> * I removed the logic for setting the closed cacerts file in the
>>>>>>>>>> open
>>>>>>>>>> configure script.
>>>>>>>>>>
>>>>>>>>>> * Also fixing JDK-8005655 by adding logic for unzipping sec-bin
>>>>>>>>>> (and
>>>>>>>>>> friends) if available.
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~erikj/8010785/webrev.jdk.01/
>>>>>>>>>> http://cr.openjdk.java.net/~erikj/8010785/webrev.root.01/
>>>>>>>>>>
>>>>>>>>>> /Erik