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