You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org> on 2017/03/24 11:13:35 UTC
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
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 <vs...@us.ibm.com>
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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.000000000 +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\u2019ll 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 all of the native-toolchain components, except, kudu. To build kudu on ppc64le, we require to translate the SSE instructions to their altivec equivalent. The gcc version 4.9.2 doesn\u2019t support certain required altivec functions, that is why I had to use gcc-4.9.3 which supports all the required altivec functions.
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,
Since I\u2019ll continue using \u2018build_fake_package\u2019 for kudu in buildall.sh (as I\u2019m currently aligning the ppc64le ported kudu to the latest upstream), I agree that it\u2019ll be better remove the kudu-config.sh for now.
Removed.
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
Fixed.
Line 35: wrap ./configure --build=powerpc64le-unknown-linux-gnu --prefix=$LOCAL_INSTALL
> Factor out the different parts into a CONFIGURE_FLAGS variable here.
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: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
Valencia Edna Serrao has posted comments on this change.
Change subject: Ported native-toolchain to work on ppc64le
......................................................................
Patch Set 1:
(1 comment)
Patch set2 is ready. I'm having a complete run of it, so as to consolidate the logs for submission.
Following is the update on the crcutil patching issue.
http://gerrit.cloudera.org:8080/#/c/6468/1/source/crcutil/build.sh
File source/crcutil/build.sh:
Line 42: wrap autoreconf -i;
> I understand that Makefile.am generates Makefile.in.
I have instead patched the autogen.sh, to set the crcutil_flags based on arch. This enables all of the crcutil patches to be applied using native-toolchain's patching mechanism.
--
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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(&g_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(&context.float_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 '<invalid>'
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: echo "ppc64_test_altivec_LDADD = \$(LIBUNWIND)" >> $THIS_DIR/$PACKAGE-$PACKAGE_VERSION/tests/Makefile.am
This could be a patch too, right?
Sorry to be pedantic but I want to make things are set up to reduce the chances of us accidentally breaking your changes.
http://gerrit.cloudera.org:8080/#/c/6468/3/source/llvm/build-3.3.sh
File source/llvm/build-3.3.sh:
Line 87: TARGET+="powerpc,cpp"
nit: don't need TARGET= above, can just use TARGET= here and on line 88
http://gerrit.cloudera.org:8080/#/c/6468/3/source/llvm/build-source-tarball.sh
File source/llvm/build-source-tarball.sh:
Line 79: if [[ "$ARCH_NAME" == "ppc64le" ]]; then
nit: don't need TARGET= above, can just use TARGET= here and on line 82
--
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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.000000000 +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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
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/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 checked if any interference, of ppc64le changes in x86 code, existed by b
I was hoping we could compare the sha1sum of the binary before/after the change? I don't want to hold this up if that doesn't line up, but it would give us confidence that we're not affecting the x86 bits :)
--
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: No
[native-toolchain-CR] Ported native-toolchain to work on ppc64le
Posted by "Valencia Edna Serrao (Code Review)" <ge...@cloudera.org>.
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 <vs...@us.ibm.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Valencia Edna Serrao <vs...@us.ibm.com>
Gerrit-HasComments: Yes