[OpenJDK 2D-Dev] RFR: JDK-8003593: build-infra: Paths to optional platform-specific files should not be hardwired to src/closed

[Phil Race]

It is it all "extensions". It is mostly just different internal code as far as we (se client) are concerned so the word extension has entirely the wrong connotation for people in SE api land.

-Phil.

> On Jul 3, 2016, at 4:01 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Erik,
> 
>> On 2/07/2016 3:47 AM, Erik Joelsson wrote:
>> The separation between OpenJDK and Oracle's closed additions have
>> historically been quite messy. The build-infra project has tried to
>> improve on this, but failed in one regard, which was to hard code all
>> references to "closed" source instead of using a variable. I decided to
>> finally fix this.
> 
> Great this is getting fixed. For the record we tried to do this originally in the build (using "custom" variables) and in hotspot Using ALT_SRC variables). But other parts of the sources tended to hardwire the xxx/closed paths.
> 
> That said there are still issues with trying to provide a mechanism for other customizations to use - more below.
> 
>> Along the way, I found that there weren't that many
>> references left in open makefiles, which is a good thing. OpenJDK should
>> not be tainted with Oracle specific stuff unnecessarily. So then I
>> decided to completely remove the last references as part of fixing this
>> bug. With this patch, the following is now in effect:
>> 
>> * There is no longer a variable named "OPENJDK". That variable was
>> confusing and got in the way of other people trying to add custom
>> additions to the OpenJDK code base. In configure there is now only
>> "SUPPRESS_CUSTOM_SOURCE" which is set using the --enable-openjdk-only
>> option. This variable can be read by custom extensions to configure and
>> should be used to disable those custom extensions.
> 
> Only nit with that is the "source" tend to imply source code and of course this "source" can be quite a range of artifacts. SUPPRESS_CUSTOM_EXTENSIONS may have been better in that regard.
> 
>> * There is no Oracle specific logic left in open makefiles. All
>> customizations and references to custom source should be done in custom
>> makefiles, included using the IncludeCustomExtension macro. I have
>> converted the last uses of "ifndef OPENJDK" to such constructs.
> 
> Just an observation as this is a problem we have hit a few times with the customization mechanism. The placement of the custom includes ( eg IncludeCustomExtension macro) is generally dictated by the custom extension you are trying to accommodate. So including at the start/end/middle of a file may work well for the related Oracle extensions, but that doesn't necessarily generalize to other customizations. I see this is partly addressed by using a "post" variant of the file in places - and even on "pre"  (lib/Awt2dLibraries-pre.gmk) which seems wrong as the default seems to be pre.
> 
> Similarly the choice of replacing or updating variables is also being dictated by the way the Oracle extension works. For example, in
> 
> jdk/make/gendata/GendataBlacklistedCerts.gmk
> 
> you have:
> 
> GENDATA_BLACKLISTED_CERTS_SRC += $(JDK_TOPDIR)/make/data/blacklistedcertsconverter/blacklisted.certs.pem
> 
> which allows the variable to be extended. In contrast in
> 
> jdk/make/gendata/GendataFontConfig.gmk
> 
> you have:
> 
> GENDATA_FONT_CONFIG_DATA_DIR ?= $(JDK_TOPDIR)/make/data/fontconfig
> 
> which allows the variable to be replaced.
> 
> There is no general solution here of course - how to replace/augment depends on which operators are used and where the custom include location is.
> 
>> 
>> I have moved all Oracle specific mapfiles out of the open jdk repository.
> 
> Ok.
> 
> Some specific comments:
> 
> jdk/make/lib/Awt2dLibraries.gmk
> 
> You seem to have lost the ability to customise the libjavajpeg mapfile. Is that just because it is not needed on the Oracle side?
> 
> jdk/make/mapfiles/libfontmanager/mapfile-vers
> 
> Would it be better to delete the above file and hg rename mapfile- version instead?
> 
> Thanks,
> David
> -----
> 
>> Specifically to 2d-dev reviewers, I have moved
>> jdk/src/java.desktop/share/classes/sun/dc/DuctusRenderingEngine.java out
>> of the open as well. This file has been explicitly excluded from all
>> open builds since forever AFAICT. I see no reason for it be in the open.
>> If someone would like to read the source outside of Oracle, it will
>> still be in the hg history.
>> 
>> I have tested these changes extensively using the compare script and
>> -testset buildinfra in JPRT. This covers a wide variety of build
>> configurations so I feel pretty confident that it won't break anything.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8003593
>> Webrev: http://cr.openjdk.java.net/~erikj/8003593/webrev.01/
>> 
>> /Erik