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

[David Holmes]

No further comments from me Erik!

Looks good.

Thanks,
David

On 5/07/2016 4:44 PM, Erik Joelsson wrote:
> 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
>