[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 6:

> Was able to run Impala exhaustive tests with this toolchain
 > version.
 > 
 > Thanks Valencia!

Thanks, Tim! I'm glad to see the ppc64le patch in the native-toolchain master 
branch.

Once again, thanks to Tim/Matthew/Jim for your co-operation throughout the 
review process.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 6
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Tim Armstrong (Code Review)]

Tim Armstrong has submitted this change and it was merged.

Change subject: Ported native-toolchain to work on ppc64le
..


Ported native-toolchain to work on ppc64le

Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
---
M buildall.sh
M functions.sh
M init-compiler.sh
M init.sh
M source/autoconf/build.sh
A 
source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch
A 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0001-Build-crcutil-440ba7b-on-ppc64le.patch
M source/cyrus-sasl/build.sh
M source/glog/build.sh
M source/kudu/build.sh
M source/libtool/build.sh
M source/libunwind/build.sh
A 
source/libunwind/libunwind-1.1-patches/0001-Build-libunwind-1.1-on-ppc64le.patch
M source/llvm/build-3.3.sh
M source/llvm/build-source-tarball.sh
M source/openldap/build.sh
M source/openssl/build.sh
M source/thrift/build.sh
18 files changed, 702 insertions(+), 19 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved; Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 6
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 6: Code-Review+2 Verified+1

Was able to run Impala exhaustive tests with this toolchain version.

Thanks Valencia!

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 6
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 6:

I'm just building and running tests on x86. So far so good. Will +2 and submit 
once the tests are done.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 6
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 5:

> nice! that's a good result. Thanks, Valencia.
 > 
 > I have no other comments. Let's let Tim give the final +2.

Thanks, Matthew!

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 5: Code-Review+1

nice! that's a good result. Thanks, Valencia.

I have no other comments. Let's let Tim give the final +2.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 5:

(1 comment)

Thanks, Matthew/Jim.

http://gerrit.cloudera.org:8080/#/c/6468/5/source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch
File 
source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch:

Line 1: Only in breakpad-88e-p3/: my_breakpad
> I was hoping we could compare the sha1sum of the binary before/after the ch
I did a sha1sum check of binaries in breakpad's bin folder with and without 
ppc64le changes. 
>From a total of 9 binaries, sha1sum matches for 7 binaries and differs for 2 
>binaries(core2md, minidump-2-core).


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 5:

> (1 comment)

Thanks Jim, I do realize that so like I said, I wouldn't hold this up if it 
doesn't match up. That said, if it does match then we know it's not affected.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6468/5/source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch
File 
source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch:

Line 1: Only in breakpad-88e-p3/: my_breakpad
> I was hoping we could compare the sha1sum of the binary before/after the ch
SOme projects don't have reproducible builds:

https://reproducible-builds.org/

https://wiki.debian.org/ReproducibleBuilds

For projects like that, the sha1sum can change even without changing the source 
code, but by just building at a different time.


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 5:

(3 comments)

Thanks, Matthew!

http://gerrit.cloudera.org:8080/#/c/6468/5/buildall.sh
File buildall.sh:

PS5, Line 101: if (( BUILD_HISTORICAL )) ; then
 :   CRCUTIL_VERSION=440ba7babeff77ffad992df3a10c767f184e946e\
 : $SOURCE_DIR/source/crcutil/build.sh
 : fi
> sorry, ignore my comment. I thought I had made this change but hadn't yet.
It's ok.


http://gerrit.cloudera.org:8080/#/c/6468/5/source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch
File 
source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch:

Line 1: Only in breakpad-88e-p3/: my_breakpad
> there should be no changes to the x86 binary, right? would it be possible t
I checked if any interference, of ppc64le changes in x86 code, existed by 
building the same code base on ppc64le and x86. It built smoothly on both. 
However, please let me know if there is any other way I could verify the 
binaries.


http://gerrit.cloudera.org:8080/#/c/6468/5/source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0001-Build-crcutil-440ba7b-on-ppc64le.patch
File 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0001-Build-crcutil-440ba7b-on-ppc64le.patch:

Line 1: diff -urx '*.m4' 
crcutil-440ba7babeff77ffad992df3a10c767f184e946e_up/autogen.sh 
crcutil-440ba7babeff77ffad992df3a10c767f184e946e/autogen.sh
> same thing about not affecting the x86 binary?
I checked if any interference, of ppc64le changes in x86 code, existed by 
building the same code base on ppc64le and x86. It built smoothly on both. 
However, please let me know if there is any other way I could verify the 
binaries.


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6468/5/buildall.sh
File buildall.sh:

PS5, Line 101: if (( BUILD_HISTORICAL )) ; then
 :   CRCUTIL_VERSION=440ba7babeff77ffad992df3a10c767f184e946e\
 : $SOURCE_DIR/source/crcutil/build.sh
 : fi
