Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9274 )

Change subject: IMPALA-5717: Build ORC C++ lib in toolchain
......................................................................


Patch Set 4:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9274/4/buildall.sh@320
PS4, Line 320:   export LZ4_VERSION=1.7.5
> Remind me what happens if these end up inconsistent with other _VERSIONs fr
Building from scratch will fail since the directories won't exist. I feel like 
this mechanism is a bit clunky but it's consistent with what we're already 
doing.


http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/build.sh
File source/orc/build.sh:

http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/build.sh@38
PS4, Line 38: DBUILD_LZ4
> Even though compression BUILDs are OFF, we still need to specify what versi
It seems to be sufficient to point it at the header and library directories. 
I'll revisit once we pick up the ORC-266 patch to see what they did there.


http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch
File 
source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch:

http://gerrit.cloudera.org:8080/#/c/9274/4/source/orc/orc-1.4.3-patches/0001-Allow-building-against-external-versions-of-dependen.patch@4
PS4, Line 4: PATCH 1/2
> What is part 2/2? Also, is there a JIRA for this? Is it slated to land upst
This was an artifact of me generating it with "git format-patch -2" on a branch 
with another commit that I replaced with the other patch.

I discovered after writing this patch that ORC-266 had landed upstream. 
Backporting ORC-266 was a nightmare though, because it was dependent on a 
series of previous patches that entangled the CMake file changes with various 
other changes. So I just decided to stick with this solution. I updated the 
commit message in this patch to make it clear that it shouldn't be carried to 
newer versions.



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6abd86a5a683f19aa44b47629edf283b938b7e7e
Gerrit-Change-Number: 9274
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Mar 2018 18:59:53 +0000
Gerrit-HasComments: Yes

Reply via email to