[patch] fix hotspot build with small SC_ARG_MAX

Tue Sep 21 06:57:18 PDT 2010

Greetings,

This is either in the "you learn something new everyday" category
or it's in the "will this Makefile pain ever stop" category!

Yesterday's comment only change in sa.make didn't pass JPRT.
When this line is indented with a tab:

    # using '$(shell rm ...' instead of using the more traditional

gnumake tries to execute the shell command. Good thing I didn't have
the trailing ')' and good thing I didn't name a real file in there.

Just for the completeness of the record:

   http://cr.openjdk.java.net/~dcubed/6985848-webrev/3/

I exdented (unindented?) the entire new comment block. This
version of the fix passes my local full build and incremental
build on Solaris X86 and I expect it to pass JPRT.

Kelly, I'll be happy if I never look at another Makefile for
a long, long time...

Dan

On 9/20/2010 9:23 AM, Daniel D. Daugherty wrote:
> On 9/19/2010 5:49 PM, Kelly O'Hair wrote:
>>
>> On Sep 19, 2010, at 4:19 PM, Daniel D. Daugherty wrote:
>>
>>> I guess that three line comment before that block doesn't do
>>> the job. :-(  Can you please suggest a change to the comment
>>> to make it more clear.
>>>
>>
>> Oh, ok, looking at it closer....  this particular make rule 
>> evaluation characteristics is a bit unknown.
>> I doubt many people will understand what is happening here.
>> I guess I would have said something more to the point for the typical 
>> person:
>>   "The '$(shell rm)' instead of $(RM) here is critical due to the 
>> foreach use in the rules."
>
> I've rewritten the entire comment block. It should
> be more clear now. Please let me know if you agree:
>
> http://cr.openjdk.java.net/~dcubed/6985848-webrev/2/
>
>
>>
>>> The file lists are generated at variable expansion time so we
>>> have to do the remove at variable expansion time also. The
>>> round 0 fix for 6985848 did the remove as a part of rule
>>> execution, but that's too late and causes the javac to fail.
>>
>> The fact that AGENT_FILES1 and AGENT_FILES2 contains wildcards. I'm 
>> suspecting a simple:
>> find $(AGENT_SRC_DIR) -type f -name \*.java > list
>> would work just as well, assuming the agent sources in the repository 
>> all need to be built.
>> But I know you are just trying to fix one issue and not redesign the 
>> sa makefiles.
>
> Redesigning the SA makefiles is tempting, but definitely not
> high on my priority list.
>
>
>> Your change should work.
>
> It made it through a test JPRT job over the weekend and I verified
> that an incremental build did not rebuild sa-jdi.jar on my machine
> here in Colorado.
>
> Dan
>
>
>>
>> -kto
>>
>>>
>>> Dan
>>>
>>>
>>> On 9/19/2010 2:08 PM, Kelly O'Hair wrote:
>>>> Why are you using
>>>>    $(shell rm -f -r ... )
>>>> instead of
>>>>   $(RM) -r ...
>>>>
>>>> ???
>>>>
>>>>
>>>> On Sep 17, 2010, at 9:49 PM, Daniel D. Daugherty wrote:
>>>>
>>>>> I messed up the rule expansion... :-(
>>>>>
>>>>> Here's another shot:
>>>>>
>>>>> http://cr.openjdk.java.net/~dcubed/6985848-webrev/1/
>>>>>
>>>>> This time I added a comment so the subtle difference
>>>>> between macro/variable expansion versus rule execution
>>>>> is a little more clear. I hope...
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>>
>>>>> On 9/17/2010 2:31 PM, Daniel D. Daugherty wrote:
>>>>>> Greetings,
>>>>>>
>>>>>> The fix for this bug (6561870) has caused the sa-jdi.jar file to
>>>>>> always be rebuilt. I have a minor tweak that fixes that problem:
>>>>>>
>>>>>>  http://cr.openjdk.java.net/~dcubed/6985848-webrev/0/
>>>>>>
>>>>>> Thanks to Coleen for spotting this issue!
>>>>>>
>>>>>> The folks on the "To" list were on the original review team
>>>>>> for 6561870. I would like to hear from at least two of those
>>>>>> folks on this tweak.
>>>>>>
>>>>>> Thanks, in advance, for the reviews!
>>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 9/3/2010 6:12 PM, Daniel D. Daugherty wrote:
>>>>>>> Here is the revised webrev:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dcubed/6561870-webrev/1/
>>>>>>>
>>>>>>> One reviewer will do, I think...
>>>>>>>
>>>>>>> Dan
>>>>>>>
>>>>>>>
>>>>>>> On 9/1/2010 12:17 PM, Tom Rodriguez wrote:
>>>>>>>> Can the agent list files go into $(GENERATED) instead of 
>>>>>>>> $(TOPDIR)?  Otherwise this looks good.
>>>>>>>>
>>>>>>>> tom
>>>>>>>>
>>>>>>>> On Sep 1, 2010, at 10:55 AM, Daniel D. Daugherty wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> I'm finally getting back to this thread. Since I'm also applying
>>>>>>>>> Matthias' changes to the Solaris sa.makefile, I figured I would
>>>>>>>>> send out a webrev:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~dcubed/6561870-webrev/0/
>>>>>>>>>
>>>>>>>>> Kelly and Christian, can I get re-reviews from the two of you?
>>>>>>>>>
>>>>>>>>> Matthias, can you verify that I got the Solaris port of your
>>>>>>>>> changes correct?
>>>>>>>>>
>>>>>>>>> Thanks, in advance, for any reviews...
>>>>>>>>>
>>>>>>>>> Dan
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 8/25/2010 10:22 AM, Daniel D. Daugherty wrote:
>>>>>>>>>
>>>>>>>>>> On 8/25/2010 10:11 AM, Christian Thalinger wrote:
>>>>>>>>>>
>>>>>>>>>>> On Wed, 2010-08-25 at 10:00 -0600, Daniel D. Daugherty wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> Thanks for the reviews Kelly and Christian.
>>>>>>>>>>>>
>>>>>>>>>>>> Is there a reason to not apply these changes to the solaris 
>>>>>>>>>>>> and
>>>>>>>>>>>> windows versions also? I haven't looked yet, but I figured I
>>>>>>>>>>>> would ask...
>>>>>>>>>>>>
>>>>>>>>>>> Yeah, I also thought about this for Solaris and it should 
>>>>>>>>>>> work there as
>>>>>>>>>>> on Linux.  Don't know about Windows, though.
>>>>>>>>>>>
>>>>>>>>>> Looks like Windows already uses a "make" construct instead of 
>>>>>>>>>> running
>>>>>>>>>> into the shell command line limitation:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> $(GENERATED)\sa-jdi.jar: $(AGENT_FILES1:/=\) 
>>>>>>>>>>> $(AGENT_FILES2:/=\)
>>>>>>>>>>>
>>>>>>>>>> <snip>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>      @$(COMPILE_JAVAC) -source 1.4 -target 1.4 -classpath 
>>>>>>>>>>> $(SA_CLASSPATH) -so
>>>>>>>>>>> urcepath $(AGENT_SRC_DIR) -d $(SA_CLASSDIR) $(AGENT_FILES1:/=\)
>>>>>>>>>>>      @$(COMPILE_JAVAC) -source 1.4 -target 1.4 -classpath 
>>>>>>>>>>> $(SA_CLASSPATH) -so
>>>>>>>>>>> urcepath $(AGENT_SRC_DIR) -d $(SA_CLASSDIR) $(AGENT_FILES2:/=\)
>>>>>>>>>>>
>>>>>>>>>> I also think this is an "nmake" Makefile instead of a GNU 
>>>>>>>>>> Makefile.
>>>>>>>>>> I think I'll look at applying the changes to the Solaris 
>>>>>>>>>> Makefile and
>>>>>>>>>> leave the Windows stuff alone :-)
>>>>>>>>>>
>>>>>>>>>> Dan
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
>