RFR: JDK-8151653: Hotspot build does not respect --enable-openjdk-only

[Erik Joelsson]

Hello,

New webrev where "closed" is replaced with "custom".
http://cr.openjdk.java.net/~erikj/8151653/webrev.hotspot.02/

Also, to clarify the refactoring of trace.xml here on the list too. I 
needed to create 2 separate entry points, one open and one closed 
trace.xml. The way xinclude works, I can't just include the open 
trace.xml from the closed one, I have to refactor them to instead 
include the same content. That's why I moved the contents of the current 
open trace.xml to two new files, one for each XML-tag at that level in 
the XML.

/Erik

On 2016-03-15 01:46, David Holmes wrote:
> On 14/03/2016 11:22 AM, David Holmes wrote:
>> Hi Erik,
>>
>> On 12/03/2016 2:31 AM, Erik Joelsson wrote:
>>> Hello,
>>>
>>> When building hotspot with closed sources present and configuring with
>>> --enable-openjdk-only, various closed parts are included in the build
>>> anyway, at least on Windows. This needs to be fixed in preparation for
>>> the new hotspot build for build output comparisons to be meaningful
>>> between the old and new.
>>>
>>> The major culprit here, which applies to all platforms, is the trace
>>> source generation. The base trace.xml uses XInclude to explicitly and
>>> unconditionally include closed xml files if present. I'm fixing this by
>>> introducing a closed version of trace.xml which includes the open and
>>> closed parts, while the open trace.xml only includes the open parts.
>>
>> You've also done a significant amount of refactoring of that file and
>> split it into three parts. It's hard to spot the actual functional
>> differences in all that.
>
> Sorry that was a bit terse. It would have been clearer if you had 
> explained about the refactoring. I can see why the refactoring was 
> needed.
>
> Thanks,
> David
> -----
>
>> Given we have distinct directories from which we locate files it is a
>> pity to introduce something like this:
>>
>>   XML_DEPS += $(TraceAltSrcDir)/traceeventsclosed.xml
>>
>> where traceevents.xml is now traceeventsclosed.xml. This "alt src"
>> mechanism was supposed to abstract away the details of any particular
>> alternative configuration so that we didn't hardcode "closed" all over
>> the place. Other community members are supposed to be able to utilize
>> these mechanisms for their own customized build environments.
>>
>>> The rest of the changes are just for Windows, making sure the OPENJDK
>>> variable is propagated into the nmake build and making all conditionals
>>> on including closed source also check that variable.
>>
>> make/windows/build.make
>>
>> The combination !openjdk && !exists "closed" should be an error.
>>
>> As a meta-comment I think we've lost the plot somewhat with these "alt"
>> locations and how we manage them. The Oracle "closed" variants should
>> only be used when not trying to build OpenJDK (even if the files exist
>> in a forest), but other custom implementations may work in conjunction
>> with an OpenJDK build. To that end the "do nothing if building OpenJDK"
>> should be located within the "alt" files themselves, not at the point of
>> inclusion/use in the open files.
>>
>> David
>> -----
>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8151653
>>> Webrev:
>>> http://cr.openjdk.java.net/~erikj/8151653/webrev.hotspot.01/index.html
>>>
>>> /Erik