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

[Erik Joelsson]

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