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

[Severin Gehwolf]

On Mon, 2019-03-11 at 18:09 +0000, 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?

Two reasons:
   1. My understanding is that one has to say ZIP_DEBUGINFO_FILES=true
      STRIP_POLICY=no_strip somehow so that this surfaces. That's
      something unsupported and that means "you are on your own when it
      breaks". Besides, these are implementation details.
   2. ZIP_DEBUGINFO_FILES=true STRIP_POLICY=no_strip as a combination are
      orthogonal. Making this use case work seems to give the illusion
      that this actually works: internal, but zipped debug info?

Sounds to me, the premise is wrong: It should have never worked ;-)
Thus, not a regression.

> > > 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?

When 8036003 was implemented for 8, there was the requirement to *not*
break existing use-cases. That is, existing config options --disable-
zip-debug-info and --disable-debug-symbols take precedence over the new
option and continue to work.

> 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