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

Reply via email to