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

[Magnus Ihse Bursie]

On 2015-11-17 10: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/

Looks good to me!

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