> we stopped doing the BUILD_HISTORICAL thing, you can remove this unless we 
sorry, ignore my comment. I thought I had made this change but hadn't yet.


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 5:

(3 comments)

looking much better, thank you Valencia!

http://gerrit.cloudera.org:8080/#/c/6468/5/buildall.sh
File buildall.sh:

PS5, Line 101: if (( BUILD_HISTORICAL )) ; then
 :   CRCUTIL_VERSION=440ba7babeff77ffad992df3a10c767f184e946e\
 : $SOURCE_DIR/source/crcutil/build.sh
 : fi
we stopped doing the BUILD_HISTORICAL thing, you can remove this unless we need 
both versions, but if that's the case then it shouldn't be conditional anyway.


http://gerrit.cloudera.org:8080/#/c/6468/5/source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch
File 
source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch:

Line 1: Only in breakpad-88e-p3/: my_breakpad
there should be no changes to the x86 binary, right? would it be possible to 
verify that?


http://gerrit.cloudera.org:8080/#/c/6468/5/source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0001-Build-crcutil-440ba7b-on-ppc64le.patch
File 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0001-Build-crcutil-440ba7b-on-ppc64le.patch:

Line 1: diff -urx '*.m4' 
crcutil-440ba7babeff77ffad992df3a10c767f184e946e_up/autogen.sh 
crcutil-440ba7babeff77ffad992df3a10c767f184e946e/autogen.sh
same thing about not affecting the x86 binary?


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 4:

(1 comment)

Thanks, Tim!

I'd like to inform you that I've initialized the new CONFIGURE_FLAG_BUILD_SYS 
in the init.sh and removed its earlier initialization in the individual scripts 
wherever possible.

Yes. I have executed the build on ppc64le as well as x86 and it works smoothly.

http://gerrit.cloudera.org:8080/#/c/6468/4/buildall.sh
File buildall.sh:

