Hadoop Common
  1. Hadoop Common
  2. HADOOP-8747

Syntax error on cmake version 2.6 patch 2 in JNIFlags.cmake

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2-alpha
    • Fix Version/s: 2.0.2-alpha
    • Component/s: native
    • Labels:
      None

      Description

      On SUSE Linux Enterprise Server 11 SP1, "cmake version 2.6 patch 2" is installed.

      It seems to have trouble parsing this "if" statement in JNIFlags.cmake:

      IF((NOT JAVA_JVM_LIBRARY) OR (NOT JAVA_INCLUDE_PATH) OR (NOT JAVA_INCLUDE_PATH2))
      

      We should rephrase this if statement so that it will work on all versions of cmake above or equal to 2.6.

      1. HADOOP-8747.002.patch
        1 kB
        Colin Patrick McCabe
      2. HADOOP-8747.001.patch
        0.9 kB
        Colin Patrick McCabe

        Activity

        Hide
        Colin Patrick McCabe added a comment -

        I think the handling of parentheses is at fault here. Here's a version of the if statement which doesn't use extra parentheses.

        Show
        Colin Patrick McCabe added a comment - I think the handling of parentheses is at fault here. Here's a version of the if statement which doesn't use extra parentheses.
        Hide
        Andy Isaacson added a comment -

        The failure we saw was:

             [exec] CMake Error: Error in cmake code at
             [exec] /var/lib/jenkins/workspace/CDH4-Hadoop-MR2-2.0.0/hadoop-common-project/hadoop-common/src/JNIFlags.cmake:106:
             [exec] Parse error.  Function missing ending ")".  Instead found left paren with text "(".
        
        Show
        Andy Isaacson added a comment - The failure we saw was: [exec] CMake Error: Error in cmake code at [exec] / var /lib/jenkins/workspace/CDH4-Hadoop-MR2-2.0.0/hadoop-common-project/hadoop-common/src/JNIFlags.cmake:106: [exec] Parse error. Function missing ending ")" . Instead found left paren with text "(" .
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12542955/HADOOP-8747.001.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

        org.apache.hadoop.ha.TestZKFailoverController

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1381//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1381//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12542955/HADOOP-8747.001.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.ha.TestZKFailoverController +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1381//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1381//console This message is automatically generated.
        Hide
        Colin Patrick McCabe added a comment -

        I tested this on on SUSE Linux Enterprise Server 11 SP1, using "cmake version 2.6 patch 2."

        Also tested on OpenSuSE 12.1 using cmake version 2.8.6.

        Show
        Colin Patrick McCabe added a comment - I tested this on on SUSE Linux Enterprise Server 11 SP1, using "cmake version 2.6 patch 2." Also tested on OpenSuSE 12.1 using cmake version 2.8.6.
        Hide
        Alejandro Abdelnur added a comment -

        +1. Colin, Before I commit, have you tested in other Linux distros as well?

        Show
        Alejandro Abdelnur added a comment - +1. Colin, Before I commit, have you tested in other Linux distros as well?
        Hide
        Todd Lipcon added a comment -

        Looks good. One suggestion: the strange empty IF clause makes this a bit hard to read. Can we do something like:

        +    IF(JAVA_JVM_LIBRARY AND JAVA_INCLUDE_PATH AND JAVA_INCLUDE_PATH2)
               MESSAGE("Using JAVA_JVM_LIBRARY=${JAVA_JVM_LIBRARY} JAVA_INCLUDE_PATH=${JAVA_INCLUDE_PATH} JAVA_INCLUDE_PATH2=${JAVA_INCLUDEPATH2}")
        +    ELSE()
        

        so that we don't have an empty block?

        Show
        Todd Lipcon added a comment - Looks good. One suggestion: the strange empty IF clause makes this a bit hard to read. Can we do something like: + IF(JAVA_JVM_LIBRARY AND JAVA_INCLUDE_PATH AND JAVA_INCLUDE_PATH2) MESSAGE( "Using JAVA_JVM_LIBRARY=${JAVA_JVM_LIBRARY} JAVA_INCLUDE_PATH=${JAVA_INCLUDE_PATH} JAVA_INCLUDE_PATH2=${JAVA_INCLUDEPATH2}" ) + ELSE() so that we don't have an empty block?
        Hide
        Colin Patrick McCabe added a comment -
        • new version without empty 'if' block
        Show
        Colin Patrick McCabe added a comment - new version without empty 'if' block
        Hide
        Colin Patrick McCabe added a comment -

        @Alejandro: Jenkins builds on Debian, so that's another distro this patch has been tested on.

        Show
        Colin Patrick McCabe added a comment - @Alejandro: Jenkins builds on Debian, so that's another distro this patch has been tested on.
        Hide
        Colin Patrick McCabe added a comment -

        Also tested on centos 5.8 and ubuntu 12.04. Anyway, the really important issue here is the cmake version.

        Show
        Colin Patrick McCabe added a comment - Also tested on centos 5.8 and ubuntu 12.04. Anyway, the really important issue here is the cmake version.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12543001/HADOOP-8747.002.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

        org.apache.hadoop.ha.TestZKFailoverController

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1384//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1384//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12543001/HADOOP-8747.002.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.ha.TestZKFailoverController +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1384//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1384//console This message is automatically generated.
        Hide
        Alejandro Abdelnur added a comment -

        +1

        Show
        Alejandro Abdelnur added a comment - +1
        Hide
        Alejandro Abdelnur added a comment -

        Thanks Colin. Committed to trunk and branch-2.

        Show
        Alejandro Abdelnur added a comment - Thanks Colin. Committed to trunk and branch-2.

          People

          • Assignee:
            Colin Patrick McCabe
            Reporter:
            Colin Patrick McCabe
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development