[patch] fix hotspot build with small SC_ARG_MAX
Kelly O'Hair
kelly.ohair at oracle.com
Mon Sep 20 13:25:40 PDT 2010
Darn, and I was going to nominate you to complete re-write all the
hotspot makefiles. ;^>
-kto
On Sep 20, 2010, at 1:19 PM, Daniel D. Daugherty wrote:
> 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