[RFR] 8220397: JDK-8036003 backport regresses no_strip builds

[Andrew John Hughes]

On 11/03/2019 18:09, Andrew John Hughes wrote:
> 
> 
> On 11/03/2019 11:04, Severin Gehwolf wrote:
>> On Mon, 2019-03-11 at 07:33 +0000, Andrew John Hughes wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8220397
>>> Webrev: https://cr.openjdk.java.net/~andrew/openjdk8/8220397/webrev.01/
>>>
>>> When JDK-8036003 was backported, it added a bunch of conditionals in
>>> make/common/NativeCompilation.gmk which cause debuginfo to not be
>>> generated if no_strip is set. However, the cases for ZIP_DEBUGINFO_FILES
>>> were missed, causing the build to fail, because there's no dependency
>>> for the .diz target.
>>>
>>> I suspect this doesn't show up when the new configure option is
>>> specified, because ZIP_DEBUGINFO_FILES is toggled appropriately, but it
>>> does regress my existing builds which don't specify that option.
>>
>> Sorry, but I'm not sure why this would be needed? There is a new
>> configure flag supporting these use-cases (including yours?):
>>
>> no debug symbols: --with-native-debug-symbols=none (or old mechanims)
>> zipped debug symbols: --with-native-debug-symbols=zipped (or old mechanism)
>> no stripping at all: --with-native-debug-symbols=internal (no supported way previously)
>> external debug symbols: --with-native-debug-symbols=external (or old mechanism)
>>
> 
> Yes, I'm aware of this, as I said with 'the new configure option'.
> 
>> Why would you want to continue some unsupported way to produce the
>> equivalent of --with-native-debug-symbols=internal?
>>
> 
> Why would you want to keep a regression of something that previously worked?
> 
>>> Simple fix and 8u only.
>>
>> Fix is simple enough, but it also encourages people to *not* change
>> their build scripts to the supported configure option :-(
> 
> So we should break people's build to force them to specify it, even
> though the breakage makes it in no way clear that this is the problem?
> 
> There's nothing different in this patch from what you added yourself to
> the same file in 8036003. You just missed a bunch of cases.
> 
>>
>> Thanks,
>> Severin
>>
> 

This is the configure output:

checking if we should generate debug symbols... true
checking if we should zip debug-info files... yes
checking what type of native debug symbols to use (this will override
previous settings)... not specified
configure: --with-native-debug-symbols not specified. Using values from
--disable-debug-symbols and --disable-zip-debug-info

It defaults to turning on zipped debug symbols. STRIP_POLICY="no_strip"
was previously ignored in that file. You made changes that made it
partially recognised, but wrongly assumed that STRIP_POLICY="no_strip"
means ZIP_DEBUGINFO_FILES is not set to "true".

You can't switch to the new configure flag if it doesn't exist. I've
adopted it when possible [0], but I originally wrote this patch in
October because I was also still having to build on a bunch of trees
that didn't have this change (including 7). That may be less likely now,
but I still think this is a bug that should be fixed.

TL;DR this is a clear regression that is easily fixed without breaking
anything else.

[0] https://icedtea.classpath.org/hg/icedtea8/rev/c5e1418ca5d0099e56c42d
-- 
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
https://keybase.io/gnu_andrew