On Wed, 3 May 2023 17:58:25 GMT, Jiangli Zhou <jian...@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
