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

[Magnus Ihse Bursie]

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
>