On Thu, Feb 27, 2020 at 3:44 PM Severin Gehwolf <[email protected]> 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 >
