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

Thu Jul 3 07:04:16 UTC 2014

Hi Daniel,

thanks a lot for pushing the change.

Regards,
Volker

On Thu, Jul 3, 2014 at 1:17 AM, Daniel D. Daugherty
<daniel.daugherty at oracle.com> wrote:
> 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
>>>>>>>>>>
>>>>>>>>>>
>>>
>>
>