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

[David Holmes]

Thanks Erik! Looks okay to me.

David

On 15/03/2016 8:36 PM, Erik Joelsson wrote:
> 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
>