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


Reply via email to