Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-12479

ProtocMojo does not log the reason for a protoc compilation failure.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: build
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      If protoc fails with a compilation error in the proto files, our Maven plugin won't print the details. The only way to figure it out is to repeat running the protoc command manually from outside the Hadoop build. This is because our ProtocMojo only captures stdout from the protoc command, and compilation errors get written to stderr.

        Activity

        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #504 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/504/)
        HADOOP-12479. ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f)

        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #504 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/504/ ) HADOOP-12479 . ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f) hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2441 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2441/)
        HADOOP-12479. ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2441 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2441/ ) HADOOP-12479 . ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2489 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2489/)
        HADOOP-12479. ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f)

        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2489 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2489/ ) HADOOP-12479 . ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f) hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #1277 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1277/)
        HADOOP-12479. ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #1277 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1277/ ) HADOOP-12479 . ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #554 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/554/)
        HADOOP-12479. ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #554 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/554/ ) HADOOP-12479 . ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #540 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/540/)
        HADOOP-12479. ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #540 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/540/ ) HADOOP-12479 . ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8648 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8648/)
        HADOOP-12479. ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f)

        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8648 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8648/ ) HADOOP-12479 . ProtocMojo does not log the reason for a protoc (cnauroth: rev fdd740622459625efe5e12f37577aa3f5746177f) hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/util/Exec.java hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        cnauroth Chris Nauroth added a comment -

        I have committed this to trunk and branch-2. Steve, thanks again for the review.

        Show
        cnauroth Chris Nauroth added a comment - I have committed this to trunk and branch-2. Steve, thanks again for the review.
        Hide
        cnauroth Chris Nauroth added a comment -

        That change to exec() doesn't break anything does it?

        No, it's fine. The only other caller is the VersionInfoMojo. I chose to do the Exec change in a way that would be backward-compatible for VersionInfoMojo. It's a new overload of the run method that captures stderr in a separate in/out variable. Only ProtocMojo calls the new overload. VersionInfoMojo still calls the old one.

        An alternative would have been just to dump both stdout and stderr into the same in/out variable used by the existing run method. That could have been dangerous, because VersionInfoMojo has specific expectations about the output, and dumping stderr on top of that could have harmed that logic.

        Thanks for the review. I'll commit later today.

        Show
        cnauroth Chris Nauroth added a comment - That change to exec() doesn't break anything does it? No, it's fine. The only other caller is the VersionInfoMojo . I chose to do the Exec change in a way that would be backward-compatible for VersionInfoMojo . It's a new overload of the run method that captures stderr in a separate in/out variable. Only ProtocMojo calls the new overload. VersionInfoMojo still calls the old one. An alternative would have been just to dump both stdout and stderr into the same in/out variable used by the existing run method. That could have been dangerous, because VersionInfoMojo has specific expectations about the output, and dumping stderr on top of that could have harmed that logic. Thanks for the review. I'll commit later today.
        Hide
        stevel@apache.org Steve Loughran added a comment -

        That change to exec() doesn't break anything does it?

        Given its inside the maven plugin, I'm happy

        +1

        Show
        stevel@apache.org Steve Loughran added a comment - That change to exec() doesn't break anything does it? Given its inside the maven plugin, I'm happy +1
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 16m 44s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s 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 8m 1s There were no new javac warning messages.
        +1 javadoc 10m 32s There were no new javadoc warning messages.
        -1 release audit 0m 17s The applied patch generated 1 release audit warnings.
        +1 checkstyle 0m 17s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 32s mvn install still works.
        +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
        +1 findbugs 0m 41s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 maven tests 0m 13s Tests passed in hadoop-maven-plugins.
            38m 55s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12766669/HADOOP-12479.001.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / be7a0ad
        Release Audit https://builds.apache.org/job/PreCommit-HADOOP-Build/7815/artifact/patchprocess/patchReleaseAuditProblems.txt
        hadoop-maven-plugins test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7815/artifact/patchprocess/testrun_hadoop-maven-plugins.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7815/testReport/
        Java 1.7.0_55
        uname Linux asf904.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7815/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 16m 44s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s 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 8m 1s There were no new javac warning messages. +1 javadoc 10m 32s There were no new javadoc warning messages. -1 release audit 0m 17s The applied patch generated 1 release audit warnings. +1 checkstyle 0m 17s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 32s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. +1 findbugs 0m 41s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 maven tests 0m 13s Tests passed in hadoop-maven-plugins.     38m 55s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12766669/HADOOP-12479.001.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / be7a0ad Release Audit https://builds.apache.org/job/PreCommit-HADOOP-Build/7815/artifact/patchprocess/patchReleaseAuditProblems.txt hadoop-maven-plugins test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7815/artifact/patchprocess/testrun_hadoop-maven-plugins.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7815/testReport/ Java 1.7.0_55 uname Linux asf904.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7815/console This message was automatically generated.
        Hide
        stevel@apache.org Steve Loughran added a comment -

        funny, protoc by hand is exactly what I was doing this morning

        Show
        stevel@apache.org Steve Loughran added a comment - funny, protoc by hand is exactly what I was doing this morning

          People

          • Assignee:
            cnauroth Chris Nauroth
            Reporter:
            cnauroth Chris Nauroth
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development