Hadoop Common
  1. Hadoop Common
  2. HADOOP-9246

Execution phase for hadoop-maven-plugin should be process-resources

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: trunk-win, 3.0.0
    • Fix Version/s: 2.1.0-beta
    • Component/s: build
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Per discussion on HADOOP-9245, the execution phase of hadoop-maven-plugin should be process-resources and not compile.

      1. HADOOP-9246-addendum.patch
        2 kB
        Alejandro Abdelnur
      2. hadoop-9246.patch
        1 kB
        Karthik Kambatla
      3. hadoop-9246.patch
        1 kB
        Karthik Kambatla
      4. HADOOP-9246.2.patch
        2 kB
        Chris Nauroth

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          9m 9s 1 Karthik Kambatla (Inactive) 24/Jan/13 23:10
          Patch Available Patch Available Resolved Resolved
          3d 21h 42m 1 Jason Lowe 28/Jan/13 20:53
          Resolved Resolved Reopened Reopened
          15d 20h 57m 1 Alejandro Abdelnur 13/Feb/13 17:51
          Reopened Reopened Resolved Resolved
          21m 27s 1 Alejandro Abdelnur 13/Feb/13 18:12
          Resolved Resolved Closed Closed
          195d 3h 53m 1 Arun C Murthy 27/Aug/13 23:06
          Gavin made changes -
          Reporter Karthik Kambatla [ kkambatl ] Karthik Kambatla [ kasha ]
          Gavin made changes -
          Assignee Karthik Kambatla [ kkambatl ] Karthik Kambatla [ kasha ]
          Arun C Murthy made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Alejandro Abdelnur made changes -
          Fix Version/s 2.0.4-beta [ 12324030 ]
          Fix Version/s 3.0.0 [ 12320357 ]
          Alejandro Abdelnur made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Alejandro Abdelnur added a comment -

          arghh, got it, i was missing that. Closing it then.

          I guess we have to do the same for the protoc plugin, HADOOP-9117, will test there.

          Show
          Alejandro Abdelnur added a comment - arghh, got it, i was missing that. Closing it then. I guess we have to do the same for the protoc plugin, HADOOP-9117 , will test there.
          Hide
          Chris Nauroth added a comment -

          Hi Alejandro,

          The addendum patch would re-break HADOOP-9245 (mvn clean without running mvn install before fails). Without specifying phase explicitly in the pom.xml, Maven needs to look at the mojo definition to determine default phase. Unfortunately, if you don't have hadoop-maven-plugins installed yet, then there is no mojo definition to inspect, and mvn clean fails.

          Of course, we can keep the change to add defaultPhase in the mojo code. I think we just also need to keep <phase> in the pom.xml files. +1 for just the change in VersionInfoMojo.

          Thanks!

          Show
          Chris Nauroth added a comment - Hi Alejandro, The addendum patch would re-break HADOOP-9245 (mvn clean without running mvn install before fails). Without specifying phase explicitly in the pom.xml, Maven needs to look at the mojo definition to determine default phase. Unfortunately, if you don't have hadoop-maven-plugins installed yet, then there is no mojo definition to inspect, and mvn clean fails. Of course, we can keep the change to add defaultPhase in the mojo code. I think we just also need to keep <phase> in the pom.xml files. +1 for just the change in VersionInfoMojo . Thanks!
          Alejandro Abdelnur made changes -
          Attachment HADOOP-9246-addendum.patch [ 12569230 ]
          Hide
          Alejandro Abdelnur added a comment -

          addendum patch moving the phase binding to the plugin definition.

          Show
          Alejandro Abdelnur added a comment - addendum patch moving the phase binding to the plugin definition.
          Alejandro Abdelnur made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Alejandro Abdelnur added a comment -

          I think a more correct way of fixing this (realized that while doing the protoc plugin, HADOOP-9117) is setting the default phase in the annotation of the plugin.

          Show
          Alejandro Abdelnur added a comment - I think a more correct way of fixing this (realized that while doing the protoc plugin, HADOOP-9117 ) is setting the default phase in the annotation of the plugin.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1328 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1328/)
          HADOOP-9246. Execution phase for hadoop-maven-plugin should be process-resources. Contributed by Karthik Kambatla and Chris Nauroth (Revision 1439620)

          Result = SUCCESS
          jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1439620
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/pom.xml
          • /hadoop/common/trunk/hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/versioninfo/VersionInfoMojo.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/pom.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1328 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1328/ ) HADOOP-9246 . Execution phase for hadoop-maven-plugin should be process-resources. Contributed by Karthik Kambatla and Chris Nauroth (Revision 1439620) Result = SUCCESS jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1439620 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/pom.xml /hadoop/common/trunk/hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/versioninfo/VersionInfoMojo.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/pom.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1300 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1300/)
          HADOOP-9246. Execution phase for hadoop-maven-plugin should be process-resources. Contributed by Karthik Kambatla and Chris Nauroth (Revision 1439620)

          Result = SUCCESS
          jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1439620
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/pom.xml
          • /hadoop/common/trunk/hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/versioninfo/VersionInfoMojo.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/pom.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1300 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1300/ ) HADOOP-9246 . Execution phase for hadoop-maven-plugin should be process-resources. Contributed by Karthik Kambatla and Chris Nauroth (Revision 1439620) Result = SUCCESS jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1439620 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/pom.xml /hadoop/common/trunk/hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/versioninfo/VersionInfoMojo.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/pom.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #111 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/111/)
          HADOOP-9246. Execution phase for hadoop-maven-plugin should be process-resources. Contributed by Karthik Kambatla and Chris Nauroth (Revision 1439620)

          Result = SUCCESS
          jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1439620
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/pom.xml
          • /hadoop/common/trunk/hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/versioninfo/VersionInfoMojo.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/pom.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #111 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/111/ ) HADOOP-9246 . Execution phase for hadoop-maven-plugin should be process-resources. Contributed by Karthik Kambatla and Chris Nauroth (Revision 1439620) Result = SUCCESS jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1439620 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/pom.xml /hadoop/common/trunk/hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/versioninfo/VersionInfoMojo.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/pom.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3289 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3289/)
          HADOOP-9246. Execution phase for hadoop-maven-plugin should be process-resources. Contributed by Karthik Kambatla and Chris Nauroth (Revision 1439620)

          Result = SUCCESS
          jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1439620
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/pom.xml
          • /hadoop/common/trunk/hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/versioninfo/VersionInfoMojo.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/pom.xml
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3289 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3289/ ) HADOOP-9246 . Execution phase for hadoop-maven-plugin should be process-resources. Contributed by Karthik Kambatla and Chris Nauroth (Revision 1439620) Result = SUCCESS jlowe : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1439620 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/pom.xml /hadoop/common/trunk/hadoop-maven-plugins/src/main/java/org/apache/hadoop/maven/plugin/versioninfo/VersionInfoMojo.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/pom.xml
          Jason Lowe made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Fix Version/s 3.0.0 [ 12320357 ]
          Resolution Fixed [ 1 ]
          Hide
          Jason Lowe added a comment -

          Thanks Karthik and Chris! I committed this to trunk.

          Show
          Jason Lowe added a comment - Thanks Karthik and Chris! I committed this to trunk.
          Hide
          Jason Lowe added a comment -

          +1

          Show
          Jason Lowe added a comment - +1
          Jason Lowe made changes -
          Link This issue is duplicated by YARN-361 [ YARN-361 ]
          Jason Lowe made changes -
          Link This issue blocks YARN-361 [ YARN-361 ]
          Karthik Kambatla (Inactive) made changes -
          Link This issue blocks YARN-361 [ YARN-361 ]
          Hide
          Chris Nauroth added a comment -

          The -1 from Jenkins for lack of tests is expected, because this is a build script change.

          Show
          Chris Nauroth added a comment - The -1 from Jenkins for lack of tests is expected, because this is a build script change.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566529/HADOOP-9246.2.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 passed unit tests in hadoop-common-project/hadoop-common hadoop-maven-plugins hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2094//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2094//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/12566529/HADOOP-9246.2.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 passed unit tests in hadoop-common-project/hadoop-common hadoop-maven-plugins hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2094//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2094//console This message is automatically generated.
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Chris - thanks for correcting the patch. The latest patch looks good to me. +1

          Show
          Karthik Kambatla (Inactive) added a comment - Chris - thanks for correcting the patch. The latest patch looks good to me. +1
          Chris Nauroth made changes -
          Attachment HADOOP-9246.2.patch [ 12566529 ]
          Hide
          Chris Nauroth added a comment -

          FYI, the HADOOP-9245 patch caused the built version info properties files to be incorrect. Running in the compile phase populated the version info properties too late, and then the filtering left tokens unreplaced like $

          {version-info.build.time}

          . Sorry for not catching this during my code review of HADOOP-9245.

          Ultimately, this caused a test failure in YARN's TestNMWebServices while trying to parse the date returned from the web service. I retested this with hadoop-9246.patch, which was binding to the prepare-resources phase, but it didn't fix the problem.

          I'm uploading HADOOP-9246.2.patch, which binds to the generate-resources phase, which runs before resource filtering. Once I did this, the version info properties files were populated correctly, and TestNMWebServices passed. I've also removed the defaultPhase from the mojo annotation.

          Show
          Chris Nauroth added a comment - FYI, the HADOOP-9245 patch caused the built version info properties files to be incorrect. Running in the compile phase populated the version info properties too late, and then the filtering left tokens unreplaced like $ {version-info.build.time} . Sorry for not catching this during my code review of HADOOP-9245 . Ultimately, this caused a test failure in YARN's TestNMWebServices while trying to parse the date returned from the web service. I retested this with hadoop-9246.patch, which was binding to the prepare-resources phase, but it didn't fix the problem. I'm uploading HADOOP-9246 .2.patch, which binds to the generate-resources phase, which runs before resource filtering. Once I did this, the version info properties files were populated correctly, and TestNMWebServices passed. I've also removed the defaultPhase from the mojo annotation.
          Hide
          Alejandro Abdelnur added a comment -

          Chris, you right, we should.

          Show
          Alejandro Abdelnur added a comment - Chris, you right, we should.
          Hide
          Chris Nauroth added a comment -

          What about the annotation of the default phase in VersionInfoMojo? Should we change that too?

          @Mojo(name="version-info", defaultPhase=LifecyclePhase.INITIALIZE)
          public class VersionInfoMojo extends AbstractMojo {
          

          I think we still need to be explicit about the <phase> in the pom.xml files to prevent the HADOOP-9245 problem, but I'm wondering if we should update the annotation for consistency, or possibly just remove defaultPhase and require users to be specific.

          Show
          Chris Nauroth added a comment - What about the annotation of the default phase in VersionInfoMojo ? Should we change that too? @Mojo(name= "version-info" , defaultPhase=LifecyclePhase.INITIALIZE) public class VersionInfoMojo extends AbstractMojo { I think we still need to be explicit about the <phase> in the pom.xml files to prevent the HADOOP-9245 problem, but I'm wondering if we should update the annotation for consistency, or possibly just remove defaultPhase and require users to be specific.
          Karthik Kambatla (Inactive) made changes -
          Link This issue is broken by HADOOP-9245 [ HADOOP-9245 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566401/hadoop-9246.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 passed unit tests in hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2089//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2089//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/12566401/hadoop-9246.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 passed unit tests in hadoop-common-project/hadoop-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2089//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2089//console This message is automatically generated.
          Hide
          Alejandro Abdelnur added a comment -

          +1

          Show
          Alejandro Abdelnur added a comment - +1
          Karthik Kambatla (Inactive) made changes -
          Description Per discussion on HADOOP-9245, the execution phase of hadoop-maven-plugin should be _prepare-resources_ and not _compile_. Per discussion on HADOOP-9245, the execution phase of hadoop-maven-plugin should be _process-resources_ and not _compile_.
          Karthik Kambatla (Inactive) made changes -
          Summary Execution phase for hadoop-maven-plugin should be prepare-resources Execution phase for hadoop-maven-plugin should be process-resources
          Karthik Kambatla (Inactive) made changes -
          Attachment hadoop-9246.patch [ 12566401 ]
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Odd that prepare-resources worked. Per http://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html#Setting_Up_Your_Project_to_Use_the_Build_Lifecycle , it should be process-resources.

          Uploading a new patch with process-resources.

          Nuked hadoop-maven-plugins from .m2/repository, and ran mvn clean; mvn install successfully.

          Show
          Karthik Kambatla (Inactive) added a comment - Odd that prepare-resources worked. Per http://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html#Setting_Up_Your_Project_to_Use_the_Build_Lifecycle , it should be process-resources. Uploading a new patch with process-resources. Nuked hadoop-maven-plugins from .m2/repository, and ran mvn clean; mvn install successfully.
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Verified locally that clean and install work (in that order) on fresh m2 repo.

          Show
          Karthik Kambatla (Inactive) added a comment - Verified locally that clean and install work (in that order) on fresh m2 repo.
          Karthik Kambatla (Inactive) made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Karthik Kambatla (Inactive) made changes -
          Description Per discussion on HADOOP-9245, the execution phase of hadoop-maven-plugin should be _prepare-resources_ and not _compile_.
          Karthik Kambatla (Inactive) made changes -
          Field Original Value New Value
          Attachment hadoop-9246.patch [ 12566399 ]
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Trivial patch to fix the issue.

          Show
          Karthik Kambatla (Inactive) added a comment - Trivial patch to fix the issue.
          Karthik Kambatla (Inactive) created issue -

            People

            • Assignee:
              Karthik Kambatla
              Reporter:
              Karthik Kambatla
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development