Hadoop Common
  1. Hadoop Common
  2. HADOOP-7435

Make pre-commit checks run against the correct branch

    Details

    • Type: Improvement Improvement
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.23.0
    • Fix Version/s: None
    • Component/s: test
    • Labels:
      None

      Description

      The Hudson pre-commit tests are presently only capable of testing a patch against trunk. It'd be nice if this could be extended to automatically run against the correct branch.

      1. HADOOP-7435-for-branch-0.23.patch
        49 kB
        Dennis Y
      2. HADOOP-7435-for-branch-2.patch
        37 kB
        Dennis Y
      3. HADOOP-7435-for-trunk-do-not-apply-this.patch
        1 kB
        Dennis Y
      4. HADOOP-7435-branch-0.23-patch-from-[branch-0.23-gd]-to-[fb-HADOOP-7435-branch-0.23-gd]-N2-1.patch
        51 kB
        Dennis Y
      5. HADOOP-7435-branch-2--N2.patch
        41 kB
        Dennis Y
      6. HADOOP-7435-branch-0.23--N3.patch
        51 kB
        Dennis Y
      7. HADOOP-7435-branch-0.23--N5.patch
        52 kB
        Dennis Y
      8. HADOOP-7435-branch-2--N5.patch
        42 kB
        Dennis Y
      9. HADOOP-7435-trunk--N5.patch
        5 kB
        Dennis Y
      10. HADOOP-7435-branch-0.23--N6.patch
        52 kB
        Dennis Y
      11. HADOOP-7435-branch-0.23--N7.patch
        53 kB
        Dennis Y
      12. HADOOP-7435-branch-2--N7.patch
        43 kB
        Dennis Y
      13. HADOOP-7435-branch-2--N8.patch
        136 kB
        Dennis Y
      14. HADOOP-7435-branch-0.23--N2.patch
        53 kB
        Dennis Y
      15. HADOOP-7435-branch-2--N3-1.patch
        44 kB
        Dennis Y
      16. HADOOP-7435-trunk--N1.patch
        3 kB
        Dennis Y
      17. JenkinsPatchQueueAdmin.py
        9 kB
        Giridharan Kesavan

        Issue Links

          Activity

          Hide
          Aaron T. Myers added a comment -

          I'm envisioning something like this:

          1. If there's only a single "fix version" set on the relevant JIRA, try to apply the patch/test against that branch.
          2. If there's multiple "fix versions" set on the relevant JIRA, we could have a convention for naming the patch to indicate which branch it should be tested against, e.g. "*-22.patch" for 0.22.
          3. If there's no fix version set, default to testing the patch against trunk.
          Show
          Aaron T. Myers added a comment - I'm envisioning something like this: If there's only a single "fix version" set on the relevant JIRA, try to apply the patch/test against that branch. If there's multiple "fix versions" set on the relevant JIRA, we could have a convention for naming the patch to indicate which branch it should be tested against, e.g. "*-22.patch" for 0.22. If there's no fix version set, default to testing the patch against trunk.
          Hide
          Jonathan Eagles added a comment -

          Thanks, Aaron for this great idea. Here are some of my thoughts on this.

          Having patch file naming convention would allow for some interesting use cases. Though a simple version of this patch naming scheme would be a great addition, here is a full blown implementation I was thinking about.

          <ISSUE-#>-<full branch name>-v<version #>.patch

          (e.g. MAPREDUCE-2569-MR-279-v2.patch)

          • This naming scheme makes the patch completely stand-alone, allowing for emailing the patch
          • Sanity of managing multiple patch files simultaneously in dev env
          • Programmatic detection of which branch to apply
          • Versioning of patches helps those who are submitting and testing out patches, and prevents hudson/jenkins from applying several versions of the same patch and instead tests only the latest version of the patch.
          • JIRA attach file pre-commit detects "*.patch" and verifies patch naming convention.

          Personally, I do not prefer to overload "Fix version/s" since that implies what branches the patch is already committed to, leading to confusion. Additionally, newbie hadoop developers may be able to attach files to issues, but are not allowed to edit issue fields.

          Show
          Jonathan Eagles added a comment - Thanks, Aaron for this great idea. Here are some of my thoughts on this. Having patch file naming convention would allow for some interesting use cases. Though a simple version of this patch naming scheme would be a great addition, here is a full blown implementation I was thinking about. <ISSUE-#>-<full branch name>-v<version #>.patch (e.g. MAPREDUCE-2569 -MR-279-v2.patch) This naming scheme makes the patch completely stand-alone, allowing for emailing the patch Sanity of managing multiple patch files simultaneously in dev env Programmatic detection of which branch to apply Versioning of patches helps those who are submitting and testing out patches, and prevents hudson/jenkins from applying several versions of the same patch and instead tests only the latest version of the patch. JIRA attach file pre-commit detects "*.patch" and verifies patch naming convention. Personally, I do not prefer to overload "Fix version/s" since that implies what branches the patch is already committed to, leading to confusion. Additionally, newbie hadoop developers may be able to attach files to issues, but are not allowed to edit issue fields.
          Hide
          Matt Foley added a comment -

          Definitely a good idea. Does Hudson handle any arbitrary branch, or only ones that have a current nightly build job?

          I'd be okay with a patch naming convention, and don't really like overloading "Fix versions". But there is one more piece of info available: Patches generated with "svn diff" have svn release numbers next to each pre-existing file name. That release number should be uniquely associated with a particular branch, which test-patch could identify and use. But git patch tags probably don't allow for the same trick.

          At any rate, the patch naming convention has the advantage of letting the developer specify what branch to test against. That's probably the best approach.

          Show
          Matt Foley added a comment - Definitely a good idea. Does Hudson handle any arbitrary branch, or only ones that have a current nightly build job? I'd be okay with a patch naming convention, and don't really like overloading "Fix versions". But there is one more piece of info available: Patches generated with "svn diff" have svn release numbers next to each pre-existing file name. That release number should be uniquely associated with a particular branch, which test-patch could identify and use. But git patch tags probably don't allow for the same trick. At any rate, the patch naming convention has the advantage of letting the developer specify what branch to test against. That's probably the best approach.
          Hide
          Matt Foley added a comment -

          A naming convention has now been established in the wiki page HowToContribute. It says:

          Patches for trunk should be named jira-xyz.patch, eg hdfs-123.patch. Patches for a specific branch should be named jira-xyz-branch.patch, eg hdfs-123-branch-0.20-security. ... The branch name allows the pre-commit tests to run against the correct branch.

          Okay, so we need to tweak the patch-testing robot to examine the trailing set of hyphen-separated components of the patch name to see if it matches any Hadoop branch names. Not a perfect spec, but sounds usable.

          Show
          Matt Foley added a comment - A naming convention has now been established in the wiki page HowToContribute . It says: Patches for trunk should be named jira-xyz.patch, eg hdfs-123.patch. Patches for a specific branch should be named jira-xyz-branch.patch, eg hdfs-123-branch-0.20-security. ... The branch name allows the pre-commit tests to run against the correct branch. Okay, so we need to tweak the patch-testing robot to examine the trailing set of hyphen-separated components of the patch name to see if it matches any Hadoop branch names. Not a perfect spec, but sounds usable.
          Hide
          Matt Foley added a comment -

          When we succeed in providing this feature, the wiki page at HowToContribute should be updated to document the test-patch behavior.

          Show
          Matt Foley added a comment - When we succeed in providing this feature, the wiki page at HowToContribute should be updated to document the test-patch behavior.
          Hide
          Dennis Y added a comment -

          Last patch is intended for pass through pre-commit job ONLY. No need to apply 'HADOOP-7435-for-trunk-do-not-apply-this.patch'

          Show
          Dennis Y added a comment - Last patch is intended for pass through pre-commit job ONLY. No need to apply ' HADOOP-7435 -for-trunk-do-not-apply-this.patch'
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12555729/HADOOP-7435-for-trunk-do-not-apply-this.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 1 new or modified test files.

          +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.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1860//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1860//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/12555729/HADOOP-7435-for-trunk-do-not-apply-this.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +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. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1860//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1860//console This message is automatically generated.
          Hide
          Junping Du added a comment -

          Hi Matt, I think setup pre-commit check for branch-1/branch-2 is very useful. Do we have a recent plan or update on this effort?

          Show
          Junping Du added a comment - Hi Matt, I think setup pre-commit check for branch-1/branch-2 is very useful. Do we have a recent plan or update on this effort?
          Hide
          Dennis Y added a comment -

          updated patch for branch-0.23

          Show
          Dennis Y added a comment - updated patch for branch-0.23
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566749/HADOOP-7435-branch-0.23-patch-from-%5Bbranch-0.23-gd%5D-to-%5Bfb-HADOOP-7435-branch-0.23-gd%5D-N2-1.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2101//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/12566749/HADOOP-7435-branch-0.23-patch-from-%5Bbranch-0.23-gd%5D-to-%5Bfb-HADOOP-7435-branch-0.23-gd%5D-N2-1.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2101//console This message is automatically generated.
          Hide
          Dennis Y added a comment -

          fixed several more findbugs warnings

          Show
          Dennis Y added a comment - fixed several more findbugs warnings
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12567557/HADOOP-7435-branch-0.23--N3.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2130//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/12567557/HADOOP-7435-branch-0.23--N3.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2130//console This message is automatically generated.
          Hide
          Dennis Y added a comment -

          HADOOP-7435branch-0.23-N5.patch
          HADOOP-7435branch-2-N5.patch
          HADOOP-7435trunk-N5.patch
          all patches contain valid fixes for corresponding branches

          Show
          Dennis Y added a comment - HADOOP-7435 branch-0.23 -N5.patch HADOOP-7435 branch-2 -N5.patch HADOOP-7435 trunk -N5.patch all patches contain valid fixes for corresponding branches
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12567839/HADOOP-7435-trunk--N5.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-tools/hadoop-gridmix.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2140//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2140//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/12567839/HADOOP-7435-trunk--N5.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-tools/hadoop-gridmix. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2140//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2140//console This message is automatically generated.
          Hide
          Dennis Y added a comment -

          decreased OK_JAVADOC_WARNINGS down to 4 (all caused by using sun internal APIs)

          Show
          Dennis Y added a comment - decreased OK_JAVADOC_WARNINGS down to 4 (all caused by using sun internal APIs)
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12567987/HADOOP-7435-branch-0.23--N6.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2148//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/12567987/HADOOP-7435-branch-0.23--N6.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2148//console This message is automatically generated.
          Hide
          Dennis Y added a comment -

          added HADOOP-9112 (test-patch should -1 for @Tests without a timeout) for branch-2 and branch-0.23

          Show
          Dennis Y added a comment - added HADOOP-9112 (test-patch should -1 for @Tests without a timeout) for branch-2 and branch-0.23
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12570442/HADOOP-7435-branch-2--N7.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2218//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/12570442/HADOOP-7435-branch-2--N7.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2218//console This message is automatically generated.
          Hide
          Dennis Y added a comment -

          updated patch for branch-2 (resolved merge-conflict)

          Show
          Dennis Y added a comment - updated patch for branch-2 (resolved merge-conflict)
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12574130/HADOOP-7435-branch-2--N8.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2337//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/12574130/HADOOP-7435-branch-2--N8.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2337//console This message is automatically generated.
          Hide
          Dennis Y added a comment -

          updated patches for branch-2 and branch-0.23 according to revertion of HADOOP-9112

          Show
          Dennis Y added a comment - updated patches for branch-2 and branch-0.23 according to revertion of HADOOP-9112
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12576360/HADOOP-7435-branch-2--N3-1.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2388//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/12576360/HADOOP-7435-branch-2--N3-1.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2388//console This message is automatically generated.
          Hide
          Dennis Y added a comment -

          resolved merge conflicts

          Show
          Dennis Y added a comment - resolved merge conflicts
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12576962/HADOOP-7435-trunk--N1.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 generated 1375 javac compiler warnings (more than the trunk's current 1367 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-tools/hadoop-gridmix.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2412//testReport/
          Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/2412//artifact/trunk/patchprocess/diffJavacWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2412//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/12576962/HADOOP-7435-trunk--N1.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 generated 1375 javac compiler warnings (more than the trunk's current 1367 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-tools/hadoop-gridmix. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2412//testReport/ Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/2412//artifact/trunk/patchprocess/diffJavacWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2412//console This message is automatically generated.
          Hide
          Giridharan Kesavan added a comment -

          JenkinsPatchQueueAdmin.py script would download the jira filter and schedules pre-commit job's for multiple projects.

          Admin job also figures out the branch name of a patch from the patch name by comparing it against the svn branches. It uses Jira project name to schedule the preCommit job by passing in the branch name and the latest attachment id for that given branch.

          Admin script is written in python which uses ElementTree and Jenkins python module.

          Show
          Giridharan Kesavan added a comment - JenkinsPatchQueueAdmin.py script would download the jira filter and schedules pre-commit job's for multiple projects. Admin job also figures out the branch name of a patch from the patch name by comparing it against the svn branches. It uses Jira project name to schedule the preCommit job by passing in the branch name and the latest attachment id for that given branch. Admin script is written in python which uses ElementTree and Jenkins python module.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12583870/JenkinsPatchQueueAdmin.py
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2550//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/12583870/JenkinsPatchQueueAdmin.py against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2550//console This message is automatically generated.
          Hide
          Giridharan Kesavan added a comment -

          linking the jira that fixes test-patch.sh for branch-1

          Show
          Giridharan Kesavan added a comment - linking the jira that fixes test-patch.sh for branch-1

            People

            • Assignee:
              Matt Foley
              Reporter:
              Aaron T. Myers
            • Votes:
              1 Vote for this issue
              Watchers:
              20 Start watching this issue

              Dates

              • Created:
                Updated:

                Development