RFR: JDK-8142336: Convert the SA agent build to modular build-infra makefiles

Dmitry Samersoff dmitry.samersoff at oracle.com
Tue Nov 24 11:11:48 UTC 2015


Looks good for me!


On 2015-11-17 12:47, Erik Joelsson wrote:
> Hello,
> I realized that there already was a mechanism for controlling the
> inclusion of SA from configure. Unfortunately, this variable is not
> picked up by any makefile currently. I changed the explicit checks for
> aix-ppc and zero to instead use the variable INCLUDE_SA which configure
> sets up based on at least these conditions.
> I also removed another lingering special case instance of the
> jdk.hotspot.agent module in Main.gmk.
> New webrev: http://cr.openjdk.java.net/~erikj/8142336/webrev.03/
> /Erik
> On 2015-11-13 11:39, Erik Joelsson wrote:
>> Widening the distribution in the hopes of finding another reviewer.
>> I've fixed the formatting in hotspot/make/bsd/makefiles/defs.make
>> locally. Merge tool error.
>> /Erik
>> On 2015-11-11 17:43, Magnus Ihse Bursie wrote:
>>> 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

Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the source code.

More information about the hotspot-dev mailing list