RFR: JDK-8003593: build-infra: Paths to optional platform-specific files should not be hardwired to src/closed

[Erik Joelsson]

Hello,

New webrev: http://cr.openjdk.java.net/~erikj/8003593/webrev.02/

Only change is the name of the suppress variable.

On 2016-07-04 03:02, David Holmes wrote:
> Fix typo ...
>
> On 4/07/2016 9:01 AM, David Holmes wrote:
>> Hi Erik,
>>
>> 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.
>>
I went with this suggestion as I like it better.
>>> * 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.
>>
This is all true. Different places have required different ways of 
overriding, appending or prepending with customizations. I will likely 
take another look at this and see if I can clean it up some at least. 
Perhaps unify the pre/post semantics. At least now we have captured all 
the current needs for customizations and gained some experience in what 
kind of constructs are needed.
>>>
>>> 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?
>
> ... hg rename the mapfile-vers.openjdk version instead ...
>
They are identical, which is why I just removed the logic. Historically 
there has been a difference but it was removed. Regarding which file we 
should save the history for, I'm not sure which is best. Delete-rename 
to the same file name can be confusing too.

/Erik