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

Support for incremental generation in the protoc plugin

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.6-alpha
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: None
    • Labels:
      None
    • Target Version/s:

      Description

      The protoc maven plugin currently generates new Java classes every time, which means Maven always picks up changed files in the build. It would be better if the protoc plugin only generated new Java classes when the source protoc files change.

      1. hadoop-12194.001.patch
        9 kB
        Andrew Wang
      2. hadoop-12194.002.patch
        10 kB
        Andrew Wang
      3. hadoop-12194.003.patch
        10 kB
        Andrew Wang

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2197 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2197/)
        HADOOP-12194. Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273)

        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/pom.xml
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2197 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2197/ ) HADOOP-12194 . Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273) hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/pom.xml
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #249 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/249/)
        HADOOP-12194. Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273)

        • hadoop-maven-plugins/pom.xml
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #249 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/249/ ) HADOOP-12194 . Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273) hadoop-maven-plugins/pom.xml hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Hdfs-trunk #2178 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2178/)
        HADOOP-12194. Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        • hadoop-maven-plugins/pom.xml
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #2178 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2178/ ) HADOOP-12194 . Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java hadoop-maven-plugins/pom.xml
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #239 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/239/)
        HADOOP-12194. Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273)

        • hadoop-maven-plugins/pom.xml
        • 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 #239 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/239/ ) HADOOP-12194 . Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273) hadoop-maven-plugins/pom.xml 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 #981 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/981/)
        HADOOP-12194. Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273)

        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        • hadoop-maven-plugins/pom.xml
        • hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #981 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/981/ ) HADOOP-12194 . Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273) hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java hadoop-maven-plugins/pom.xml hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #251 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/251/)
        HADOOP-12194. Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273)

        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/pom.xml
        • 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 #251 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/251/ ) HADOOP-12194 . Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/pom.xml 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 #8134 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8134/)
        HADOOP-12194. Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273)

        • hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-maven-plugins/pom.xml
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8134 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8134/ ) HADOOP-12194 . Support for incremental generation in the protoc plugin. (wang: rev 625d7ed9eb65f0df204b506ce92c11803fbce273) hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/protoc/ProtocMojo.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-maven-plugins/pom.xml
        Hide
        andrew.wang Andrew Wang added a comment -

        Pushed to trunk and branch-2, thanks for reviewing Ravi and Vinay!

        I think with this, incremental compilation should finally work as expected

        Show
        andrew.wang Andrew Wang added a comment - Pushed to trunk and branch-2, thanks for reviewing Ravi and Vinay! I think with this, incremental compilation should finally work as expected
        Hide
        andrew.wang Andrew Wang added a comment -

        Hi Vinay, thanks for reviewing,

        I think we're safe because we only write the new checksum file on success, it acts like a "commit". Since the checksum file is stored in target, running a "mvn clean" also wipes it out in case of any issues.

        I'll commit shortly based on your +1, thanks again. The jenkins run looks screwed up, no changes in the most recent patch besides the additional line break.

        Show
        andrew.wang Andrew Wang added a comment - Hi Vinay, thanks for reviewing, I think we're safe because we only write the new checksum file on success, it acts like a "commit". Since the checksum file is stored in target, running a "mvn clean" also wipes it out in case of any issues. I'll commit shortly based on your +1, thanks again. The jenkins run looks screwed up, no changes in the most recent patch besides the additional line break.
        Hide
        vinayrpet Vinayakumar B added a comment -

        Do you think need to delete the checksum file if any failure occurs so that freshly all files could be generated?

        If not required, here is my +1.

        Show
        vinayrpet Vinayakumar B added a comment - Do you think need to delete the checksum file if any failure occurs so that freshly all files could be generated? If not required, here is my +1.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        -1 pre-patch 13m 14s Pre-patch trunk JavaDoc compilation may be broken.
        +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 7m 39s There were no new javac warning messages.
        -1 javadoc 9m 43s The applied patch generated 50 additional warning messages.
        -1 release audit 0m 21s The applied patch generated 2 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 14s mvn install still works.
        +1 eclipse:eclipse 0m 31s The patch built with eclipse:eclipse.
        +1 findbugs 0m 40s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 maven tests 0m 13s Tests passed in hadoop-maven-plugins.
            33m 55s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12744083/hadoop-12194.003.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / c9dd2ca
        javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/7189/artifact/patchprocess/diffJavadocWarnings.txt
        Release Audit https://builds.apache.org/job/PreCommit-HADOOP-Build/7189/artifact/patchprocess/patchReleaseAuditProblems.txt
        hadoop-maven-plugins test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7189/artifact/patchprocess/testrun_hadoop-maven-plugins.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7189/testReport/
        Java 1.7.0_55
        uname Linux asf909.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/7189/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 13m 14s Pre-patch trunk JavaDoc compilation may be broken. +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 7m 39s There were no new javac warning messages. -1 javadoc 9m 43s The applied patch generated 50 additional warning messages. -1 release audit 0m 21s The applied patch generated 2 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 14s mvn install still works. +1 eclipse:eclipse 0m 31s The patch built with eclipse:eclipse. +1 findbugs 0m 40s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 maven tests 0m 13s Tests passed in hadoop-maven-plugins.     33m 55s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12744083/hadoop-12194.003.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / c9dd2ca javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/7189/artifact/patchprocess/diffJavadocWarnings.txt Release Audit https://builds.apache.org/job/PreCommit-HADOOP-Build/7189/artifact/patchprocess/patchReleaseAuditProblems.txt hadoop-maven-plugins test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7189/artifact/patchprocess/testrun_hadoop-maven-plugins.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7189/testReport/ Java 1.7.0_55 uname Linux asf909.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/7189/console This message was automatically generated.
        Hide
        andrew.wang Andrew Wang added a comment -

        Fix checkstyle, a line was too long. Otherwise ready for review though.

        Show
        andrew.wang Andrew Wang added a comment - Fix checkstyle, a line was too long. Otherwise ready for review though.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 15m 28s 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 7m 36s There were no new javac warning messages.
        +1 javadoc 9m 42s There were no new javadoc warning messages.
        +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 0m 16s The applied patch generated 1 new checkstyle issues (total was 2, now 3).
        +1 whitespace 0m 1s The patch has no lines that end in whitespace.
        +1 install 1m 13s mvn install still works.
        +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
        +1 findbugs 0m 39s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 maven tests 0m 13s Tests passed in hadoop-maven-plugins.
            36m 9s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12744015/hadoop-12194.002.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 3dc92e8
        checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7179/artifact/patchprocess/diffcheckstylehadoop-maven-plugins.txt
        hadoop-maven-plugins test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7179/artifact/patchprocess/testrun_hadoop-maven-plugins.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7179/testReport/
        Java 1.7.0_55
        uname Linux asf900.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/7179/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 15m 28s 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 7m 36s There were no new javac warning messages. +1 javadoc 9m 42s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 16s The applied patch generated 1 new checkstyle issues (total was 2, now 3). +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 13s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. +1 findbugs 0m 39s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 maven tests 0m 13s Tests passed in hadoop-maven-plugins.     36m 9s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12744015/hadoop-12194.002.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 3dc92e8 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7179/artifact/patchprocess/diffcheckstylehadoop-maven-plugins.txt hadoop-maven-plugins test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7179/artifact/patchprocess/testrun_hadoop-maven-plugins.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7179/testReport/ Java 1.7.0_55 uname Linux asf900.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/7179/console This message was automatically generated.
        Hide
        andrew.wang Andrew Wang added a comment -

        New patch attached, I missed iterating through directories recursively.

        Hi Ravi, since I already have the crc method written up and I think it provides better properties than looking at modtimes, I'd like to stick with it. Thanks for reviewing.

        Show
        andrew.wang Andrew Wang added a comment - New patch attached, I missed iterating through directories recursively. Hi Ravi, since I already have the crc method written up and I think it provides better properties than looking at modtimes, I'd like to stick with it. Thanks for reviewing.
        Hide
        raviprak Ravi Prakash added a comment -

        The stack trace when I run with mvn -X is:

        Caused by: java.io.FileNotFoundException: Specified protoc include or source does not exist: /home/raviprak/Code/hadoop/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/proto/server
        	at org.apache.hadoop.maven.plugin.protoc.ProtocMojo$ChecksumComparator.hasChanged(ProtocMojo.java:92)
        	at org.apache.hadoop.maven.plugin.protoc.ProtocMojo.execute(ProtocMojo.java:203)
        	... 21 more
        
        Show
        raviprak Ravi Prakash added a comment - The stack trace when I run with mvn -X is: Caused by: java.io.FileNotFoundException: Specified protoc include or source does not exist: /home/raviprak/Code/hadoop/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/proto/server at org.apache.hadoop.maven.plugin.protoc.ProtocMojo$ChecksumComparator.hasChanged(ProtocMojo.java:92) at org.apache.hadoop.maven.plugin.protoc.ProtocMojo.execute(ProtocMojo.java:203) ... 21 more
        Hide
        raviprak Ravi Prakash added a comment -

        Just for the record, I appreciate you doing this. Thanks a lot. This will hopefully speed up our builds a lot! Also, I'm fine with either way. I would agree with you that the additional code complexity in hadoop-maven-plugin isn't too onerous, and the benefit of compiling only the protoc files that were changed, is probably worth it. Soon as I can figure out why my builds are failing, we can move forward with either method.

        Show
        raviprak Ravi Prakash added a comment - Just for the record, I appreciate you doing this. Thanks a lot. This will hopefully speed up our builds a lot! Also, I'm fine with either way. I would agree with you that the additional code complexity in hadoop-maven-plugin isn't too onerous, and the benefit of compiling only the protoc files that were changed, is probably worth it. Soon as I can figure out why my builds are failing, we can move forward with either method.
        Hide
        raviprak Ravi Prakash added a comment -

        Hi Andrew! It should be sufficient to check the modification time of the output directory, don't you think? I understand that has the downside that all protoc files will be compiled again, but how often do we change the protoc files in the first place?

        Anyway, my build which I run as mvn -Pdist -Pnative -Dmaven.javadoc.skip -DskipTests install is failing with:

        [ERROR] Failed to execute goal org.apache.hadoop:hadoop-maven-plugins:3.0.0-SNAPSHOT:protoc (compile-protoc) on project hadoop-yarn-api: java.io.FileNotFoundException: <my-source-code-directory>/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/proto/server (Is a directory) -> [Help 1]
        

        I did a git clean -fdx too.

        Show
        raviprak Ravi Prakash added a comment - Hi Andrew! It should be sufficient to check the modification time of the output directory, don't you think? I understand that has the downside that all protoc files will be compiled again, but how often do we change the protoc files in the first place? Anyway, my build which I run as mvn -Pdist -Pnative -Dmaven.javadoc.skip -DskipTests install is failing with: [ERROR] Failed to execute goal org.apache.hadoop:hadoop-maven-plugins:3.0.0-SNAPSHOT:protoc (compile-protoc) on project hadoop-yarn-api: java.io.FileNotFoundException: <my-source-code-directory>/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/proto/server (Is a directory) -> [Help 1] I did a git clean -fdx too.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 15m 33s 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 2m 14s The patch appears to cause the build to fail.



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12743831/hadoop-12194.001.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 81f3644
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7171/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 15m 33s 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 2m 14s The patch appears to cause the build to fail. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12743831/hadoop-12194.001.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 81f3644 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7171/console This message was automatically generated.
        Hide
        andrew.wang Andrew Wang added a comment -

        Hi Ravi, I thought about that, but there's no easy way to enumerate the output from the protoc generator. It ends up generating more than one class file per input protoc, and it's not possible to determine the output file name just from the input file name.

        I think CRC is also a much safer way of detecting changes, so prefer it for that reason as well. We already compute MD5s of all input files for the VersionInfo plugin too (including the proto files), so doing CRC for just the proto files shouldn't be a big overhead.

        I guess one potential enhancement is sharing the checksums between the VersionInfo and Protoc plugins, but that seems reasonable to leave for future work.

        Show
        andrew.wang Andrew Wang added a comment - Hi Ravi, I thought about that, but there's no easy way to enumerate the output from the protoc generator. It ends up generating more than one class file per input protoc, and it's not possible to determine the output file name just from the input file name. I think CRC is also a much safer way of detecting changes, so prefer it for that reason as well. We already compute MD5s of all input files for the VersionInfo plugin too (including the proto files), so doing CRC for just the proto files shouldn't be a big overhead. I guess one potential enhancement is sharing the checksums between the VersionInfo and Protoc plugins, but that seems reasonable to leave for future work.
        Hide
        raviprak Ravi Prakash added a comment -

        Hi Andrew! Thanks for the patch. Should we simply look at the modification times of the protoc files and the generated files?

        Show
        raviprak Ravi Prakash added a comment - Hi Andrew! Thanks for the patch. Should we simply look at the modification times of the protoc files and the generated files?
        Hide
        andrew.wang Andrew Wang added a comment -

        Patch attached. Saves a json file with CRC32 checksums in the build directory, and skips protoc generation if they match. I tested by running mvn -X compile twice, seeing the "Stale source" logs going way down, then modifying a proto file and seeing it recompile.

        Would appreciate some similar testing from reviewers.

        Between this and HADOOP-12193, we're much closer to a working incremental build, at least for some modules.

        Show
        andrew.wang Andrew Wang added a comment - Patch attached. Saves a json file with CRC32 checksums in the build directory, and skips protoc generation if they match. I tested by running mvn -X compile twice, seeing the "Stale source" logs going way down, then modifying a proto file and seeing it recompile. Would appreciate some similar testing from reviewers. Between this and HADOOP-12193 , we're much closer to a working incremental build, at least for some modules.

          People

          • Assignee:
            andrew.wang Andrew Wang
            Reporter:
            andrew.wang Andrew Wang
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development