RFR: 8307194: Enhance static-libs-image [v2]

Erik Joelsson erikj at openjdk.org
Wed May 3 18:37:16 UTC 2023


On Wed, 3 May 2023 17:58:25 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:

>> This PR is branched from the makefile changes for https://bugs.openjdk.org/browse/JDK-8303796 and contains the following for handling the JDK/VM static libraries:
>> 
>> - Create libjvm.a together with other JDK static libraries when building 'static-libs-image' (or 'static-libs-bundles') target, include it in 'images/static-libs/lib';
>> - For libjvm.a specifically, exclude operator_new.o;
>> - Filter out "external" .o files (those are the .o files included from a different JDK library and needed when creating the .so shared library only) from .a libraries; That's to avoid linker errors due to the duplicate symbols problems from the related .o files;
>> - Handle long arguments case for static build in make/common/NativeCompilation.gmk;
>> - Address @erikj79's comment in https://github.com/openjdk/jdk/pull/13709#discussion_r1180750185 for LIBJLI_STATIC_EXCLUDE_OBJS;
>
> Jiangli Zhou has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update based on @erikj79 review comments and suggestions:
>   - Change to copy libjvm.a for $(JVM_VARIANT_MAIN) only in make/StaticLibsImage.gmk.
>   - Use $(OBJ_SUFFIX) instead of .o in make/modules/java.base/lib/CoreLibraries.gmk.

make/Main.gmk line 261:

> 259: endef
> 260: 
> 261: $(foreach v, $(JVM_VARIANTS), $(eval $(call DeclareHotspotStaticLibsRecipe,$v)))

If we are only using the libjvm.a from the main variant, we don't need to generate top level rules for all variants, or we could at least limit the dependency declaration at line 1109 to only need the main variant.

make/StaticLibsImage.gmk line 56:

> 54:     DEST := $(STATIC_LIBS_IMAGE_DIR)/lib, \
> 55:     FILES := $(filter %$(STATIC_LIBRARY_SUFFIX), \
> 56:         $(call FindFiles, $(HOTSPOT_OUTPUTDIR)/variant-$(JVM_VARIANT_MAIN)/libjvm/objs/static))))

I would recommend using `wildcard` instead of `FindFiles` as the files are expected to be in a single directory (no recursive directory searching needed). We also try to put the closing double braces on a new line matching the initial eval in indentation.

Suggestion:

    FILES := $(wildcard $(HOTSPOT_OUTPUTDIR)/variant-$(JVM_VARIANT_MAIN)/libjvm/objs/static/*$(STATIC_LIBRARY_SUFFIX)), \
))

make/StaticLibsImage.gmk line 57:

> 55:     FILES := $(filter %$(STATIC_LIBRARY_SUFFIX), \
> 56:         $(call FindFiles, $(HOTSPOT_OUTPUTDIR)/variant-$(JVM_VARIANT_MAIN)/libjvm/objs/static))))
> 57: $(eval TARGETS += $$(COPY_STATIC_LIBS_libjvm))

No need for eval around this assignment.
Suggestion:

TARGETS += $(COPY_STATIC_LIBS_libjvm)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13768#discussion_r1184105236
PR Review Comment: https://git.openjdk.org/jdk/pull/13768#discussion_r1184103554
PR Review Comment: https://git.openjdk.org/jdk/pull/13768#discussion_r1184099771



More information about the client-libs-dev mailing list