On 2015-11-11 10:31, Erik Joelsson wrote:
New webrev: http://cr.openjdk.java.net/~erikj/8142336/webrev.02/
Fixed the issues listed below. Reverted the faulty attempt at fixing a
warning. Did a more thorough attempt at clearing out all references to
SA in the old makefiles.
Looks great. There was a few lines in
hotspot/make/bsd/makefiles/defs.make where you seem to have accidentally
broken the indentation. Apart from this it looks good. If you just
restore the indentation I'm okay (you don't need to respin the webrev
for that).
/Magnus
/Erik
On 2015-11-10 14:49, Magnus Ihse Bursie wrote:
On 2015-11-10 11:39, Magnus Ihse Bursie wrote:
On 2015-11-09 19:33, Erik Joelsson wrote:
Hello,
As a stepping stone in the hotspot makefile conversion, I have
broken out the serviceability agent separately and converted it
into proper modular build-infra makefiles. Doing this conversion
separately has some value on its own by reducing the special cases
currently needed for building the jdk.hotspot.agent module.
The current SA java build compiles with the boot jdk javac with
-source/-target JDK N-1. The proposed change instead builds SA with
the interim-langtools javac for JDK N, like all the rest of the JDK
classes.
There is already a bug filed for reorganizing the source of the SA
agent to conform to the Jigsaw style modular source layout:
JDK-8067194, so I have left the source in its current location.
The native compilation and linking has been changed to base the
flags used on what configure sets up for the other JDK libraries.
This has caused some changes in flag usage. From what I can tell,
nothing important is different however. I have run the relevant
jtreg tests on all OSes to verify that it still works. Some of the
differences include:
* Linux: "-Xlinker z -Xlinker defs" was added to LDFLAGS, which
causes link failure unless "-ldl" was also added to LIBS.
* Solaris: More warnings activated through "+w" caused need for
disabling some warnings. I fixed one warning instance which was
trivial (int->size_t), but couldn't figure out the rest. I will
file a followup bug for fixing those if this patch is accepted.
I tried to mimic the current behavior of excluding SA on linux-ppc
and zero that Volker added a while back. Now it's excluded on the
module level instead so that jdk.hotspot.agent isn't even built in
that case. It would be good if this could be tested.
Bug: https://bugs.openjdk.java.net/browse/JDK-8142336
Webrev: http://cr.openjdk.java.net/~erikj/8142336/webrev.01/
A few remarks:
* Could you please document the new DISABLED_WARNINGS_CXX and
DISABLED_WARNINGS_C in the function header?
* I believe the use of {} here was to signify a set. When only jsig
remains, it just looks strange:
-# SYMFLAG is used by {jsig,saproc}.make
+# SYMFLAG is used by {jsig}.make
* The logic of setting up "--hash-style=both" is already done in
configure for LDFLAGS_JDKLIB, so you do not need to repeat it, if
you include the LDFLAGS_JDKLIB variable.
Also, SA_WINDOWS_LDFLAGS is read but never defined.
/Magnus