Line 105: CRCUTIL_VERSION=440ba7babeff77ffad992df3a10c767f184e946e-p1\
> Can you add the old versions back in, just inside an "if ((BUILD_HISTORICAL
Sure. Done.


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 4
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has uploaded a new patch set (#5).

Change subject: Ported native-toolchain to work on ppc64le
..

Ported native-toolchain to work on ppc64le

Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
---
M buildall.sh
M functions.sh
M init-compiler.sh
M init.sh
M source/autoconf/build.sh
A 
source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch
A 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0001-Build-crcutil-440ba7b-on-ppc64le.patch
M source/cyrus-sasl/build.sh
M source/glog/build.sh
M source/kudu/build.sh
M source/libtool/build.sh
M source/libunwind/build.sh
A 
source/libunwind/libunwind-1.1-patches/0001-Build-libunwind-1.1-on-ppc64le.patch
M source/llvm/build-3.3.sh
M source/llvm/build-source-tarball.sh
M source/openldap/build.sh
M source/openssl/build.sh
M source/thrift/build.sh
18 files changed, 702 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/68/6468/5
-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 5
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 4:

(1 comment)

Thanks for the changes. The code changes look good aside from one minor comment.

After that I'll need to do a build on x86 and just do some tests to make sure 
nothing broke there. If you can just confirm on your own that it builds on x86 
ok that would be good - it will save us a round-trip if there is anything wrong.

http://gerrit.cloudera.org:8080/#/c/6468/4/buildall.sh
File buildall.sh:

Line 105: CRCUTIL_VERSION=440ba7babeff77ffad992df3a10c767f184e946e-p1\
Can you add the old versions back in, just inside an "if ((BUILD_HISTORICAL))". 
The idea is that we can generate all of the old versions of things if needed.


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 4
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 2:

(14 comments)

Thanks for the comments on the patchset, Tim!

I've worked on the points you mentioned. Please let me know if any issues.

http://gerrit.cloudera.org:8080/#/c/6468/3/buildall.sh
File buildall.sh:

Line 106:   $SOURCE_DIR/source/crcutil/build.sh
> The -p2 patch should work for x86-64 too, right? Let's just bump the versio
Resolved. Necessary fix done in the ppc patch for crcutil


Line 262: 

> The control flow here changed for x86-64 - now if BUILD_HISTORICAL is true,
Resolved. Necessary fix done in ppc patches for breakpad.


Line 263: FLATBUFFERS_VERSION=1.6.0 $SOURCE_DIR/source/flatbuffers/build.sh
> Same as above - let's just bump the version for both platforms.
Ok


http://gerrit.cloudera.org:8080/#/c/6468/3/functions.sh
File functions.sh:

PS3, Line 305: o
> This logic is pretty weird - the patches will be numbered differently on di
The idea behind this was to apply the ppc patches only if arch is ppc.
However, as I have fixed the patches to be generic, I have already removed this 
'if' condition


http://gerrit.cloudera.org:8080/#/c/6468/3/source/breakpad/breakpad-88e5b2c8806bac3f2c80d2fe80094be5bd371601-patches/0003-Build-breakpad_88e5b2c-on-ppc64le.patch
File 
source/breakpad/breakpad-88e5b2c8806bac3f2c80d2fe80094be5bd371601-patches/0003-Build-breakpad_88e5b2c-on-ppc64le.patch:

> I really encourage you to get this patch into upstream breakpad. The proble
I understand your point. I've already started working on the breakpad 
upstreaming activity. 

Thanks for bringing this to my notice. I've fixed the issues and it runs 
smoothly now on x86 even when ppc patch is applied


http://gerrit.cloudera.org:8080/#/c/6468/3/source/breakpad/breakpad-88e5b2c_ppc.patch
File source/breakpad/breakpad-88e5b2c_ppc.patch:

> Was this accidentally checked in?
Yes.


http://gerrit.cloudera.org:8080/#/c/6468/3/source/crcutil/build.sh
File source/crcutil/build.sh:

Line 36: 
> Don't need this change
Ok. Removed it.


http://gerrit.cloudera.org:8080/#/c/6468/3/source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0002_crc_interface_cc.patch
File 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0002_crc_interface_cc.patch:

Line 7
> If I understand correctly, this is just a hack to remove code that doesn't 
I've fixed the issue that required this conditional. The conditional is no 
longer required.


Line 12
> The second return should be in a #else, right? Otherwise the function has t
With the issue now fixed with the new crcutil patch that I've uploaded, the 
second return is removed.


http://gerrit.cloudera.org:8080/#/c/6468/3/source/kudu/build.sh
File source/kudu/build.sh:

Line 93:  # Removing trailing slash
> Can you revert the formatting changes? it just makes it harder to see where
Done.


http://gerrit.cloudera.org:8080/#/c/6468/3/source/libunwind/build.sh
File source/libunwind/build.sh:

Line 35:   autoreconf -i
> This could be a patch too, right?
Sure. Done.

No problem. Thanks!


http://gerrit.cloudera.org:8080/#/c/6468/3/source/llvm/build-3.3.sh
File source/llvm/build-3.3.sh:

PS3, Line 85:   wrap ../llvm-
I noticed that CONFIGURE_FLAGS var has many ocurrences in the native-toolchain 
code, therefore, I'll be using the var 'CONFIGURE_FLAG_BUILD_SYS' instead of 
'CONFIGURE_FLAGS' for setting the ppc-specific config flags.


Line 87: --prefix=$LOCAL_INSTALL --with-pic $EXTRA_CONFIG_ARG \
> nit: don't need TARGET= above, can just use TARGET= here and on line 88
Ok.
I noticed that TARGET var has many occurences throughout native-toolchain code, 
therefore, I have used a new var 'LLVM_BUILD_TARGET' instead of 'TARGET' var.


http://gerrit.cloudera.org:8080/#/c/6468/3/source/llvm/build-source-tarball.sh
File source/llvm/build-source-tarball.sh:

Line 79:   # Invoke CMake with the correct configuration
> nit: don't need TARGET= above, can just use TARGET= here and on line 82
Ok.
I noticed that TARGET var has many occurences throughout native-toolchain code, 
therefore, I have used a new var 'LLVM_BUILD_TARGET' instead of 'TARGET' var.


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has uploaded a new patch set (#4).

Change subject: Ported native-toolchain to work on ppc64le
..

Ported native-toolchain to work on ppc64le

Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
---
M buildall.sh
M functions.sh
M init-compiler.sh
M init.sh
M source/autoconf/build.sh
A 
source/breakpad/breakpad-ffe3e478657dc7126fca6329dfcedc49f4c726d9-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch
A 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0001-Build-crcutil-440ba7b-on-ppc64le.patch
M source/cyrus-sasl/build.sh
M source/glog/build.sh
M source/kudu/build.sh
M source/libevent/build.sh
M source/libtool/build.sh
M source/libunwind/build.sh
A 
source/libunwind/libunwind-1.1-patches/0001-Build-libunwind-1.1-on-ppc64le.patch
M source/llvm/build-3.3.sh
M source/llvm/build-source-tarball.sh
M source/openldap/build.sh
M source/openssl/build.sh
M source/thrift/build.sh
19 files changed, 729 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/68/6468/4
-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 4
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 3:

(13 comments)

Sorry for the slow turnaround - was on holiday for a couple of weeks. I think 
we're converging here to something that will be more maintainable.

http://gerrit.cloudera.org:8080/#/c/6468/3/buildall.sh
File buildall.sh:

Line 106:   CRCUTIL_VERSION=440ba7babeff77ffad992df3a10c767f184e946e-p2\
The -p2 patch should work for x86-64 too, right? Let's just bump the version 
for both platforms and move the old version under "BUILD_HISTORICAL" like some 
of the examples above.


Line 262: elif [[ "$ARCH_NAME" == "ppc64le" ]]; then
The control flow here changed for x86-64 - now if BUILD_HISTORICAL is true, we 
don't build -p2.


Line 263:   BREAKPAD_VERSION=88e5b2c8806bac3f2c80d2fe80094be5bd371601-p3 
$SOURCE_DIR/source/breakpad/build.sh
Same as above - let's just bump the version for both platforms.


http://gerrit.cloudera.org:8080/#/c/6468/3/functions.sh
File functions.sh:

PS3, Line 305: [
This logic is pretty weird - the patches will be numbered differently on 
different architectures.

Actually is this even needed any more - can we just remove it?


http://gerrit.cloudera.org:8080/#/c/6468/3/source/breakpad/breakpad-88e5b2c8806bac3f2c80d2fe80094be5bd371601-patches/0003-Build-breakpad_88e5b2c-on-ppc64le.patch
File 
source/breakpad/breakpad-88e5b2c8806bac3f2c80d2fe80094be5bd371601-patches/0003-Build-breakpad_88e5b2c-on-ppc64le.patch:

I really encourage you to get this patch into upstream breakpad. The problem is 
that if we rebase onto a new upstream breakpad, and your changes no longer work 
or don't cleanly apply, we don't have the knowledge or time to fix it so might 
have to discard the patch until you can look at it.

I didn't validate the PPC implementation but I did a pass to make sure that all 
the PPC stuff is included in the appropriate #ifdefs. Mostly looks good, but 
there were a few places where logical changes are outside of #ifdefs and thus  
affect other platforms. I actually can't build breakpad on x86 after applying 
this patch.

64 -std=c++11 -MT src/client/linux/minidump_writer/minidump_writer.o -MD -MP 
-MF $depbase.Tpo -c -o src/client/linux/minidump_writer/minidump_writer.o 
src/client/linux/minidump_writer/minidump_writer.cc &&\
mv -f $depbase.Tpo $depbase.Po
src/client/linux/handler/exception_handler.cc: In member function ‘bool 
google_breakpad::ExceptionHandler::HandleSignal(int, siginfo_t*, void*)’:
src/client/linux/handler/exception_handler.cc:456:27: error: ‘struct 
mcontext_t’ has no member named ‘fp_regs’
   if (uc_ptr->uc_mcontext.fp_regs) {
   ^
src/client/linux/handler/exception_handler.cc:457:63: error: ‘struct 
mcontext_t’ has no member named ‘fp_regs’
 memcpy(_crash_context_.float_state, uc_ptr->uc_mcontext.fp_regs,
   ^
src/client/linux/handler/exception_handler.cc: In member function ‘bool 
google_breakpad::ExceptionHandler::WriteMinidump()’:
src/client/linux/handler/exception_handler.cc:692:60: error: ‘struct 
mcontext_t’ has no member named ‘fp_regs’
   memcpy(_state, context.context.uc_mcontext.fp_regs,
^
Makefile:4986: recipe for target 'src/client/linux/handler/exception_handler.o' 
failed
make: *** [src/client/linux/handler/exception_handler.o] Error 1
make: *** Waiting for unfinished jobs
tarmstrong@tarmstrong-box:~/Impala/native-toolchain$ 
(gvim:12601): GLib-GObject-WARNING **: cannot retrieve class for invalid 
(unclassed) type ''


http://gerrit.cloudera.org:8080/#/c/6468/3/source/breakpad/breakpad-88e5b2c_ppc.patch
File source/breakpad/breakpad-88e5b2c_ppc.patch:

Was this accidentally checked in?


http://gerrit.cloudera.org:8080/#/c/6468/3/source/crcutil/build.sh
File source/crcutil/build.sh:

Line 36: 
Don't need this change


http://gerrit.cloudera.org:8080/#/c/6468/3/source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0002_crc_interface_cc.patch
File 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0002_crc_interface_cc.patch:

Line 7: +  #if CRCUTIL_USE_MM_CRC32
If I understand correctly, this is just a hack to remove code that doesn't 
compile without SSE4? And it doesn't matter because this is just example code? 
Can you comment that just so it's clear.

E.g.:
 // Hack: get example to compile without SSE4.


Line 12: +  return 0;
The second return should be in a #else, right? Otherwise the function has two 
returns on x86-64?


http://gerrit.cloudera.org:8080/#/c/6468/3/source/kudu/build.sh
File source/kudu/build.sh:

Line 93:   
Can you revert the formatting changes? it just makes it harder to see where the 
lines came from historically.


http://gerrit.cloudera.org:8080/#/c/6468/3/source/libunwind/build.sh
File source/libunwind/build.sh:

Line 35: 

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 3:

> Uploaded patch set 3.

It would be great if I could get your comments on the new patchset I've 
uploaded.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 3
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has uploaded a new patch set (#3).

Change subject: Ported native-toolchain to work on ppc64le
..

Ported native-toolchain to work on ppc64le

Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
---
M buildall.sh
M functions.sh
M init-compiler.sh
M init.sh
M source/autoconf/build.sh
A 
source/breakpad/breakpad-88e5b2c8806bac3f2c80d2fe80094be5bd371601-patches/0003-Build-breakpad_88e5b2c-on-ppc64le.patch
A source/breakpad/breakpad-88e5b2c_ppc.patch
M source/crcutil/build.sh
A 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0001_crc_autogen.patch
A 
source/crcutil/crcutil-440ba7babeff77ffad992df3a10c767f184e946e-patches/0002_crc_interface_cc.patch
M source/cyrus-sasl/build.sh
M source/glog/build.sh
M source/kudu/build.sh
M source/libevent/build.sh
M source/libtool/build.sh
M source/libunwind/build.sh
M source/llvm/build-3.3.sh
M source/llvm/build-source-tarball.sh
M source/openldap/build.sh
M source/openssl/build.sh
M source/thrift/build.sh
21 files changed, 1,279 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/68/6468/3
-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 3
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

(1 comment)

Thanks, Tim.

http://gerrit.cloudera.org:8080/#/c/6468/1/source/crcutil/build.sh
File source/crcutil/build.sh:

Line 42:   wrap autoreconf -i;
> Shouldn't you patch Makefile.in then so that the generated Makefile.am is c
I understand that Makefile.am generates Makefile.in.
Only post running autogen.sh the Makefile.am is generated followed by 
Makefile.in


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

(1 comment)

Thanks for the responses - looking forward to the next patchset. I missed that 
you were waiting for a comment to push the next patchset - usually I wait until 
a new patchset has been pushed to do the next review pass.

http://gerrit.cloudera.org:8080/#/c/6468/1/source/crcutil/build.sh
File source/crcutil/build.sh:

Line 42:   wrap autoreconf -i;
> Patching mechanism usage for 2nd patch file. Please let me know your views 
Shouldn't you patch Makefile.in then so that the generated Makefile.am is 
correct?


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

> We talked about this on dev@ and it seems there are no objections
 > to starting code reviews without a testing infrastructure set up.
 > 
 > That is not an approval of this patch, only an indication that
 > reviews can begin.

Thanks, Jim! Good to know that the review process can continue.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

> > > > @Matthew: Would you be needing the ppc64le infra for
 > temporary
 > > > > usage to test the changes until Cloudera/Apache arranges its
 > > own
 > > > > node or is it needed permanently for maintenance of ppc64le
 > > code?
 > 
 > > >
 > > > We don't have a way to allocate a ppc64le node in ou pre-commit
 > > > testing, which uses Jenkins on AWS. We will need a way to test
 > > > ppc64le works as long as we want to support it.
 > > >
 > > > This means a physical node outside of AWS that Jenkins can talk
 > > to
 > > > or an emulator on an AWS node that Jenkins can talk to.
 > > >
 > > > And this needs to exist not just for testing your new patchset
 > > but
 > > > indefinitely.
 > >
 > > Also, this discussion should probably happen on dev@, not here.
 > > Patch comment discussions are probably read by fewer people. Can
 > > you start your discussion on testing on dev@, please?
 > 
 > Thanks, Jim! Surely I'll start the discussion on dev@ too.

We talked about this on dev@ and it seems there are no objections to starting 
code reviews without a testing infrastructure set up.

That is not an approval of this patch, only an indication that reviews can 
begin.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

> > > @Matthew: Would you be needing the ppc64le infra for temporary
 > > > usage to test the changes until Cloudera/Apache arranges its
 > own
 > > > node or is it needed permanently for maintenance of ppc64le
 > code?

 > >
 > > We don't have a way to allocate a ppc64le node in ou pre-commit
 > > testing, which uses Jenkins on AWS. We will need a way to test
 > > ppc64le works as long as we want to support it.
 > >
 > > This means a physical node outside of AWS that Jenkins can talk
 > to
 > > or an emulator on an AWS node that Jenkins can talk to.
 > >
 > > And this needs to exist not just for testing your new patchset
 > but
 > > indefinitely.
 > 
 > Also, this discussion should probably happen on dev@, not here.
 > Patch comment discussions are probably read by fewer people. Can
 > you start your discussion on testing on dev@, please?

Thanks, Jim! Surely I'll start the discussion on dev@ too.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

> > @Matthew: Would you be needing the ppc64le infra for temporary
 > > usage to test the changes until Cloudera/Apache arranges its own
 > > node or is it needed permanently for maintenance of ppc64le code?
 > 
 > We don't have a way to allocate a ppc64le node in ou pre-commit
 > testing, which uses Jenkins on AWS. We will need a way to test
 > ppc64le works as long as we want to support it.
 > 
 > This means a physical node outside of AWS that Jenkins can talk to
 > or an emulator on an AWS node that Jenkins can talk to.
 > 
 > And this needs to exist not just for testing your new patchset but
 > indefinitely.

Also, this discussion should probably happen on dev@, not here. Patch comment 
discussions are probably read by fewer people. Can you start your discussion on 
testing on dev@, please?

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

> @Matthew: Would you be needing the ppc64le infra for temporary
 > usage to test the changes until Cloudera/Apache arranges its own
 > node or is it needed permanently for maintenance of ppc64le code?

We don't have a way to allocate a ppc64le node in ou pre-commit testing, which 
uses Jenkins on AWS. We will need a way to test ppc64le works as long as we 
want to support it.

This means a physical node outside of AWS that Jenkins can talk to or an 
emulator on an AWS node that Jenkins can talk to.

And this needs to exist not just for testing your new patchset but indefinitely.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

@Matthew: Would you be needing the ppc64le infra for temporary usage to test 
the changes until Cloudera/Apache arranges its own node or is it needed 
permanently for maintenance of ppc64le code?

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

Matthew, could you let me know the system configuration required on the ppc64le 
infra ?

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

> Valencia, before we continue the discussion of the work on this
 > toolchain patch much further, I think it would be good to know what
 > the overarching plan is for the ppc64le work across Impala,
 > including testing. For example, one major concern is that the
 > current jenkins test infrastructure does not have ppc64le worker
 > nodes, so I don't see how we'll be able to test this. Can you
 > submit a plan for your ppc64le work to the Apache dev@ list so we
 > can have a broader discussion with the community?

I understand your concern, Matthew. We are working on it. I'll initiate the 
thread at Apache dev@ about ppc64le work plan for Impala and its 
native-toolchain, after getting clarity from my IBM managers.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

Valencia, before we continue the discussion of the work on this toolchain patch 
much further, I think it would be good to know what the overarching plan is for 
the ppc64le work across Impala, including testing. For example, one major 
concern is that the current jenkins test infrastructure does not have ppc64le 
worker nodes, so I don't see how we'll be able to test this. Can you submit a 
plan for your ppc64le work to the Apache dev@ list so we can have a broader 
discussion with the community?

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

(1 comment)

I'm ready with almost all required changes, except, for crcutil. Before I push 
them on gerrit, I need to know your views on patching mechanism usage issue 
w.r.t crcutil below.

http://gerrit.cloudera.org:8080/#/c/6468/1/source/crcutil/build.sh
File source/crcutil/build.sh:

Line 42:   wrap autoreconf -i;
> The patch is applied on Makefile.am which is generated only after autogen.s
Patching mechanism usage for 2nd patch file. Please let me know your views on 
this.


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

(16 comments)

> (5 comments)

@Matt: Thanks for your comments. I've incorporated the modifications you asked 
for.


> (11 comments)

@Tim: Thanks for your comments.

> I did a pass over most of it. It would be easier to review with
> some cleanup to avoid duplicating code and make things consistent
> with the style of the rest of the toolchain.
I've aimed to eliminate the duplicated code in most of the places and align it 
to toolchain style with the help of your comments. But in some places, 
especially, using patching mechanism in crcutil and breakpad, I will require to 
add a conditional check for them in buildall.sh. I would like to know if there 
is any better way to do it.

> I'm also reluctant to add patches here that aren't in the upstream
> projects - we're not really equipped to review them.
Yes...I understand your concern w.r.t patches which are not yet upstream. We 
are already communicating with the respective communities to get the patches 
upstreamed.

http://gerrit.cloudera.org:8080/#/c/6468/1/buildall.sh
File buildall.sh:

PS1, Line 270: master
> it would be better to use the same version as below, but just build_fake_pa
Done.


http://gerrit.cloudera.org:8080/#/c/6468/1/source/autoconf/build.sh
File source/autoconf/build.sh:

Line 31: if [[ "$(uname -p)" == "ppc64le" ]]; then
> Can you add a variable to init.sh to hold the architecture instead of repea
Done. I've added a new var 'ARCH_NAME' in init.sh that will be initialized to 
the underlying machine arch.


Line 32:   wrap ./configure \
> No need to wrap lines when they fit in 90 characters
Ok. But this is as it is upstream.


Line 35: else
> The indentation of the if is wrong. Can you fix this here and elsewhere?
Ok. Fixed.


http://gerrit.cloudera.org:8080/#/c/6468/1/source/breakpad/build.sh
File source/breakpad/build.sh:

Line 35:  wrap patch  -p1 < 
$SOURCE_DIR/source/breakpad/breakpad-88e5b2c_ppc.patch
> We already have a mechanism for including patches in the toolchain - see th
Ok. I've been able to successfully use the native-toolchain patching mechanism 
to apply the ppc-patches for breakpad. However, this leads to conditional 
setting of BREAKPAD_VERSION in buildall.sh based on a arch check.


http://gerrit.cloudera.org:8080/#/c/6468/1/source/crcutil/build.sh
File source/crcutil/build.sh:

Line 37: if [[ "$(uname -p)" == "ppc64le" ]]; then
> Indentation
Fixed.


Line 42:   wrap autoreconf -i;
> Why not apply the patch before running autogen.sh above?
The patch is applied on Makefile.am which is generated only after autogen.sh. 
Therefore, cannot apply the patch before autogen.sh. 

I used the native-toolchain patching mechanism here. It works fine for the 
"crc_interface_cc.patch", however, not for the "crc_makefile_am.patch", as the 
required file "Makefile.am" is not available at this point and will be 
generated later when the autogen.sh is executed.

In this case, I might have to use my current approach without using the 
patching mechanism.

Please let me know if there is any other better way to get the Makefile.am 
patch applied.


http://gerrit.cloudera.org:8080/#/c/6468/1/source/crcutil/ppc-patches/crc_interface_cc.patch
File source/crcutil/ppc-patches/crc_interface_cc.patch:

Line 1: --- 
/home/test/crcutil-440ba7babeff77ffad992df3a10c767f184e946e/examples/interface.cc
   2014-05-23 09:31:26.0 +0530
> I don't really understand what this patch does and we don't know enough abo
Since this code is part of the examples section, I have just worked around the 
compilation error. I am yet to fix it appropriately. Once done I’ll surely push 
it upstream.


http://gerrit.cloudera.org:8080/#/c/6468/1/source/cyrus-sasl/build.sh
File source/cyrus-sasl/build.sh:

Line 51: CFLAGS="$CFLAGS -fPIC -DPIC" CXXFLAGS="$CXXFLAGS -fPIC -DPIC" wrap 
./configure \
> Why not just put the additional flag into CONFIGURE_FLAGS above?
Agreed. I've done the change.


http://gerrit.cloudera.org:8080/#/c/6468/1/source/glog/build.sh
File source/glog/build.sh:

Line 37: --build=powerpc64le-unknown-linux-gnu \
> The command is mostly the same. Why not factor out the different flag into 
Yes. I've done the change.


http://gerrit.cloudera.org:8080/#/c/6468/1/source/kudu/build.sh
File source/kudu/build.sh:

Line 68:  
> Can you remove the build changes for now since it sounds like this is still
Ok. Done


PS1, Line 70: git clone https://github.com/ibmsoe/kudu
> The Kudu changes will be merged into upstream Kudu master, right?
Yes. Right.


PS1, Line 121: echo "Installing gcc-4.9.3 to build kudu src code on ppc"
 : source $SOURCE_DIR/source/kudu/setup_gcc493.sh
> why do you need a different version of gcc to build kudu but not for other 
GCC_VERSION in init.sh is set to 4.9.2. This gcc version works perfect for 

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

@Matthew, @Tim: Thanks for reviewing the patch. I'm working on the pointers you 
have suggested.

-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Valencia Edna Serrao 
Gerrit-HasComments: No

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

(11 comments)

I did a pass over most of it. It would be easier to review with some cleanup to 
avoid duplicating code and make things consistent with the style of the rest of 
the toolchain.

I'm also reluctant to add patches here that aren't in the upstream projects - 
we're not really equipped to review them.

Matt I think looked at the Kudu stuff in detail so I skipped that.

http://gerrit.cloudera.org:8080/#/c/6468/1/source/autoconf/build.sh
File source/autoconf/build.sh:

Line 31: if [[ "$(uname -p)" == "ppc64le" ]]; then
Can you add a variable to init.sh to hold the architecture instead of repeating 
the uname call everywhere (e.g. see OS_NAME).


Line 32:   wrap ./configure \
No need to wrap lines when they fit in 90 characters


Line 35: else
The indentation of the if is wrong. Can you fix this here and elsewhere?


http://gerrit.cloudera.org:8080/#/c/6468/1/source/breakpad/build.sh
File source/breakpad/build.sh:

Line 35:  wrap patch  -p1 < 
$SOURCE_DIR/source/breakpad/breakpad-88e5b2c_ppc.patch
We already have a mechanism for including patches in the toolchain - see the 
-patches directories.


http://gerrit.cloudera.org:8080/#/c/6468/1/source/crcutil/build.sh
File source/crcutil/build.sh:

Line 37: if [[ "$(uname -p)" == "ppc64le" ]]; then
Indentation


Line 42:   wrap autoreconf -i;
Why not apply the patch before running autogen.sh above?


http://gerrit.cloudera.org:8080/#/c/6468/1/source/crcutil/ppc-patches/crc_interface_cc.patch
File source/crcutil/ppc-patches/crc_interface_cc.patch:

Line 1: --- 
/home/test/crcutil-440ba7babeff77ffad992df3a10c767f184e946e/examples/interface.cc
   2014-05-23 09:31:26.0 +0530
I don't really understand what this patch does and we don't know enough about 
crcutil to review it. Have you tried submitting it to upstream crcutil?


http://gerrit.cloudera.org:8080/#/c/6468/1/source/cyrus-sasl/build.sh
File source/cyrus-sasl/build.sh:

Line 51: CFLAGS="$CFLAGS -fPIC -DPIC" CXXFLAGS="$CXXFLAGS -fPIC -DPIC" wrap 
./configure \
Why not just put the additional flag into CONFIGURE_FLAGS above?


http://gerrit.cloudera.org:8080/#/c/6468/1/source/glog/build.sh
File source/glog/build.sh:

Line 37: --build=powerpc64le-unknown-linux-gnu \
The command is mostly the same. Why not factor out the different flag into a 
variable like CONFIGURE_FLAGS


http://gerrit.cloudera.org:8080/#/c/6468/1/source/libevent/build.sh
File source/libevent/build.sh:

Line 34: if [[ "$(uname -p)" == "ppc64le" ]]; then
Indentation


Line 35:   wrap ./configure --build=powerpc64le-unknown-linux-gnu 
--prefix=$LOCAL_INSTALL
Factor out the different parts into a CONFIGURE_FLAGS variable here.


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: Ported native-toolchain to work on ppc64le
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6468/1/buildall.sh
File buildall.sh:

PS1, Line 270: master
it would be better to use the same version as below, but just 
build_fake_package as we do if it's not a 'supported platform'.

A cleaner solution would just be to modify 'is_supported_platform' to return 
false if ppc64le


http://gerrit.cloudera.org:8080/#/c/6468/1/source/kudu/build.sh
File source/kudu/build.sh:

Line 68:  
Can you remove the build changes for now since it sounds like this is still a 
WIP and you'll be building the 'fake package'?


PS1, Line 70: git clone https://github.com/ibmsoe/kudu
The Kudu changes will be merged into upstream Kudu master, right?


PS1, Line 121: echo "Installing gcc-4.9.3 to build kudu src code on ppc"
 : source $SOURCE_DIR/source/kudu/setup_gcc493.sh
why do you need a different version of gcc to build kudu but not for other 
packages? i.e. why can't you just use this version of gcc for the entire 
toolchain (see GCC_VERSION in init.sh )


http://gerrit.cloudera.org:8080/#/c/6468/1/source/kudu/kudu-config.sh
File source/kudu/kudu-config.sh:

this file is misleading because it's specific to the ppc build. regardless, it 
looks to me like you're not building kudu yet from buildall.sh which calls 
build_fake_package, so it'd be better to remove this for now


-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes

[native-toolchain-CR] Ported native-toolchain to work on ppc64le

[Valencia Edna Serrao (Code Review)]

Valencia Edna Serrao has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/6468

Change subject: Ported native-toolchain to work on ppc64le
..

Ported native-toolchain to work on ppc64le

Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
---
M buildall.sh
M init-compiler.sh
M source/autoconf/build.sh
A source/breakpad/breakpad-88e5b2c_ppc.patch
M source/breakpad/build.sh
M source/crcutil/build.sh
A source/crcutil/ppc-patches/crc_interface_cc.patch
A source/crcutil/ppc-patches/crc_makefile_am.patch
M source/cyrus-sasl/build.sh
M source/glog/build.sh
M source/kudu/build.sh
A source/kudu/kudu-config.sh
A source/kudu/setup_gcc493.sh
M source/libevent/build.sh
M source/libtool/build.sh
M source/libunwind/build.sh
M source/llvm/build-3.3.sh
M source/llvm/build-source-tarball.sh
M source/openldap/build.sh
M source/openssl/build.sh
M source/thrift/build.sh
21 files changed, 795 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/68/6468/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7931752ca497bc7a5e3cc574bbb54637f382c72a
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao