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

Fri Feb 28 09:07:02 UTC 2020

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

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
>