RFR: JDK-8151653: Hotspot build does not respect --enable-openjdk-only
david.holmes at oracle.com
Tue Mar 15 12:33:49 UTC 2016
Thanks Erik! Looks okay to me.
On 15/03/2016 8:36 PM, Erik Joelsson wrote:
> New webrev where "closed" is replaced with "custom".
> 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.
> 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:
>>>> 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
>>> 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.
>>> 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.
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8151653
More information about the hotspot-dev