Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries
On 2/12/19 11:52 AM, Severin Gehwolf wrote: Hi Mandy, Alan, Please find the proposal for CLI option of --strip-native-debug-symbols below. The current implementation here has the following options: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/05/webrev/ [i] --strip-native-debug-symbols defaults [ii] --strip-native-debug-symbols options:objcopy-cmd= [iii] --strip-native-debug-symbols options:debuginfo-file-ext= [iv] --strip-native-debug-symbols options:include-debug-syms=true The first option is a work-around for JDK-8218761. AFAIUI, fixing it will need rework of the Plugin interface and probably of the options parsing. Hence, I'd like to defer this post integration of the initial version of --strip-native-debug-symbols plugin. Cases [iii] and [iv] can be folded into one as suggested by Mandy with: --strip-native-debug-symbols keep-debuginfo --strip-native-debug-symbols keep-debuginfo= Case [ii] would become: --strip-native-debug-symbols objcopy= we could relax this to a command that can contain arguments. So in summary I'd propose these, where a) and b) may be combined, c) and a) or c) and b) combined would be an error: [a] --strip-native-debug-symbols keep-debuginfo[=] [b] --strip-native-debug-symbols objcopy= [c] --strip-native-debug-symbols defaults This is a good compromise. When JDK-8218761 is implemented, [c] can become `--strip-native-debug-symbols` "defaults" is unclear to what it does. What about --strip-native-debug-symbols no-keep-debuginfo As a follow-up to an initial implementation of the above, I'd propose to hook it up with the current --strip-debug by a follow-up patch. It would first rename --strip-debug to --strip-debug-attribute or perhaps --strip-java-debug-symbols, and then let --strip-debug perform java and native debug symbols stripping as Alan suggested. The renaming can be done separately. I would prefer changing --strip-debug to invoke --strip-native-debug-symbols, if present, at the same time with this new strip native debug symbols plugin to ensure that they all go in the same release. In other words, the renaming should be done before this new plugin. That's my opinion. Mandy Does that sound reasonable to you? Thanks, Severin
Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries
On 2/12/19 12:05 PM, Severin Gehwolf wrote: Hi Mandy, Thanks for the review! OK. Here is the summary: http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-February/014132.html Personally, I find --strip-native-debug-info or --strip-native-debug- symbols clearer than just --strip-native-debug. Fine with me and we can stick with --strip-native-debug-symbols W.r.t. the test: > The test currently only runs on Linux > and requires objcopy to be available. Otherwise the test is being > skipped. We can create a fake objcopy utility for testing purpose such that the test will validate if the options are passed properly to the test utility. AFAIK, objcopy is a build requirement for OpenJDK already, so I'm wondering whether it makes sense to stub objcopy for the tests. Perhaps you meant that to be used in addition to existing tests? Test machines may not have objcopy. It'd increase the test coverage to include a test that can run on all test machines. Yes it's an additional test in addition to a test you have that only runs if objcopy utility is present. Anyhow, I'll look into it. It'll likely have to use the '--strip- native-debug-symbols objcopy=case. What I was thinking is: --strip-native-debug-symbols objcopy="java fakeobjcopy" where objcopy accepts a command that can contain arguments. It'll have to be some native code which will require some custom make machinery to get compiled, no? If you have pointers to examples in the JDK already, I'd appreciate it. See the idea above. > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/ I haven't reviewed the new files. Just notice that very long lines in the new files that you may want to fix the formatting that will help further side-by-side review. Some of the longer lines have been cleaned up in the lates webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/05/webrev/ The --list-plugin output is very very long. The existing plugins didn't set a good example to keep the well formatted (I recorded that they were cleaned up at one point to keep 80 chars width). Right. I'll make sure --list-plugins looks right for --strip-native- debug-symbols once we've agreed on the options. One other question: should this plugin be moved to linux/classes rather than unix/classes given that that's the platform it intends to support? Done in version 05. Thanks for making the change. I will reply the other thread. Mandy
Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries
Hi Mandy, Thanks for the review! On Thu, 2019-02-07 at 12:43 -0800, Mandy Chung wrote: > Hi Severin, > > Using the plugin specific ResourceBundle is good. Thanks for making > the change. > > I see that Alan's comment [1] on the plugin options and I assume > you will investigate the plugin options and bring back a proposal. > Did I miss the discussion on these options? > > Option: > --strip-native-debug-symbols=][:debuginfo-file-ext=][:include-debug-syms=true]> > > Examples: --strip-native-debug-symbols=defaults > > I suggest the above be simplified and drop the "=defaults". Simply: > --strip-native-debug-symbols > > Examples: > --strip-native-debug-symbols=options:objcopy-cmd=/usr/bin/objcopy:debuginfo-file-ext=dbg:include-debug-syms=true > > > > If include-debug-syms=true then it will run >objcopy --only-keep-debug foo foo. to create debug symbols file >objcopy --add-gnu-debuglink=foo.dbg foo > > So what about simplifying the options to: > > --strip-native-debug-symbols=keep-debug-symbols=dbg,objcopy-path=/usr/bin/objcopy > > We could also drop the word "symbols": > > > --strip-native-debug=[keep-debug=,][objcopy-path=] > > By default, `--strip-native-debug` will strip native debug symbols > and don't keep debug symbols. > > keep-debug= creates the debug symbols file. > specifies the file extension. It would be > nice to use the default `debuginfo` extension and simply > accept `keep-debug` option. > > `objcopy-cmd` may be okay too. Others may have opinion. > > I think we should agree on the plugin options first before > doing the code review. OK. Here is the summary: http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-February/014132.html Personally, I find --strip-native-debug-info or --strip-native-debug- symbols clearer than just --strip-native-debug. > > W.r.t. the test: > > > The test currently only runs on Linux > > and requires objcopy to be available. Otherwise the test is being > > skipped. > > We can create a fake objcopy utility for testing purpose > such that the test will validate if the options are passed > properly to the test utility. AFAIK, objcopy is a build requirement for OpenJDK already, so I'm wondering whether it makes sense to stub objcopy for the tests. Perhaps you meant that to be used in addition to existing tests? Anyhow, I'll look into it. It'll likely have to use the '--strip- native-debug-symbols objcopy= > webrev: > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/ > > I haven't reviewed the new files. Just notice that very long > lines in the new files that you may want to fix the formatting > that will help further side-by-side review. Some of the longer lines have been cleaned up in the lates webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/05/webrev/ > The --list-plugin output is very very long. The existing plugins > didn't set a good example to keep the well formatted (I recorded > that they were cleaned up at one point to keep 80 chars width). Right. I'll make sure --list-plugins looks right for --strip-native- debug-symbols once we've agreed on the options. > One other question: should this plugin be moved to linux/classes > rather than unix/classes given that that's the platform it > intends to support? Done in version 05. Thanks, Severin > Mandy > [1] > http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014099.html
Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries
Hi Mandy, Alan, Please find the proposal for CLI option of --strip-native-debug-symbols below. On Fri, 2019-02-08 at 10:53 -0800, Mandy Chung wrote: > > On 2/8/19 2:08 AM, Alan Bateman wrote: > > I agree with Mandy that the CLI options need examination. The proposed > > `--strip-native-debug-symbols` seems reasonable but it will be > > inconsistent with the existing `--strip-debug` option. Mandy - what you > > would think about renaming the existing option to something that makes > > it clear that it strips the debug attribute from class files? (we would > > of course need to do something to keep the existing option working). > > Renaming it to make it clear is good. How about: > > --strip-debug-attribute > --strip-native-debug-symbols > > --strip-debug will invoke both --strip-debug-attribute and > --strip-native-debug-symbols, if supported. > > Typically we want to strip both. If only stripping debug attribute, > then it can use --strip-debug-attribute. > > What do you think? > > > The need to specify the argument as "defaults" or "options" is a bit > > annoying. It might be time to replace hasArguments in the plugin > > > interface to allow for optional arguments. > > Hmm... I thought it allows optional arguments. LegalNoticeFilePlugin > accepts an optional argument. > > On the other hand, pluginToMaps will put an empty map if hasArguments > return false. The plugin processing code (PluginsHelper) is quite > complicated that I can't quite tell without spending time. > > In any case I think a plugin should support optional arguments. > > > The plugin interface is not > > exported/documented/supported so we have flexibility to change it. If we > > do this then it should mean the usages reduce down to: > > > > --strip-native-debug-symbols > > --strip-native-debug-symbols objcopy=:... > > objcopy is fine. > > It would also be nice to allow `keep-debuginfo` taking an optional > file extension. > > --strip-native-debug-symbols keep-debuginfo > --strip-native-debug-symbols keep-debuginfo=dbg The current implementation here has the following options: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/05/webrev/ [i] --strip-native-debug-symbols defaults [ii] --strip-native-debug-symbols options:objcopy-cmd= [iii] --strip-native-debug-symbols options:debuginfo-file-ext= [iv] --strip-native-debug-symbols options:include-debug-syms=true The first option is a work-around for JDK-8218761. AFAIUI, fixing it will need rework of the Plugin interface and probably of the options parsing. Hence, I'd like to defer this post integration of the initial version of --strip-native-debug-symbols plugin. Cases [iii] and [iv] can be folded into one as suggested by Mandy with: --strip-native-debug-symbols keep-debuginfo --strip-native-debug-symbols keep-debuginfo= Case [ii] would become: --strip-native-debug-symbols objcopy= So in summary I'd propose these, where a) and b) may be combined, c) and a) or c) and b) combined would be an error: [a] --strip-native-debug-symbols keep-debuginfo[=] [b] --strip-native-debug-symbols objcopy= [c] --strip-native-debug-symbols defaults As a follow-up to an initial implementation of the above, I'd propose to hook it up with the current --strip-debug by a follow-up patch. It would first rename --strip-debug to --strip-debug-attribute or perhaps --strip-java-debug-symbols, and then let --strip-debug perform java and native debug symbols stripping as Alan suggested. Does that sound reasonable to you? Thanks, Severin
Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries
Hello, On 2019-02-12 09:05, Severin Gehwolf wrote: Hi Erik, On Tue, 2019-02-12 at 08:44 -0800, Erik Joelsson wrote: On 2019-02-12 01:35, Severin Gehwolf wrote: Hi Erik, On Mon, 2019-02-11 at 11:12 -0800, Erik Joelsson wrote: Hello Severin, I think you also need a $(wildcard ) around it, but I may be wrong. Did you try this version? Yes, this version works for me without $(wildcard). Why is it needed? I had to go back and check again to verify, but now I'm sure. The list of directories returned by FindModuleSrcDirs is all src dirs for the module. Not all of them are going to contain the specific directory jdk/tools/jlink/resources. This means SetupCompileProperties will get called with a few non existing directories. While this will work fine, the find implementation on some platforms will complain (Macos in particular), so to avoid introducing confusing warning messages in the build, it's better to filter down the list of directories to those that actually exist. OK, thanks for the explanation. I suppose $(wildcard ...) does that, then? I've added it back locally but I have no way of testing whether this makes any difference, except jdk/submit perhaps? Yes, that is what wildcard does, it filters out any non existing dirs. No need for you to verify anything but that it works as far as I am concerned. I'm happy with the below. /Erik diff --git a/make/gensrc/Gensrc-jdk.jlink.gmk b/make/gensrc/Gensrc-jdk.jlink.gmk --- a/make/gensrc/Gensrc-jdk.jlink.gmk +++ b/make/gensrc/Gensrc-jdk.jlink.gmk @@ -29,8 +29,9 @@ -JLINK_RESOURCES_DIRS := $(addsuffix /jdk/tools/jlink/resources, \ -$(call FindModuleSrcDirs, jdk.jlink)) +# Use wildcard so as to avoid getting non-existing directories back +JLINK_RESOURCES_DIRS := $(wildcard $(addsuffix /jdk/tools/jlink/resources, \ +$(call FindModuleSrcDirs, jdk.jlink))) $(eval $(call SetupCompileProperties, JLINK_PROPERTIES, \ SRC_DIRS := $(JLINK_RESOURCES_DIRS), \ Thanks, Severin Also, please do not indent so much. We have style guidelines [1], which recommend 4 spaces for line break indentation. OK, sorry. Fixed locally. Thanks! /Erik Thanks, Severin /Erik [1] http://openjdk.java.net/groups/build/doc/code-conventions.html On 2019-02-11 10:03, Severin Gehwolf wrote: Hi Erik, On Thu, 2019-02-07 at 17:01 -0800, Erik Joelsson wrote: On 2019-02-07 11:09, Severin Gehwolf wrote: Hi Erik, On Thu, 2019-02-07 at 09:39 -0800, Erik Joelsson wrote: Hello Severin, There is a macro for automatically finding all source dirs for a module. So in Gensrc-jdk.jlink.gmk, I think it would be better expressed using that macro, like this: JLINK_RESOURCE_DIRS := $(wildcard $(addsuffix /jdk/tools/jlink/resources, $(call FindModuleSrcSdirs, jdk.jlink))) The above could/should even be inlined. I've considered this. It seems, though, that FindModuleSrcDirs comes from make/common/Modules.gmk which isn't included in make/gensrc/Gensrc-jdk.jlink.gmk. Given that it has already caused problems with multiple includes of Modules.gmk (JDK-8213736) I was reluctant to include it here too. Without the new include the above won't work. The approach I've taken here seems to be the lesser evil. Thoughts? I appreciate your concern, but JDK-8213736 was a problem in Main.gmk, which is part of where Modules.gmk gets bootstrapped. In any normal makefile (as in where stuff is just being built), the bootstrapping is done and including Modules.gmk is completely fine. Any -.gmk file certainly qualifies here. OK. Updated: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/05/ Thanks, Severin /Erik Thanks, Severin Otherwise build changes look ok. /Erik On 2019-02-07 09:09, Severin Gehwolf wrote: Hi, Could I please get reviews for this enhancement? It adds a debug symbols stripping plug-in to jlink for Linux and Unix platforms. It's the first platform specific jlink plugin and the approach taken for keeping it contained is to use a plugin specific ResourceBundle. Discussion for this happened in [1]. The test uses a native library which should never get debug symbols stripped during the test library build. As such, tiny modifications were needed to make/common/TestFilesCompilation.gmk. Hence, build-dev being on the list for this RFR. The test currently only runs on Linux and requires objcopy to be available. Otherwise the test is being skipped. Example usage of this plugin is described in the bug. webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/ Bug: https://bugs.openjdk.java.net/browse/JDK-8214796 Testing: test/jdk/tools/jlink test/jdk/jdk/modules tests on Linux x86_64 (with good and broken objcopy) and the newly added test. It's currently running through jdk/submit too. Thoughts? Thanks, Severin [1] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014109.html
Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries
Hi Erik, On Tue, 2019-02-12 at 08:44 -0800, Erik Joelsson wrote: > On 2019-02-12 01:35, Severin Gehwolf wrote: > > Hi Erik, > > > > On Mon, 2019-02-11 at 11:12 -0800, Erik Joelsson wrote: > > > Hello Severin, > > > > > > I think you also need a $(wildcard ) around it, but I may be wrong. Did > > > you try this version? > > Yes, this version works for me without $(wildcard). Why is it needed? > > I had to go back and check again to verify, but now I'm sure. The list > of directories returned by FindModuleSrcDirs is all src dirs for the > module. Not all of them are going to contain the specific directory > jdk/tools/jlink/resources. This means SetupCompileProperties will get > called with a few non existing directories. While this will work fine, > the find implementation on some platforms will complain (Macos in > particular), so to avoid introducing confusing warning messages in the > build, it's better to filter down the list of directories to those that > actually exist. OK, thanks for the explanation. I suppose $(wildcard ...) does that, then? I've added it back locally but I have no way of testing whether this makes any difference, except jdk/submit perhaps? diff --git a/make/gensrc/Gensrc-jdk.jlink.gmk b/make/gensrc/Gensrc-jdk.jlink.gmk --- a/make/gensrc/Gensrc-jdk.jlink.gmk +++ b/make/gensrc/Gensrc-jdk.jlink.gmk @@ -29,8 +29,9 @@ -JLINK_RESOURCES_DIRS := $(addsuffix /jdk/tools/jlink/resources, \ -$(call FindModuleSrcDirs, jdk.jlink)) +# Use wildcard so as to avoid getting non-existing directories back +JLINK_RESOURCES_DIRS := $(wildcard $(addsuffix /jdk/tools/jlink/resources, \ +$(call FindModuleSrcDirs, jdk.jlink))) $(eval $(call SetupCompileProperties, JLINK_PROPERTIES, \ SRC_DIRS := $(JLINK_RESOURCES_DIRS), \ Thanks, Severin > > > Also, please do not indent so much. We have style guidelines [1], which > > > recommend 4 spaces for line break indentation. > > OK, sorry. Fixed locally. > > Thanks! > > /Erik > > > Thanks, > > Severin > > > > > /Erik > > > > > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.html > > > > > > On 2019-02-11 10:03, Severin Gehwolf wrote: > > > > Hi Erik, > > > > > > > > On Thu, 2019-02-07 at 17:01 -0800, Erik Joelsson wrote: > > > > > On 2019-02-07 11:09, Severin Gehwolf wrote: > > > > > > Hi Erik, > > > > > > > > > > > > On Thu, 2019-02-07 at 09:39 -0800, Erik Joelsson wrote: > > > > > > > Hello Severin, > > > > > > > > > > > > > > There is a macro for automatically finding all source dirs for a > > > > > > > module. > > > > > > > So in Gensrc-jdk.jlink.gmk, I think it would be better expressed > > > > > > > using > > > > > > > that macro, like this: > > > > > > > > > > > > > > JLINK_RESOURCE_DIRS := $(wildcard $(addsuffix > > > > > > > /jdk/tools/jlink/resources, $(call FindModuleSrcSdirs, > > > > > > > jdk.jlink))) > > > > > > > > > > > > > > The above could/should even be inlined. > > > > > > I've considered this. It seems, though, that FindModuleSrcDirs comes > > > > > > from make/common/Modules.gmk which isn't included in > > > > > > make/gensrc/Gensrc-jdk.jlink.gmk. Given that it has already caused > > > > > > problems with multiple includes of Modules.gmk (JDK-8213736) I was > > > > > > reluctant to include it here too. Without the new include the above > > > > > > won't work. > > > > > > > > > > > > The approach I've taken here seems to be the lesser evil. Thoughts? > > > > > I appreciate your concern, but JDK-8213736 was a problem in Main.gmk, > > > > > which is part of where Modules.gmk gets bootstrapped. In any normal > > > > > makefile (as in where stuff is just being built), the bootstrapping is > > > > > done and including Modules.gmk is completely fine. Any > > > > > -.gmk file certainly qualifies here. > > > > OK. Updated: > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/05/ > > > > > > > > Thanks, > > > > Severin > > > > > > > > > /Erik > > > > > > > > > > > Thanks, > > > > > > Severin > > > > > > > > > > > > > Otherwise build changes look ok. > > > > > > > > > > > > > > /Erik > > > > > > > > > > > > > > On 2019-02-07 09:09, Severin Gehwolf wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > Could I please get reviews for this enhancement? It adds a > > > > > > > > debug > > > > > > > > symbols stripping plug-in to jlink for Linux and Unix > > > > > > > > platforms. It's > > > > > > > > the first platform specific jlink plugin and the approach taken > > > > > > > > for > > > > > > > > keeping it contained is to use a plugin specific > > > > > > > > ResourceBundle. > > > > > > > > Discussion for this happened in [1]. > > > > > > > > > > > > > > > > The test uses a native library which should never get debug > > > > > > > > symbols > > > > > > > > stripped during the test library build. As such, tiny > > > > > > > > modifications > > > > > > > > were needed to make/common/TestFi
Re: RFR: JDK-8218807: Compilation database (compile_commands.json) may contain obsolete items
Looks good. /Erik On 2019-02-12 00:01, Robin Westberg wrote: Hi all, When generating a compilation database by either "make compile-commands" or "make compile-commands-hotspot", every object file that should be built results in a temporary json fragment describing the compiler invocation. However, when the final compile_commands.json file is assembled, all these temporary files are combined, regardless of whether they belong to the current make invocation or were left behind from a previous one. This has the unfortunate effect that "make compile-commands" followed by "make compile-commands-hotspot" generates something very different from an initial invocation of "make compile-commands-hotspot". Also, if a source file has been removed, it will still retain an entry in the final compile_commands.json file until the build directory is cleaned. This will lead to errors if generating an IDE project from the compile_commands.json file. Proposed solution is to always remove all previous temporary json fragments and generate them again. This generation is quite fast and only adds a second or two for repeated invocations of "make compile-commands”, which should be a reasonable tradeoff for ensuring that the compilation database contains correct information. Issue: https://bugs.openjdk.java.net/browse/JDK-8218807 Webrev: http://cr.openjdk.java.net/~rwestberg/8218807/ Testing: make test-make-compile-commands, build windows, linux, mac, solaris Best regards, Robin
Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries
On 2019-02-12 01:35, Severin Gehwolf wrote: Hi Erik, On Mon, 2019-02-11 at 11:12 -0800, Erik Joelsson wrote: Hello Severin, I think you also need a $(wildcard ) around it, but I may be wrong. Did you try this version? Yes, this version works for me without $(wildcard). Why is it needed? I had to go back and check again to verify, but now I'm sure. The list of directories returned by FindModuleSrcDirs is all src dirs for the module. Not all of them are going to contain the specific directory jdk/tools/jlink/resources. This means SetupCompileProperties will get called with a few non existing directories. While this will work fine, the find implementation on some platforms will complain (Macos in particular), so to avoid introducing confusing warning messages in the build, it's better to filter down the list of directories to those that actually exist. Also, please do not indent so much. We have style guidelines [1], which recommend 4 spaces for line break indentation. OK, sorry. Fixed locally. Thanks! /Erik Thanks, Severin /Erik [1] http://openjdk.java.net/groups/build/doc/code-conventions.html On 2019-02-11 10:03, Severin Gehwolf wrote: Hi Erik, On Thu, 2019-02-07 at 17:01 -0800, Erik Joelsson wrote: On 2019-02-07 11:09, Severin Gehwolf wrote: Hi Erik, On Thu, 2019-02-07 at 09:39 -0800, Erik Joelsson wrote: Hello Severin, There is a macro for automatically finding all source dirs for a module. So in Gensrc-jdk.jlink.gmk, I think it would be better expressed using that macro, like this: JLINK_RESOURCE_DIRS := $(wildcard $(addsuffix /jdk/tools/jlink/resources, $(call FindModuleSrcSdirs, jdk.jlink))) The above could/should even be inlined. I've considered this. It seems, though, that FindModuleSrcDirs comes from make/common/Modules.gmk which isn't included in make/gensrc/Gensrc-jdk.jlink.gmk. Given that it has already caused problems with multiple includes of Modules.gmk (JDK-8213736) I was reluctant to include it here too. Without the new include the above won't work. The approach I've taken here seems to be the lesser evil. Thoughts? I appreciate your concern, but JDK-8213736 was a problem in Main.gmk, which is part of where Modules.gmk gets bootstrapped. In any normal makefile (as in where stuff is just being built), the bootstrapping is done and including Modules.gmk is completely fine. Any -.gmk file certainly qualifies here. OK. Updated: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/05/ Thanks, Severin /Erik Thanks, Severin Otherwise build changes look ok. /Erik On 2019-02-07 09:09, Severin Gehwolf wrote: Hi, Could I please get reviews for this enhancement? It adds a debug symbols stripping plug-in to jlink for Linux and Unix platforms. It's the first platform specific jlink plugin and the approach taken for keeping it contained is to use a plugin specific ResourceBundle. Discussion for this happened in [1]. The test uses a native library which should never get debug symbols stripped during the test library build. As such, tiny modifications were needed to make/common/TestFilesCompilation.gmk. Hence, build-dev being on the list for this RFR. The test currently only runs on Linux and requires objcopy to be available. Otherwise the test is being skipped. Example usage of this plugin is described in the bug. webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/ Bug: https://bugs.openjdk.java.net/browse/JDK-8214796 Testing: test/jdk/tools/jlink test/jdk/jdk/modules tests on Linux x86_64 (with good and broken objcopy) and the newly added test. It's currently running through jdk/submit too. Thoughts? Thanks, Severin [1] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014109.html
Re: compiling openJdk 11 on windows 7 32bits fail
On 12/02/2019 14:37, Magnus Ihse Bursie wrote: There has been some fallout due to the mapfile/linking changes made in JDK 12 that affected JNICALL. However, that should not be affecting JDK 11. Wasn't it done in JDK 11? If my memory serves me right, it was done in JDK 11. -- Alexey
Re: RFR: JDK-8218807: Compilation database (compile_commands.json) may contain obsolete items
On 2019-02-12 09:01, Robin Westberg wrote: Hi all, When generating a compilation database by either "make compile-commands" or "make compile-commands-hotspot", every object file that should be built results in a temporary json fragment describing the compiler invocation. However, when the final compile_commands.json file is assembled, all these temporary files are combined, regardless of whether they belong to the current make invocation or were left behind from a previous one. This has the unfortunate effect that "make compile-commands" followed by "make compile-commands-hotspot" generates something very different from an initial invocation of "make compile-commands-hotspot". Also, if a source file has been removed, it will still retain an entry in the final compile_commands.json file until the build directory is cleaned. This will lead to errors if generating an IDE project from the compile_commands.json file. Proposed solution is to always remove all previous temporary json fragments and generate them again. This generation is quite fast and only adds a second or two for repeated invocations of "make compile-commands”, which should be a reasonable tradeoff for ensuring that the compilation database contains correct information. Issue: https://bugs.openjdk.java.net/browse/JDK-8218807 Webrev: http://cr.openjdk.java.net/~rwestberg/8218807/ Looks good to me. /Magnus Testing: make test-make-compile-commands, build windows, linux, mac, solaris Best regards, Robin
Re: compiling openJdk 11 on windows 7 32bits fail
On 2019-02-12 14:38, Alexey Ivanov wrote: On 12/02/2019 04:25, David Holmes wrote: On 12/02/2019 1:45 pm, Franco Gastón Pellegrini wrote: I just downloaded again (to be sure about my errors) JDK 12 and use this command: bash ./configure --enable-debug --disable-warnings-as-errors --with-target-bits=32 --with-toolchain-version=2017 --with-jvm-variants=client --with-boot-jdk="/cygdrive/c/Program Files/Java/jdk-11.0.2/"; and I get: ERROR: Build failed for target 'default (exploded-image)' in configuration 'windows-x86-client-fastdebug' (exit code 2) Stopping sjavac server === Output from failing command(s) repeated here === * For target support_native_java.desktop_libawt_Hashtable.obj: Hashtable.cpp c:/cygwin64/home/franc/java/jdk12/src/java.desktop/windows/native/libawt/windows/Hashtable.cpp(53): error C2664: 'void DTrace_PrintFunction(DTRACE_PRINT_CALLBACK,dtrace_id *,dtrace_id *,const char *,int,int,const char *,...)': cannot convert argument 1 from 'void (__stdcall *)(const char *,int,int,const char *,va_list)' to 'DTRACE_PRINT_CALLBACK' c:/cygwin64/home/franc/java/jdk12/src/java.desktop/windows/native/libawt/windows/Hashtable.cpp(53): note: None of the functions with this name in scope match the target type Maybe CALLBACK / JNICALL macro is missing somewhere… If I understand correctly, a pointer to function with __stdcall calling convention is expected; but DTRACE_PRINT_CALLBACK probably does not have it. There has been some fallout due to the mapfile/linking changes made in JDK 12 that affected JNICALL. However, that should not be affecting JDK 11. /Magnus Does it build successfully if you don't define DEBUG? Regards, Alexey Sorry I can't see anything that would cause that - certainly nothing 32-bit specific. David - ... (rest of output omitted) * All command lines available in /cygdrive/c/cygwin64/home/franc/java/jdk12/build/windows-x86-client-fastdebug/make-support/failure-logs. === End of repeated output === No indication of failed target found. Hint: Try searching the build log for '] Error'. Hint: See doc/building.html#troubleshooting for assistance. make[1]: *** [/home/franc/java/jdk12/make/Init.gmk:310: main] Error 2 make: *** [/home/franc/java/jdk12/make/Init.gmk:186: default] Error 2
Re: compiling openJdk 11 on windows 7 32bits fail
On 12/02/2019 04:25, David Holmes wrote: On 12/02/2019 1:45 pm, Franco Gastón Pellegrini wrote: I just downloaded again (to be sure about my errors) JDK 12 and use this command: bash ./configure --enable-debug --disable-warnings-as-errors --with-target-bits=32 --with-toolchain-version=2017 --with-jvm-variants=client --with-boot-jdk="/cygdrive/c/Program Files/Java/jdk-11.0.2/"; and I get: ERROR: Build failed for target 'default (exploded-image)' in configuration 'windows-x86-client-fastdebug' (exit code 2) Stopping sjavac server === Output from failing command(s) repeated here === * For target support_native_java.desktop_libawt_Hashtable.obj: Hashtable.cpp c:/cygwin64/home/franc/java/jdk12/src/java.desktop/windows/native/libawt/windows/Hashtable.cpp(53): error C2664: 'void DTrace_PrintFunction(DTRACE_PRINT_CALLBACK,dtrace_id *,dtrace_id *,const char *,int,int,const char *,...)': cannot convert argument 1 from 'void (__stdcall *)(const char *,int,int,const char *,va_list)' to 'DTRACE_PRINT_CALLBACK' c:/cygwin64/home/franc/java/jdk12/src/java.desktop/windows/native/libawt/windows/Hashtable.cpp(53): note: None of the functions with this name in scope match the target type Maybe CALLBACK / JNICALL macro is missing somewhere… If I understand correctly, a pointer to function with __stdcall calling convention is expected; but DTRACE_PRINT_CALLBACK probably does not have it. Does it build successfully if you don't define DEBUG? Regards, Alexey Sorry I can't see anything that would cause that - certainly nothing 32-bit specific. David - ... (rest of output omitted) * All command lines available in /cygdrive/c/cygwin64/home/franc/java/jdk12/build/windows-x86-client-fastdebug/make-support/failure-logs. === End of repeated output === No indication of failed target found. Hint: Try searching the build log for '] Error'. Hint: See doc/building.html#troubleshooting for assistance. make[1]: *** [/home/franc/java/jdk12/make/Init.gmk:310: main] Error 2 make: *** [/home/franc/java/jdk12/make/Init.gmk:186: default] Error 2
Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries
Hi Erik, On Mon, 2019-02-11 at 11:12 -0800, Erik Joelsson wrote: > Hello Severin, > > I think you also need a $(wildcard ) around it, but I may be wrong. Did > you try this version? Yes, this version works for me without $(wildcard). Why is it needed? > Also, please do not indent so much. We have style guidelines [1], which > recommend 4 spaces for line break indentation. OK, sorry. Fixed locally. Thanks, Severin > /Erik > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.html > > On 2019-02-11 10:03, Severin Gehwolf wrote: > > Hi Erik, > > > > On Thu, 2019-02-07 at 17:01 -0800, Erik Joelsson wrote: > > > On 2019-02-07 11:09, Severin Gehwolf wrote: > > > > Hi Erik, > > > > > > > > On Thu, 2019-02-07 at 09:39 -0800, Erik Joelsson wrote: > > > > > Hello Severin, > > > > > > > > > > There is a macro for automatically finding all source dirs for a > > > > > module. > > > > > So in Gensrc-jdk.jlink.gmk, I think it would be better expressed using > > > > > that macro, like this: > > > > > > > > > > JLINK_RESOURCE_DIRS := $(wildcard $(addsuffix > > > > > /jdk/tools/jlink/resources, $(call FindModuleSrcSdirs, jdk.jlink))) > > > > > > > > > > The above could/should even be inlined. > > > > I've considered this. It seems, though, that FindModuleSrcDirs comes > > > > from make/common/Modules.gmk which isn't included in > > > > make/gensrc/Gensrc-jdk.jlink.gmk. Given that it has already caused > > > > problems with multiple includes of Modules.gmk (JDK-8213736) I was > > > > reluctant to include it here too. Without the new include the above > > > > won't work. > > > > > > > > The approach I've taken here seems to be the lesser evil. Thoughts? > > > I appreciate your concern, but JDK-8213736 was a problem in Main.gmk, > > > which is part of where Modules.gmk gets bootstrapped. In any normal > > > makefile (as in where stuff is just being built), the bootstrapping is > > > done and including Modules.gmk is completely fine. Any > > > -.gmk file certainly qualifies here. > > OK. Updated: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/05/ > > > > Thanks, > > Severin > > > > > /Erik > > > > > > > Thanks, > > > > Severin > > > > > > > > > Otherwise build changes look ok. > > > > > > > > > > /Erik > > > > > > > > > > On 2019-02-07 09:09, Severin Gehwolf wrote: > > > > > > Hi, > > > > > > > > > > > > Could I please get reviews for this enhancement? It adds a > > > > > > debug > > > > > > symbols stripping plug-in to jlink for Linux and Unix > > > > > > platforms. It's > > > > > > the first platform specific jlink plugin and the approach taken > > > > > > for > > > > > > keeping it contained is to use a plugin specific > > > > > > ResourceBundle. > > > > > > Discussion for this happened in [1]. > > > > > > > > > > > > The test uses a native library which should never get debug > > > > > > symbols > > > > > > stripped during the test library build. As such, tiny > > > > > > modifications > > > > > > were needed to make/common/TestFilesCompilation.gmk. Hence, > > > > > > build-dev > > > > > > being on the list for this RFR. The test currently only runs on > > > > > > Linux > > > > > > and requires objcopy to be available. Otherwise the test is > > > > > > being > > > > > > skipped. > > > > > > > > > > > > Example usage of this plugin is described in the bug. > > > > > > > > > > > > webrev: > > > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/ > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8214796 > > > > > > > > > > > > Testing: test/jdk/tools/jlink test/jdk/jdk/modules tests on > > > > > > Linux > > > > > > x86_64 (with good and broken objcopy) and the newly added test. > > > > > > It's > > > > > > currently running through jdk/submit too. > > > > > > > > > > > > Thoughts? > > > > > > > > > > > > Thanks, > > > > > > Severin > > > > > > > > > > > > [1] > > > > > > http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014109.html > > > > > >
RFR: JDK-8218807: Compilation database (compile_commands.json) may contain obsolete items
Hi all, When generating a compilation database by either "make compile-commands" or "make compile-commands-hotspot", every object file that should be built results in a temporary json fragment describing the compiler invocation. However, when the final compile_commands.json file is assembled, all these temporary files are combined, regardless of whether they belong to the current make invocation or were left behind from a previous one. This has the unfortunate effect that "make compile-commands" followed by "make compile-commands-hotspot" generates something very different from an initial invocation of "make compile-commands-hotspot". Also, if a source file has been removed, it will still retain an entry in the final compile_commands.json file until the build directory is cleaned. This will lead to errors if generating an IDE project from the compile_commands.json file. Proposed solution is to always remove all previous temporary json fragments and generate them again. This generation is quite fast and only adds a second or two for repeated invocations of "make compile-commands”, which should be a reasonable tradeoff for ensuring that the compilation database contains correct information. Issue: https://bugs.openjdk.java.net/browse/JDK-8218807 Webrev: http://cr.openjdk.java.net/~rwestberg/8218807/ Testing: make test-make-compile-commands, build windows, linux, mac, solaris Best regards, Robin
