回复: [8u]RFR: 8263061: copy wrong unpack200.diz to bin directory on linux after 8252395(Internet mail)

[kalinshi(施慧)]

Thanks Severin!

It's more reasonable to keep same copy way in Makefile. Tested with both --with-native-debug-symbols=external --with-native-debug-symbols=zipped
Updated Webrev: http://cr.openjdk.java.net/~hshi/8263061/webrev_v2

Another finding is that debuginfo files (not zipped) are executable and this fails VersionCheck.java test.
Instead of porting JDK-8237192 changes to filter these debuginfo files, is it more reasonable to make them not executable on linux/solaris?
Making following changes in top level mercurial jdk8u repository.

diff -r 8006ef1eeba6 make/common/NativeCompilation.gmk
--- a/make/common/NativeCompilation.gmk Mon Mar 01 20:02:56 2021 +0000
+++ b/make/common/NativeCompilation.gmk Sat Mar 06 18:45:29 2021 +0800
@@ -480,6 +480,7 @@
                        $(OBJCOPY) --only-keep-debug $$< $$@
                        $(CD) $$(@D) && $(ADD_GNU_DEBUGLINK) $(LOG_INFO) $$(@F) $$<
                        $(TOUCH) $$@
+                       $(CHMOD) a-x $$@
             endif
           else ifeq ($(OPENJDK_TARGET_OS), linux)
             ifneq ($$($1_STRIP_POLICY), no_strip)
@@ -489,6 +490,7 @@
                        $(OBJCOPY) --only-keep-debug $$< $$@
                        $(CD) $$(@D) && $(OBJCOPY) --add-gnu-debuglink=$$(@F) $$<
                        $(TOUCH) $$@
+                       $(CHMOD) a-x $$@
             endif
           endif # No MacOS X support

Regards
Hui

-----邮件原件-----
发件人: Severin Gehwolf <sgehwolf at redhat.com> 
发送时间: 2021年3月6日 1:55
收件人: kalinshi(施慧) <kalinshi at tencent.com>; jdk8u-dev at openjdk.java.net
主题: Re: [8u]RFR: 8263061: copy wrong unpack200.diz to bin directory on linux after 8252395(Internet mail)

On Fri, 2021-03-05 at 07:35 +0000, kalinshi(施慧) wrote:
> Hi,
> 
> Please help review this fix.
> After 8252395, unpack200 executable is wrongly copied to bin directory 
> as unpack200.diz when configure with debuginfo.
> 
> Makefile sets same prerequisite for unpack200 executable and debuginfo 
> file. install-file function always copies first prerequisite, which is 
> executable.
> Fix is use debuginfo file as first prerequisite and copy it.
> 
> webrev: http://cr.openjdk.java.net/~hshi/8263061/webrev1/
> bug: https://bugs.openjdk.java.net/browse/JDK-8263061

558 # move DEBUGINFO_EXT suffix prerequisite to first prerequisite
559 BUILD_UNPACK_DEBUGINFO := $(filter %$(DEBUGINFO_EXT), $(BUILD_UNPACKEXE))
560 BUILD_UNPACK_DEBUGINFO += $(filter-out %$(DEBUGINFO_EXT), $(BUILD_UNPACKEXE))
561 $(JDK_OUTPUTDIR)/bin$(OUTPUT_SUBDIR)/unpack200$(DEBUGINFO_EXT): $(BUILD_UNPACK_DEBUGINFO)
562         $(call install-file)

Rather than sorting dependencies to be in the right order (make
unpack200.(debuginfo|diz) the first dependency) just to suit the install-file macro, have you considered using a similar pattern than is used for the java launcher? I.e. lets just not use install-file for this one.

This perhaps?

diff --git a/make/CompileLaunchers.gmk b/make/CompileLaunchers.gmk
--- a/make/CompileLaunchers.gmk
+++ b/make/CompileLaunchers.gmk
@@ -556,7 +556,9 @@
        $(call install-file)
 
 $(JDK_OUTPUTDIR)/bin$(OUTPUT_SUBDIR)/unpack200$(DEBUGINFO_EXT): $(BUILD_UNPACKEXE)
-       $(call install-file)
+       $(MKDIR) -p $(@D)
+       $(RM) $@
+       $(CP) -R 
+ $(JDK_OUTPUTDIR)/objs/unpackexe$(OUTPUT_SUBDIR)/unpack200$(DEBUGINFO_E
+ XT) $@
 
 BUILD_LAUNCHERS += $(JDK_OUTPUTDIR)/bin$(OUTPUT_SUBDIR)/unpack200$(EXE_SUFFIX)
 ifeq ($(ENABLE_DEBUG_SYMBOLS), true)


Thanks,
Severin