On 28/02/2020 09:07, Volker Simonis wrote: > 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? >>
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
