[patch] fix hotspot build with small SC_ARG_MAX
Daniel D. Daugherty
daniel.daugherty at oracle.com
Mon Sep 20 13:19:43 PDT 2010
Thanks for the review! And thanks for reviewing multiple rounds
of _both_ fixes... Did I mention how I hate Makefile changes
recently? :-)
Dan
On 9/20/2010 2:11 PM, Tom Rodriguez wrote:
> Looks fine.
>
> tom
>
> On Sep 20, 2010, at 8: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
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>
>
More information about the hotspot-dev
mailing list