Re: [8u] RFR: 8206425: .gnu_debuglink sections added unconditionally when no debuginfo is stripped
On 17 July 2018 at 14:57, Severin Gehwolf wrote: > Hi Andrew, > > On Sun, 2018-07-15 at 03:11 +0100, Andrew Hughes wrote: >> On 6 July 2018 at 09:26, Severin Gehwolf wrote: >> > Hi David, >> > >> > On Fri, 2018-07-06 at 11:53 +1000, David Holmes wrote: >> > > Hi Severin, >> > > >> > > On 6/07/2018 2:41 AM, Severin Gehwolf wrote: >> > > > Hi, >> > > > >> > > > Please review this 8u-only change. JDK 10+ are not affected. >> > > > >> > > > For JDK 8 builds which don't perform any debug info stripping no >> > > > .gnu_debuglink sections should get generated. The fix is to move the >> > > > objcopy --add-gnu-debuglink calls into branches which actually perform >> > > > some stripping: all_strip, min_strip. Thoughts? >> > > >> > > Has anything changed with the tools here? This has been in place since >> > > back in 2011 with 7u2 and 8 GA! Why has it only become an issue now? >> > >> > I'm doubtful anything has changed with the tools. This is one of those >> > patches which we seem to have been carrying downstream for a very long >> > time. My archeology seems to suggest it started to show up in Fedora 23 >> > in November 2016. If anything, we'd be to blame to not push this fix >> > upstream earlier. >> >> The issue of 'no_strip' not being respected is a lot older than that >> and has been >> discussed on these lists at length before (e.g. [0] [1] [2]). The >> impression I got >> back that was that change was not wanted upstream and we should hack around >> the defaults with various environment variables. >> >> This fix only seems to make things more confusing. The .gnu_debuglink section >> is now only created when STRIP_POLICY is 'all_strip' or 'min_strip', but the >> debuginfo file is created for any value of STRIP_POLICY. The result is that >> other values of STRIP_POLICY will create a debuginfo file that is not linked >> to the original object. That seems like a regression from what happened >> previously. >> >> defs.make refers to a 'no_strip' value for STRIP_POLICY, but it is never >> checked for elsewhere in the build, as far as I can see. I feel that the >> clearer >> logic in these files would be: >> >> ifneq ($(STRIP_POLICY),no_strip) >> $(QUIETLY) $(OBJCOPY) --only-keep-debug $@ $(LIBJVM_DEBUGINFO) >> $(QUIETLY) $(OBJCOPY) --add-gnu-debuglink=$(LIBJVM_DEBUGINFO) $@ >> endif > > I can fix this with https://bugs.openjdk.java.net/browse/JDK-8207402, > but if JDK-8207234 goes into JDK 8u, unknown values for STRIP_POLICY > should become less likely as users would be able to use --with-native- > debug-symbols=internal for that use-case. That would be in-line with > JDK 9+ > > Thanks, > Severin > >> [0] >> http://mail.openjdk.java.net/pipermail/build-dev/2012-September/006680.html >> [1] >> http://mail.openjdk.java.net/pipermail/build-infra-dev/2013-March/003186.html >> [2] >> http://mail.openjdk.java.net/pipermail/build-dev/2014-February/012019.html >> [3] >> http://mail.openjdk.java.net/pipermail/build-dev/2012-September/006695.html I'm not too familiar with how it works in 9, but this sounds good. I'll look over these changes. My main point was to make clear that the reason we haven't fixed this already is not for lack of trying! -- Andrew :) Senior Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Web Site: http://fuseyism.com Twitter: https://twitter.com/gnu_andrew_java PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net) Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
adding additional numbers to the Java version string
Hi all, According to the Java version string spec (JEPs 223 and 322) the first part of the version string is a sequence of numbers separated by periods. The sequence can be of arbitrary length. However, in the OpenJDK configure scripts, the sequence length is fixed to exactly four numbers. For our internal builds we’d like to add at least one additional number to the version string. Is there any interest in a change to the scripts to allow that? I’ve prototyped this in a generic way (can add up to 3 additional numbers, called “extra1”, “extra2”, and “extra3”). These are set by passing --with-version-extra(1|2|3)=… to configure. If they are not set, the version string is of course exactly the same as it was before. I also changed the way the value of --with-version-string=… is parsed to be able to also extract the additional three numbers, if present. Would this be generally helpful? Tony — Tony Printezis | @TonyPrintezis | tprinte...@twitter.com
Re: custom extension for make/SourceRevision.gmk
> On Jul 18, 2018, at 1:46 PM, Erik Joelsson wrote: > > Hello Christian, > > Sometimes we need hooks both close to the beginning and close to the end of > the file, and in that case we create a SourceBundle-post.gmk. The recommended > position of the post inclusion is right before the typical "all: $(TARGETS)" > declaration. This file has the all target depend explicitly on a list of > phony targets and no TARGETS variable, so I would recommend changing that to > building a TARGETS variable like we usually do. That way you can create a > SourceBundle-post.gmk and clear the TARGETS variable from any targets you > don't want to run from the open file. Does that sound ok? Yes, that would be great. In JDK 11, please :-) > > /Erik > > > On 2018-07-18 10:31, Christian Thalinger wrote: >> Here at Twitter our OpenJDK clone lives in a GIT repository and we would >> like to use the source-revision feature of the release file. >> >> I have all the custom extension logic in place to create the revision >> tracker: >> >> $ cat >> build/macosx-x86_64-normal-server-release/support/src-rev/source-revision-tracker >> .:ea60d3b1efc0 >> >> but the way make/SourceRevision.gmk is structured (the custom extensions is >> included at the top of the file) always triggers the warning: >> >> $ make >> Building target 'default (exploded-image)' in configuration >> 'macosx-x86_64-normal-server-release' >> Warning: No mercurial configuration present and no .src-rev >> Finished building target 'default (exploded-image)' in configuration >> 'macosx-x86_64-normal-server-release' >> >> Is there a way so silence the warning without restructuring the upstream >> file? And if no, can we change it so it works? >
Re: custom extension for make/SourceRevision.gmk
Hello Christian, Sometimes we need hooks both close to the beginning and close to the end of the file, and in that case we create a SourceBundle-post.gmk. The recommended position of the post inclusion is right before the typical "all: $(TARGETS)" declaration. This file has the all target depend explicitly on a list of phony targets and no TARGETS variable, so I would recommend changing that to building a TARGETS variable like we usually do. That way you can create a SourceBundle-post.gmk and clear the TARGETS variable from any targets you don't want to run from the open file. Does that sound ok? /Erik On 2018-07-18 10:31, Christian Thalinger wrote: Here at Twitter our OpenJDK clone lives in a GIT repository and we would like to use the source-revision feature of the release file. I have all the custom extension logic in place to create the revision tracker: $ cat build/macosx-x86_64-normal-server-release/support/src-rev/source-revision-tracker .:ea60d3b1efc0 but the way make/SourceRevision.gmk is structured (the custom extensions is included at the top of the file) always triggers the warning: $ make Building target 'default (exploded-image)' in configuration 'macosx-x86_64-normal-server-release' Warning: No mercurial configuration present and no .src-rev Finished building target 'default (exploded-image)' in configuration 'macosx-x86_64-normal-server-release' Is there a way so silence the warning without restructuring the upstream file? And if no, can we change it so it works?
custom extension for make/SourceRevision.gmk
Here at Twitter our OpenJDK clone lives in a GIT repository and we would like to use the source-revision feature of the release file. I have all the custom extension logic in place to create the revision tracker: $ cat build/macosx-x86_64-normal-server-release/support/src-rev/source-revision-tracker .:ea60d3b1efc0 but the way make/SourceRevision.gmk is structured (the custom extensions is included at the top of the file) always triggers the warning: $ make Building target 'default (exploded-image)' in configuration 'macosx-x86_64-normal-server-release' Warning: No mercurial configuration present and no .src-rev Finished building target 'default (exploded-image)' in configuration 'macosx-x86_64-normal-server-release' Is there a way so silence the warning without restructuring the upstream file? And if no, can we change it so it works?
Re: [8u] RFR 8036003: Add --with-native-debug-symbols=[none|internal|external|zipped]
Hi Erik,
On Wed, 2018-07-18 at 08:44 -0700, Erik Joelsson wrote:
> Hello,
>
> Except for the left over debug printing $(info ...) line this looks good
> to me.
Thanks, Erik!
webrev with info line removed:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8036003/webrev.03/
HG-export:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8036003/JDK-8036003.jdk8.export.patch
Could you please sponsor this change once [I] gets approved? I'd then
close JDK-8207234 as a duplicate of the 8036003 8u-backport bug.
Thanks,
Severin
[I] http://mail.openjdk.java.net/pipermail/jdk8u-dev/2018-July/007670.html
> /Erik
>
>
> On 2018-07-18 06:42, Severin Gehwolf wrote:
> > Hi Erik,
> >
> > Latest webrev with default param over changes in the jdk tree:
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8036003/webrev.02/
> >
> > Thanks,
> > Severin
> >
> >
> > On Tue, 2018-07-17 at 09:16 -0700, Erik Joelsson wrote:
> > > That should work, but if it doesn't, please try double dollar on
> > > $(STRIP_POLICY). Then it gets evaluated when the macro is called instead
> > > of when it is defined.
> > >
> > > /Erik
> > >
> > >
> > > On 2018-07-17 09:01, Severin Gehwolf wrote:
> > > > Hi Erik,
> > > >
> > > > On Tue, 2018-07-17 at 07:24 -0700, Erik Joelsson wrote:
> > > > > Good work backporting the proper fix!
> > > >
> > > > Thanks for the review!
> > > >
> > > > > I think it would be good if you could make $1_STRIP_POLICY default to
> > > > > $(STRIP_POLICY) in SetupNativeCompilation instead of setting it as
> > > > > parameter basically everywhere. That shouldn't change the default
> > > > > behavior anywhere I think?
> > > >
> > > > How would I do that? No values from spec.gmk seem to be available in
> > > > SetupNativeCompilation. For example, this doesn't work (with the params
> > > > passed in in the jdk repo reverted):
> > > >
> > > > diff --git a/make/common/NativeCompilation.gmk
> > > > b/make/common/NativeCompilation.gmk
> > > > --- a/make/common/NativeCompilation.gmk
> > > > +++ b/make/common/NativeCompilation.gmk
> > > > @@ -260,6 +260,10 @@
> > > >$1_CC:=$(CC)
> > > > endif
> > > >
> > > > + ifeq ($$($1_STRIP_POLICY),)
> > > > +$1_STRIP_POLICY:=$(STRIP_POLICY)
> > > > + endif
> > > > +
> > > > # Make sure the dirs exist.
> > > > $$(eval $$(call MakeDir,$$($1_OBJECT_DIR) $$($1_OUTPUT_DIR)))
> > > > $$(foreach d,$$($1_SRC), $$(if $$(wildcard $$d),,$$(error SRC
> > > > specified to SetupNativeCompilation $1 contains missing directory $$d)))
> > > >
> > > > Thanks,
> > > > Severin
> > > >
> > > > > On 2018-07-17 05:54, Severin Gehwolf wrote:
> > > > > > Hi,
> > > > > >
> > > > > > This work started as a patch for JDK-8207234[1], but turned out to
> > > > > > become a 8-backport of JDK-8036003[2] using the old build logic and
> > > > > > with backwards-compatibilty. We are facing an issue where
> > > > > > .gnu_debuglink sections get generated unconditionally on the various
> > > > > > JDK libs (serviceability, security, nio, awt, etc.). That's an
> > > > > > issue,
> > > > > > since for our distro builds the stripping happens outside the
> > > > > > OpenJDK
> > > > > > build post-factum. The result of adding debug links unconditionally
> > > > > > is
> > > > > > that our binaries would have two links, one pointing to a file which
> > > > > > doesn't exist.
> > > > > >
> > > > > > What's more, there is no real support for the kind of debug-info
> > > > > > build
> > > > > > we need as downstream distribution consumers: Have all debug symbols
> > > > > > present, but leave them in the binary/library itself without any
> > > > > > stripping. That's basically what JDK 11's --with-native-debug-
> > > > > > symbols=internal does. This patch ports that configure flag to JDK
> > > > > > 8u
> > > > > > and keeps backward-compatibility with --disable-debug-symbols and --
> > > > > > disable-zip-debug-info flags. More info on this in [1].
> > > > > >
> > > > > > Bug(s): https://bugs.openjdk.java.net/browse/JDK-8207234
> > > > > >https://bugs.openjdk.java.net/browse/JDK-8036003
> > > > > > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8207234/01/
> > > > > >
> > > > > > Testing:
> > > > > >
> > > > > > Tested all build configs that I could think of (on Linux x86_64):
> > > > > > * Old config is being preserved (default build of debug symbols
> > > > > > +
> > > > > > zipped; no --disable-zip-debug-info, --disable-debug-symbols,
> > > > > > --
> > > > > > with-native-debug-symbols flags)
> > > > > > * Plain external debug symbols (flag --disable-zip-debug-info)
> > > > > > * No debug-symbols build (flags --disable-zip-debug-info,
> > > > > > --disable-
> > > > > > debug-symbols)
> > > > > > * Only using --with-native-debug-
> > > > > > symbols={none,zipped,external,internal} (no flags
> > > > > > --disable-zip-
> > > > > > debug-info, --disable-debug-symbols). Where "internal" passes
> > >
Re: [8u] RFR 8036003: Add --with-native-debug-symbols=[none|internal|external|zipped]
Hi Erik,
Latest webrev with default param over changes in the jdk tree:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8036003/webrev.02/
Thanks,
Severin
On Tue, 2018-07-17 at 09:16 -0700, Erik Joelsson wrote:
> That should work, but if it doesn't, please try double dollar on
> $(STRIP_POLICY). Then it gets evaluated when the macro is called instead
> of when it is defined.
>
> /Erik
>
>
> On 2018-07-17 09:01, Severin Gehwolf wrote:
> > Hi Erik,
> >
> > On Tue, 2018-07-17 at 07:24 -0700, Erik Joelsson wrote:
> > > Good work backporting the proper fix!
> >
> > Thanks for the review!
> >
> > > I think it would be good if you could make $1_STRIP_POLICY default to
> > > $(STRIP_POLICY) in SetupNativeCompilation instead of setting it as
> > > parameter basically everywhere. That shouldn't change the default
> > > behavior anywhere I think?
> >
> > How would I do that? No values from spec.gmk seem to be available in
> > SetupNativeCompilation. For example, this doesn't work (with the params
> > passed in in the jdk repo reverted):
> >
> > diff --git a/make/common/NativeCompilation.gmk
> > b/make/common/NativeCompilation.gmk
> > --- a/make/common/NativeCompilation.gmk
> > +++ b/make/common/NativeCompilation.gmk
> > @@ -260,6 +260,10 @@
> > $1_CC:=$(CC)
> > endif
> >
> > + ifeq ($$($1_STRIP_POLICY),)
> > +$1_STRIP_POLICY:=$(STRIP_POLICY)
> > + endif
> > +
> > # Make sure the dirs exist.
> > $$(eval $$(call MakeDir,$$($1_OBJECT_DIR) $$($1_OUTPUT_DIR)))
> > $$(foreach d,$$($1_SRC), $$(if $$(wildcard $$d),,$$(error SRC specified
> > to SetupNativeCompilation $1 contains missing directory $$d)))
> >
> > Thanks,
> > Severin
> >
> > > On 2018-07-17 05:54, Severin Gehwolf wrote:
> > > > Hi,
> > > >
> > > > This work started as a patch for JDK-8207234[1], but turned out to
> > > > become a 8-backport of JDK-8036003[2] using the old build logic and
> > > > with backwards-compatibilty. We are facing an issue where
> > > > .gnu_debuglink sections get generated unconditionally on the various
> > > > JDK libs (serviceability, security, nio, awt, etc.). That's an issue,
> > > > since for our distro builds the stripping happens outside the OpenJDK
> > > > build post-factum. The result of adding debug links unconditionally is
> > > > that our binaries would have two links, one pointing to a file which
> > > > doesn't exist.
> > > >
> > > > What's more, there is no real support for the kind of debug-info build
> > > > we need as downstream distribution consumers: Have all debug symbols
> > > > present, but leave them in the binary/library itself without any
> > > > stripping. That's basically what JDK 11's --with-native-debug-
> > > > symbols=internal does. This patch ports that configure flag to JDK 8u
> > > > and keeps backward-compatibility with --disable-debug-symbols and --
> > > > disable-zip-debug-info flags. More info on this in [1].
> > > >
> > > > Bug(s): https://bugs.openjdk.java.net/browse/JDK-8207234
> > > > https://bugs.openjdk.java.net/browse/JDK-8036003
> > > > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8207234/01/
> > > >
> > > > Testing:
> > > >
> > > > Tested all build configs that I could think of (on Linux x86_64):
> > > >* Old config is being preserved (default build of debug symbols +
> > > > zipped; no --disable-zip-debug-info, --disable-debug-symbols, --
> > > > with-native-debug-symbols flags)
> > > >* Plain external debug symbols (flag --disable-zip-debug-info)
> > > >* No debug-symbols build (flags --disable-zip-debug-info, --disable-
> > > > debug-symbols)
> > > >* Only using --with-native-debug-
> > > > symbols={none,zipped,external,internal} (no flags --disable-zip-
> > > > debug-info, --disable-debug-symbols). Where "internal" passes tests
> > > > in JDK-8207234
> > > >
> > > > Examples:
> > > >
> > > > configure output post-patch with flags --disable-zip-debug-info, --
> > > > disable-debug-symbols:
> > > >
> > > > [...]
> > > > checking if we should generate debug symbols... false
> > > > checking if we should zip debug-info files... no
> > > > 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
> > > > [...]
> > > >
> > > > configure output post-patch with flags --with-native-debug-
> > > > symbols=internal:
> > > >
> > > > [...]
> > > > 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)... internal
> > > > [...]
> > > >
> > > > configure output post-patch with flags --with-native-debug-
> > > > symbols=foobar:
> > > >
> > > > [...]
> > > > checking if we should generate debug symbols... true
> > > >
