Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/12285 )
Change subject: Initial support for building the toolchain in docker ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/12285/6/docker/redhat/CentOS-6.6-Base.repo File docker/redhat/CentOS-6.6-Base.repo: http://gerrit.cloudera.org:8080/#/c/12285/6/docker/redhat/CentOS-6.6-Base.repo@34 PS6, Line 34: enabled=1 The Centos Plus repo is enabled for 6.6, but it is disabled for 7.2 (see the 7.2. repo descriptor file). I admit I don't know which one is the correct approach, I just wonder if the discrepancy is intentional, or maybe just an oversight. http://gerrit.cloudera.org:8080/#/c/12285/6/docker/redhat6.df File docker/redhat6.df: http://gerrit.cloudera.org:8080/#/c/12285/6/docker/redhat6.df@26 PS6, Line 26: java-1.8.0-openjdk-devel I also wonder about the JDK version selection logic: RedHat variants get 1.8, Debian and Ubuntu receive 1.7, except Ubuntu 16.04, which get 1.8. IIRC Kudu is the only toolchain component that requires Java. If this patch is intended to support only the master branch of Impala, then this is probably OK. Impala master uses Java 8, and code built with Java 7 should be able to run. Additionally, Impala 3.x usually consumes Kudu from the Cloudera-built CDH tarballs (see the environment variable USE_CDH_KUDU in Impala/bin/impala-config.sh), so the Kudu bits built and published with the toolchain may be less interesting. For this case I am assuming that binary toolchain bits for Impala 2.x would continue to be published by the existing build mechanisms. However, if this patch, or patch set targets the Impala 2.x branch as well, then we would need Java 7 everywhere for two reasons: 1. Impala 2.x is built with Java 7 2. Impala 2.x consumes Kudu exclusively from the toolchain bits, so the JDK used for building Kudu will impact Impala 2.x directly. Happy to discuss off-Gerritt if that's more comfortable for you. -- To view, visit http://gerrit.cloudera.org:8080/12285 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If42c9bc06a3d303642eb37dea784b61e2a1f5cc6 Gerrit-Change-Number: 12285 Gerrit-PatchSet: 6 Gerrit-Owner: hector.aco...@cloudera.com <hector.aco...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: hector.aco...@cloudera.com <hector.aco...@cloudera.com> Gerrit-Comment-Date: Wed, 20 Feb 2019 21:44:41 +0000 Gerrit-HasComments: Yes
