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

David Holmes david.holmes at oracle.com
Mon Jul 4 01:17:09 UTC 2016


On 4/07/2016 10:21 AM, Phil Race wrote:
>
>
> -Phil.
>
>> On Jul 3, 2016, at 4:55 PM, Phil Race <philip.race at oracle.com> wrote:
>>
>> It is it all "extensions".
> That should have read :
> It is "not" all  ...

True but they are all customizations - so SUPPRESS_CUSTOMIZATIONS 
instead of SUPPRESS_CUSTOM_SOURCES ?

David

> - Phil
>
>> 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
>>
>



More information about the 2d-dev mailing list