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

Tue Jun 11 10:23:57 UTC 2013


On 2013-06-11 10:59, David Holmes wrote:
> On 11/06/2013 5:47 PM, Erik Joelsson wrote:
>>
>>
>> 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.
>
> Okay. Seemed like a simple sanity check to me without a need for too 
> much thought :)
>
You were indeed correct to bring it up and I was close to adding a 
check. But when weighing the pros and cons of where to put it, I started 
thinking about when it would be triggered, and discovered that I 
couldn't really trigger it. At that point it felt silly to implement it. 
Thanks for the review!

/Erik
> David
>
>> /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