You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by svarnau <gi...@git.apache.org> on 2016/03/09 00:52:47 UTC

[GitHub] incubator-trafodion pull request: [TRAFODION-1880] Do not build li...

GitHub user svarnau opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/373

    [TRAFODION-1880] Do not build libhdfs dependency from source

    Make sure various pom.xml files with hadoop dependency are all pulling in
    the same version.
    
    Make HDFS dependencies a build-time pre-req. As with other build
    dependencies on shared libraries, make them part of the build environment
    (TOOLSDIR).
    
    Prior to this change, we were picking up these dependencies (2 shlib &
    1 header) from environment (such as local_hadoop) or downloading the
    source and building it on the fly. Building it made trafodion build more
    than twice as long.
    
    Now requiring a consistent build dependency, separate from the runtime
    environment.
    
    The 64-bit native libraries are now available from binary distro of
    hadoop-common. So we can download binaries and extract files we need rather
    than build them.  Dependencies can be updated via install/traf_tools_setup.sh,
    but in case the environment is not updated, the build-time script get_hdfs_files
    is also updated to downoad the distro. If you really want to build
    hadoop-common, that portion of the script was retained under a new --source
    option.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/svarnau/incubator-trafodion libs1880

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafodion/pull/373.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #373
    
----
commit 3853382a5d5386f1b048f2c1bdb4eea0440b4a16
Author: Steve Varnau <sv...@apache.org>
Date:   2016-03-08T23:26:48Z

    [TRAFODION-1880] Do not build libhdfs dependency from source
    
    Make sure various pom.xml files with hadoop dependency are all pulling in
    the same version.
    
    Make HDFS dependencies a build-time pre-req. As with other build
    dependencies on shared libraries, make them part of the build environment
    (TOOLSDIR).
    
    Prior to this change, we were picking up these dependencies (2 shlib &
    1 header) from environment (such as local_hadoop) or downloading the
    source and building it on the fly. Building it made trafodion build more
    than twice as long.
    
    Now requiring a consistent build dependency, separate from the runtime
    environment.
    
    The 64-bit native libraries are now available from binary distro of
    hadoop-common. So we can download binaries and extract files we need rather
    than build them.  Dependencies can be updated via install/traf_tools_setup.sh,
    but in case the environment is not updated, the build-time script get_hdfs_files
    is also updated to downoad the distro. If you really want to build
    hadoop-common, that portion of the script was retained under a new --source
    option.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1880] Do not build li...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/373#discussion_r55451520
  
    --- Diff: core/sqf/sql/scripts/get_libhdfs_files ---
    @@ -110,113 +123,141 @@ if [[ $FORCE_BUILD == true || \
           exit 1
         fi
       fi
    -
       cd $LIBHDFS_TEMP_DIR
     
    -  PROTOBUF_VER=`protoc --version 2>/dev/null | cut -f 2 -d ' '`
    +  if [[ $SOURCE_BUILD != true ]]; then
    +    echo "Downloading Hadoop-common binary distro..." | tee -a ${LOGFILE}
    --- End diff --
    
    There might be a small chance that we would pick up 32 bit libraries, since older Hadoop binary packages contain those instead of 64 bit libraries. Probably ok, as we move away from those older versions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1880] Do not build li...

Posted by svarnau <gi...@git.apache.org>.
Github user svarnau commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/373#discussion_r55452062
  
    --- Diff: core/sqf/sql/scripts/get_libhdfs_files ---
    @@ -110,113 +123,141 @@ if [[ $FORCE_BUILD == true || \
           exit 1
         fi
       fi
    -
       cd $LIBHDFS_TEMP_DIR
     
    -  PROTOBUF_VER=`protoc --version 2>/dev/null | cut -f 2 -d ' '`
    +  if [[ $SOURCE_BUILD != true ]]; then
    +    echo "Downloading Hadoop-common binary distro..." | tee -a ${LOGFILE}
    +    wget ${HADOOP_MIRROR_URL}/${HADOOP_BIN_TAR} 2>&1 >>${LOGFILE}
    +    tar -xzf $HADOOP_BIN_TAR  \
    +       $HADOOP_ID/lib/native/libhadoop\*so\* \
    +       $HADOOP_ID/lib/native/libhdfs\*so\* \
    +       $HADOOP_ID/include/hdfs.h
    +
    +    cp -f ${HADOOP_ID}/include/hdfs.h ${TGT_INC_DIR}
    +    cp -Pf ${HADOOP_ID}/lib/native/libhdfs*.so* ${TGT_LIB_DIR}
    +    cp -Pf ${HADOOP_ID}/lib/native/libhadoop*.so* ${TGT_LIB_DIR}
    +
    +  else
    +
    +    PROTOBUF_VER=`protoc --version 2>/dev/null | cut -f 2 -d ' '`
    --- End diff --
    
    Right. Above this line are new, but the large section below was indented for the else clause.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1880] Do not build li...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/373#discussion_r55450938
  
    --- Diff: core/sqf/sql/scripts/get_libhdfs_files ---
    @@ -37,10 +42,11 @@ fi
     LOGFILE=${LIBHDFS_TEMP_DIR}/build.log
     
     # Hadoop source tar file to build libhdfs from
    -HADOOP_SRC_MIRROR_URL=https://archive.apache.org/dist/hadoop/common/hadoop-2.6.0
    +HADOOP_MIRROR_URL=https://archive.apache.org/dist/hadoop/common/hadoop-2.6.0
    --- End diff --
    
    Would it make sense to replace the 2.6.0 with ${HADOOP_DEP_VER}? Just in case this script survives for a longer time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1880] Do not build li...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/373#discussion_r55451610
  
    --- Diff: core/sqf/sql/scripts/get_libhdfs_files ---
    @@ -110,113 +123,141 @@ if [[ $FORCE_BUILD == true || \
           exit 1
         fi
       fi
    -
       cd $LIBHDFS_TEMP_DIR
     
    -  PROTOBUF_VER=`protoc --version 2>/dev/null | cut -f 2 -d ' '`
    +  if [[ $SOURCE_BUILD != true ]]; then
    +    echo "Downloading Hadoop-common binary distro..." | tee -a ${LOGFILE}
    +    wget ${HADOOP_MIRROR_URL}/${HADOOP_BIN_TAR} 2>&1 >>${LOGFILE}
    +    tar -xzf $HADOOP_BIN_TAR  \
    +       $HADOOP_ID/lib/native/libhadoop\*so\* \
    +       $HADOOP_ID/lib/native/libhdfs\*so\* \
    +       $HADOOP_ID/include/hdfs.h
    +
    +    cp -f ${HADOOP_ID}/include/hdfs.h ${TGT_INC_DIR}
    +    cp -Pf ${HADOOP_ID}/lib/native/libhdfs*.so* ${TGT_LIB_DIR}
    +    cp -Pf ${HADOOP_ID}/lib/native/libhadoop*.so* ${TGT_LIB_DIR}
    +
    +  else
    +
    +    PROTOBUF_VER=`protoc --version 2>/dev/null | cut -f 2 -d ' '`
    --- End diff --
    
    Those are all marked as new, but my guess is that they are just indented, so I didn't look at these lines in detail.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1880] Do not build li...

Posted by svarnau <gi...@git.apache.org>.
Github user svarnau commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/373#discussion_r55452134
  
    --- Diff: core/sqf/sql/scripts/get_libhdfs_files ---
    @@ -37,10 +42,11 @@ fi
     LOGFILE=${LIBHDFS_TEMP_DIR}/build.log
     
     # Hadoop source tar file to build libhdfs from
    -HADOOP_SRC_MIRROR_URL=https://archive.apache.org/dist/hadoop/common/hadoop-2.6.0
    +HADOOP_MIRROR_URL=https://archive.apache.org/dist/hadoop/common/hadoop-2.6.0
    --- End diff --
    
    Yes, makes sense.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1880] Do not build li...

Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/373#discussion_r55451702
  
    --- Diff: core/sqf/sql/scripts/get_libhdfs_files ---
    @@ -110,113 +123,141 @@ if [[ $FORCE_BUILD == true || \
           exit 1
         fi
       fi
    -
       cd $LIBHDFS_TEMP_DIR
     
    -  PROTOBUF_VER=`protoc --version 2>/dev/null | cut -f 2 -d ' '`
    +  if [[ $SOURCE_BUILD != true ]]; then
    +    echo "Downloading Hadoop-common binary distro..." | tee -a ${LOGFILE}
    --- End diff --
    
    Never mind, just saw the check for 64 bit libraries at the end.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1880] Do not build li...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafodion/pull/373


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---