[RFR] [11u] JDK-8232748: "Build static versions of certain JDK libraries"

[Andrew Hughes]

On 28/02/2020 09:07, Volker Simonis wrote:
> On Thu, Feb 27, 2020 at 3:44 PM Severin Gehwolf <sgehwolf at redhat.com> wrote:
>>
>> Hi Andrew,
>>
>> On Thu, 2020-02-27 at 04:52 +0000, Andrew Hughes wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8232748
>>> Webrev: https://cr.openjdk.java.net/~andrew/openjdk11/8232748/webrev.01/
>>>
>>> This patch adds targets to the build so that static versions of the JDK
>>> native libraries can be built, using static-libs-image. Such static
>>> versions of the libraries are required for consumption by Graal.
>>>
>>> With JDK-8210459 now in 11u, this is a largely clean backport, aside
>>> from some context changes, due to additional targets (JCov, JMH) being
>>> present in trunk:
>>>
>>> * In make/Bundles.gmk, 11u does not have jcov-bundles
>>> * In make/Main.gmk, 11u does not have jcov-image or jcov-bundles
>>> * In make/autoconf/spec.gmk.in, 11u does not have JMH_CORE_JAR, etc or
>>> JCOV_BUNDLE_NAME.
>>> * In make/conf/jib-profiles.js, the dependencies list in 11u doesn't
>>> include jmh and jcov.
>>>
>>> Building the new target, static-libs-image, succeeded. This should have
>>> no effect on other targets.
>>
>> Unfortunately this patch doesn't work. While a build of static-libs-
>> image succeeds, it doesn't create the image in build/<config-
>> name>/images. Expected is this:
>>
>> $ find build/linux-x86_64-normal-server-release/images/static-libs/
>> build/linux-x86_64-normal-server-release/images/static-libs/
>> build/linux-x86_64-normal-server-release/images/static-libs/lib
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libj2pkcs11.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libj2pcsc.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libnio.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libprefs.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjava.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjli.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libnet.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjimage.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjaas.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libfdlibm.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libj2gss.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libsunec.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjsig.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libextnet.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libverify.a
>> build/linux-x86_64-normal-server-release/images/static-libs/lib/libzip.a
>>
>> The reason for this is that FindFiles isn't available in JDK 11u. I had
>> to do these modifications to your patch to make it work:
>>
>> diff --git a/make/Bundles.gmk b/make/Bundles.gmk
>> --- a/make/Bundles.gmk
>> +++ b/make/Bundles.gmk
>> @@ -367,7 +367,7 @@
>>  ################################################################################
>>
>>  ifneq ($(filter static-libs-bundles, $(MAKECMDGOALS)), )
>> -  STATIC_LIBS_BUNDLE_FILES := $(call FindFiles, $(STATIC_LIBS_IMAGE_DIR))
>> +  STATIC_LIBS_BUNDLE_FILES := $(shell $(FIND) $(STATIC_LIBS_IMAGE_DIR))
>>
>>    ifeq ($(OPENJDK_TARGET_OS)-$(DEBUG_LEVEL), macosx-release)
>>      STATIC_LIBS_BUNDLE_SUBDIR := $(JDK_MACOSX_CONTENTS_SUBDIR)/Home
>> diff --git a/make/StaticLibsImage.gmk b/make/StaticLibsImage.gmk
>> --- a/make/StaticLibsImage.gmk
>> +++ b/make/StaticLibsImage.gmk
>> @@ -42,7 +42,7 @@
>>        SRC := $(SUPPORT_OUTPUTDIR)/native/$m, \
>>        DEST := $(STATIC_LIBS_IMAGE_DIR)/lib, \
>>        FILES := $(filter %$(STATIC_LIBRARY_SUFFIX), \
>> -          $(call FindFiles, $(SUPPORT_OUTPUTDIR)/native/$m/*/static)), \
>> +          $(shell $(FIND) $(SUPPORT_OUTPUTDIR)/native/$m/*/static)), \
>>    )) \
>>    $(eval TARGETS += $$(COPY_STATIC_LIBS_$m)) \
>>  )
>>
>> This uses FIND directly, but there ought to be a (convoluted?) way to
>> use CacheFind instead of FindFiles in 11u. Maybe build-dev folks could
>> help there?
>>

Right. Seems odd to me that make doesn't fail because of this, or even
issue a warning as far as I can see.

> 
> Hi Severin, Andrew,
> 
> there's no magic behind calling "CacheFind", you can just replace
> FindFiles by CacheFind. I've just did it and a test build with "make
> static-libs-image" resulted in the following output which seems to be
> correct:
> 
> $ find images/static-libs/
> images/static-libs/
> images/static-libs/lib
> images/static-libs/lib/libjaas.a
> images/static-libs/lib/libnet.a
> images/static-libs/lib/libzip.a
> images/static-libs/lib/libfdlibm.a
> images/static-libs/lib/libprefs.a
> images/static-libs/lib/libnio.a
> images/static-libs/lib/libextnet.a
> images/static-libs/lib/libj2pcsc.a
> images/static-libs/lib/libjimage.a
> images/static-libs/lib/libsunec.a
> images/static-libs/lib/libjava.a
> images/static-libs/lib/libjsig.a
> images/static-libs/lib/libj2gss.a
> images/static-libs/lib/libverify.a
> images/static-libs/lib/libjli.a
> images/static-libs/lib/libj2pkcs11.a
> 
> I've uploaded the updated webrev for your convenience to:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2020/8232748.02/
> 
> While I think replacing "FindFiles" by "CacheFind" in this downport is
> OK, we might also consider to downport "8189861: Refactor CacheFind"
> which introduced "FindFiles".
> 
> I think this might be useful because it was introduced in jdk13, so
> there might be other downports in the future which might depend on it
> and benefit from it being in jdk11u. We recently had another downport
> (8223678: Add Visual Studio Code workspace generation support (for
> native code)) which required "AssertEquals", something that's
> introduced by "8189861" as well. Andrew asked if it would make sense
> to downport the "AssertEquals" macro and I wnated to answer that it
> will probably make sense if we see another downport requiring it. Now,
> even before I answered we have this situation :) (this downport
> doesn't exactly require "AssertEquals" but "FindFiles" which was
> introduced with the same change).
> 
> On the other hand "8189861: Refactor CacheFind" which introduced
> "FindFiles" isn't trivial to downport. I've just did a quick check and
> it require manual resolving in several files (even if I ignore the
> copyright-headers for now). I think I could volunteer to do it, but it
> would definitely require some work. Maybe now, where we'te at the
> beginning of 11.0.8 it's a good point in time to do such a change?
> 
> I'll leave the final decision up to Andrew?
> 
> Best regards,
> Volker
> 
>> Thanks,
>> Severin
>>
> 

I'll look at backporting 8189861. Seems it gives us two functions we
need for the price of one backport.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222