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

[David Holmes]

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