RFR (XXS) [URGENT]: 8048232: Fix for 8046471 breaks PPC64 build

Wed Jul 2 23:17:33 UTC 2014

Alejandro,

I pushed to RT_Baseline. Didn't want to by-pass the usual process
without a darn good reason...

Dan

On 7/2/14 4:50 PM, Alejandro E Murillo wrote:
>
> On 7/2/2014 2:46 PM, Daniel D. Daugherty wrote:
>> Hi Volker,
>>
>> Yes, I can sponsor this change also.
>>
>> > http://cr.openjdk.java.net/~simonis/webrevs/8048232.v2/
>>
>> make/linux/Makefile
>>     No comments.
>>
>> make/linux/makefiles/defs.make
>>     No comments.
>>
>> Thumbs up!
>>
>> I also see this below:
>>
>> > Please push this right to http://hg.openjdk.java.net/jdk9/hs/hotspot
>> > in order to get it into http://hg.openjdk.java.net/jdk9/dev/hotspot
>> > together with 8046471.
>>
>> However, I don't see an approval from Alejandro on this e-mail thread
> I missed that. Please do not push changes straight to jdk9/hs/hotspot
> or jdk9/dev/hotspot. We recently had problems by doing so.
> Even small changes need to go through nightly, unless it is an 
> emergency fix.
> We risk introducing further problems
>> nor is it possible to catch up to the fix for 8046471 since it was
>> included in the 2014-06-27 Main_Baseline snapshot that should get
>> pushed to JDK9-dev soon.
> There was a problem preventing the integration of last week snapshot 
> into jdk9/dev,
> so 8046471 should be in jdk9/dev next week
>
> Alejandro
>>
>> My current plan is to push the fix to RT_Baseline and follow the
>> normal process.
>>
>> Dan
>>
>>
>> On 7/2/14 12:27 PM, Volker Simonis wrote:
>>> Hi Daniel,
>>>
>>> I saw that you've sponsored 8046471 which unfortunately broke our 
>>> PPC64 build.
>>>
>>> Could you please be so kind to also review and sponsor this tiny
>>> little change which fixes the problems on PPC64.
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>>
>>> On Tue, Jul 1, 2014 at 2:33 PM, Volker Simonis 
>>> <volker.simonis at gmail.com> wrote:
>>>> Hi Mikael,
>>>>
>>>> thanks for reviewing at the change.
>>>>
>>>> Can I please have one more reviewer/sponsor for this tiny change?
>>>>
>>>> Thanks,
>>>> Volker
>>>>
>>>>
>>>> On Tue, Jul 1, 2014 at 2:11 AM, Mikael Vidstedt
>>>> <mikael.vidstedt at oracle.com> wrote:
>>>>> Looks good.
>>>>>
>>>>> Cheers,
>>>>> Mikael
>>>>>
>>>>>
>>>>> On 2014-06-30 07:28, Volker Simonis wrote:
>>>>>> Can somebody please review and push this small build change to 
>>>>>> fix our
>>>>>> ppc64 build errors.
>>>>>>
>>>>>> Thanks,
>>>>>> Volker
>>>>>>
>>>>>> On Fri, Jun 27, 2014 at 5:48 PM, Volker Simonis
>>>>>> <volker.simonis at gmail.com> wrote:
>>>>>>> On Thu, Jun 26, 2014 at 10:59 PM, Volker Simonis
>>>>>>> <volker.simonis at gmail.com> wrote:
>>>>>>>>
>>>>>>>> On Thursday, June 26, 2014, Mikael Vidstedt 
>>>>>>>> <mikael.vidstedt at oracle.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> This will work for top level builds. For Hotspot-only builds 
>>>>>>>>> ARCH will
>>>>>>>>> (still) be the value of uname -m, so if you want to support
>>>>>>>>> Hotspot-only
>>>>>>>>> builds you'll probably want to do the "ifneq (,$(findstring 
>>>>>>>>> $(ARCH),
>>>>>>>>> ppc))"
>>>>>>>>> trick to catch both "ppc" (which is what a top level build 
>>>>>>>>> will use)
>>>>>>>>> and
>>>>>>>>> "ppc64" (for Hotspot-only).
>>>>>>>>>
>>>>>>>> Hi Mikael,
>>>>>>>>
>>>>>>>> yes you're right.
>>>>>>> I have to correct myself - you're nearly right:)
>>>>>>>
>>>>>>> In the term "$(findstring $(ARCH), ppc)" '$ARCH' is the needle and
>>>>>>> 'ppc is the stack, so it won't catch 'ppc64' either. I could write
>>>>>>> "$(findstring ppc, $(ARCH))" which would catch both, 'ppc' and 
>>>>>>> 'ppc64'
>>>>>>> but I decided to use the slightly more verbose "$(findstring 
>>>>>>> $(ARCH),
>>>>>>> ppc ppc64)" because it seemed clearer to me. I also added a 
>>>>>>> comment to
>>>>>>> explain the problematic of the different ARCH values for 
>>>>>>> top-level and
>>>>>>> HotSpot-only builds. Once we have the new HS build, this can 
>>>>>>> hopefully
>>>>>>> all go away.
>>>>>>>
>>>>>>> By, the way, I also had to apply this change to your 
>>>>>>> ppc-modifications
>>>>>>> in make/linux/makefiles/defs.make. And I think that the same 
>>>>>>> reasoning
>>>>>>> may also apply to "$(findstring $(ARCH), sparc)" which won't catch
>>>>>>> 'sparc64' any more after your change but I have no Linux/SPARC 
>>>>>>> box to
>>>>>>> test this. You may change it accordingly at your discretion.
>>>>>>>
>>>>>>> So here's the new webrev:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/8048232.v2/
>>>>>>>
>>>>>>> Please review and sponsor:)
>>>>>>>
>>>>>>> Thank you and best regards,
>>>>>>> Volker
>>>>>>>
>>>>>>>> I only tested a complete make but I indeed want to support
>>>>>>>> HotSpot only makes as well. I'll change it as requested 
>>>>>>>> although I won't
>>>>>>>> have chance to do that before tomorrow morning (European time).
>>>>>>>>
>>>>>>>> Thanks you and best regards,
>>>>>>>> Volker
>>>>>>>>
>>>>>>>>> Sorry for breaking it.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Mikael
>>>>>>>>>
>>>>>>>>> PS. We so need to clean up these makefiles...
>>>>>>>>>
>>>>>>>>> On 2014-06-26 07:25, Volker Simonis wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> could somebody please review and push the following tiny change:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/8048232/
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8048232
>>>>>>>>>>
>>>>>>>>>> It fixes the build on Linux/PPC64 after "8046471 Use
>>>>>>>>>> OPENJDK_TARGET_CPU_ARCH instead of legacy value for hotspot 
>>>>>>>>>> ARCH".
>>>>>>>>>>
>>>>>>>>>> Before 8046471, the top-level make passed ARCH=ppc64 to the 
>>>>>>>>>> HotSpot
>>>>>>>>>> make. After 8046471, it now passes ARCH=ppc. But there was 
>>>>>>>>>> one place
>>>>>>>>>> in  make/linux/Makefile which checked for ARCH=ppc64 in order to
>>>>>>>>>> disable the TIERED build. This place has to be adapted to 
>>>>>>>>>> handle the
>>>>>>>>>> new ARCH value.
>>>>>>>>>>
>>>>>>>>>> Please push this right to 
>>>>>>>>>> http://hg.openjdk.java.net/jdk9/hs/hotspot
>>>>>>>>>> in order to get it into 
>>>>>>>>>> http://hg.openjdk.java.net/jdk9/dev/hotspot
>>>>>>>>>> together with 8046471.
>>>>>>>>>>
>>>>>>>>>> Note: this change depends on 8046471 in the hotspot AND in the
>>>>>>>>>> top-level directory!
>>>>>>>>>>
>>>>>>>>>> Thank you and best regards,
>>>>>>>>>> Volker
>>>>>>>>>
>>
>