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 > > > <phase>-<module>.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 > > > > > >
