Details

    • Sub-task
    • Status: Resolved
    • Blocker
    • Resolution: Fixed
    • HADOOP-12111
    • HADOOP-12111
    • yetus
    • None
    • Incompatible change

    Description

      WARNING: this is a fairly big project.

      See first comment for a brain dump on the issues.

      Attachments

        1. HADOOP-12129.HADOOP-12111.00.patch
          33 kB
          Allen Wittenauer
        2. HADOOP-12129.HADOOP-12111.01.patch
          41 kB
          Allen Wittenauer
        3. HADOOP-12129.HADOOP-12111.02.patch
          44 kB
          Allen Wittenauer
        4. HADOOP-12129.HADOOP-12111.03.patch
          63 kB
          Allen Wittenauer
        5. HADOOP-12129.HADOOP-12111.04.patch
          71 kB
          Allen Wittenauer
        6. HADOOP-12129.HADOOP-12111.05.patch
          74 kB
          Allen Wittenauer
        7. HADOOP-12129.HADOOP-12111.06.patch
          74 kB
          Allen Wittenauer

        Issue Links

          Activity

            There are a bunch of interleaved issues in play here. This is a summary of some of the discussions that sekikn and I had around these topics.

            • Some projects use github PRs as their patch repository, with comments going to both github and JIRA. In these projects (e.g., tajo), JIRA status is never patch available and a comment is made by an ASF infra bot in the JIRA that points to the github PR.
            • It's not uncommon to have JIRA point to another system for code reviews and patch repository via JIRA APIs.
            • In the future, we should be able to support more than one bug system simultaneously. e.g., github:project/# jira:project/# bugzilla:project/# would push a precommit message to all three.
            • In the far future, it'd be nice to be able to support multiples of the same comment system.
            • Bonus points: How does reviewboard, gerrit, etc fit into this?

            At a minimum, I think we need to do this list sooner rather than later:

            a) pull more of the JIRA-specific code out of locate_patch and into the jira.sh plugin
            b) support the not-patch-available-but-referenced-in-JIRA-comment use case

            aw Allen Wittenauer added a comment - There are a bunch of interleaved issues in play here. This is a summary of some of the discussions that sekikn and I had around these topics. Some projects use github PRs as their patch repository, with comments going to both github and JIRA. In these projects (e.g., tajo), JIRA status is never patch available and a comment is made by an ASF infra bot in the JIRA that points to the github PR. It's not uncommon to have JIRA point to another system for code reviews and patch repository via JIRA APIs. In the future, we should be able to support more than one bug system simultaneously. e.g., github:project/# jira:project/# bugzilla:project/# would push a precommit message to all three. In the far future, it'd be nice to be able to support multiples of the same comment system. Bonus points: How does reviewboard, gerrit, etc fit into this? At a minimum, I think we need to do this list sooner rather than later: a) pull more of the JIRA-specific code out of locate_patch and into the jira.sh plugin b) support the not-patch-available-but-referenced-in-JIRA-comment use case

            -00:

            • this is incomplete; many todo's
            • move the logic of patch downloading to a plugin
            • flesh out more of the github support
            • move the console output to a 'fake' bug system plugin
            • fix a few bugs here and there
            • change more local's to declare's
            • now run through all of the bug systems to allow posting comments to multiple systems at once.
            • break write_comment stability
            • change ISSUE_RE to JIRA_ISSUE_RE
            • we might support more than http URLs now, but I need to test it....
            • fix a few offline bits that were missing
            • jira now uses username/pass much more than it did
            • jira should work on non-apache setups now (altho we really need to do more RESTy things rather than depending upon jira-cli.)
            • may have found a bug with shellcheck wrt declare -a...
            • removed a few dead vars
            aw Allen Wittenauer added a comment - -00: this is incomplete; many todo's move the logic of patch downloading to a plugin flesh out more of the github support move the console output to a 'fake' bug system plugin fix a few bugs here and there change more local's to declare's now run through all of the bug systems to allow posting comments to multiple systems at once. break write_comment stability change ISSUE_RE to JIRA_ISSUE_RE we might support more than http URLs now, but I need to test it.... fix a few offline bits that were missing jira now uses username/pass much more than it did jira should work on non-apache setups now (altho we really need to do more RESTy things rather than depending upon jira-cli.) may have found a bug with shellcheck wrt declare -a... removed a few dead vars
            hadoopqa Hadoop QA added a comment -

            A patch to the files used for the QA process has been detected.
            Re-executing against the patched versions to perform further tests.
            The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7422/console in case of problems.

            hadoopqa Hadoop QA added a comment - A patch to the files used for the QA process has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7422/console in case of problems.

            Hmm. Clearly some bugs, but more troubling is it looks like this might not be compatible with the old reexec system.

            aw Allen Wittenauer added a comment - Hmm. Clearly some bugs, but more troubling is it looks like this might not be compatible with the old reexec system.

            The sort pipe error is HADOOP-12310 .

            aw Allen Wittenauer added a comment - The sort pipe error is HADOOP-12310 .
            aw Allen Wittenauer added a comment - - edited

            OK, I've figured out how to remove the JIRA CLI requirement. This is important because (at least the version with the least encumbered license) uses SOAP, an interface that Atlassian is disabling everywhere.

            HOWEVER.

            It means using curl because I could not get this work with wget no matter what I tried.. So I'm going to switch all the wgets to curls.

            (cc: abayer , who might be interested in this detail.)

            aw Allen Wittenauer added a comment - - edited OK, I've figured out how to remove the JIRA CLI requirement. This is important because (at least the version with the least encumbered license) uses SOAP, an interface that Atlassian is disabling everywhere. HOWEVER. It means using curl because I could not get this work with wget no matter what I tried.. So I'm going to switch all the wgets to curls. (cc: abayer , who might be interested in this detail.)

            -01:

            • switch from wget and jira cli to tools to curl + jira REST API
            • issue determination bugfix
            aw Allen Wittenauer added a comment - -01: switch from wget and jira cli to tools to curl + jira REST API issue determination bugfix
            hadoopqa Hadoop QA added a comment -

            A patch to the files used for the QA process has been detected.
            Re-executing against the patched versions to perform further tests.
            The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7427/console in case of problems.

            hadoopqa Hadoop QA added a comment - A patch to the files used for the QA process has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7427/console in case of problems.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 0s precommit patch detected.
            +1 site 0m 0s HADOOP-12111 passed
            0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched.
            +1 site 0m 0s the patch passed
            +1 asflicense 0m 20s Patch does not generate ASF License warnings.
            -1 shellcheck 0m 11s The applied patch generated 6 new shellcheck issues (total was 22, now 28).
            +1 whitespace 0m 0s Patch has no whitespace issues.
            0m 37s



            Subsystem Report/Notes
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12749398/HADOOP-12129.HADOOP-12111.01.patch
            JIRA Issue HADOOP-12129
            git revision HADOOP-12111 / c393182
            Optional Tests asflicense site shellcheck
            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
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build@2/patchprocess/dev-support-test/personality/hadoop.sh
            Default Java 1.7.0_55
            Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55
            shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.)
            shellcheck https://builds.apache.org/job/PreCommit-HADOOP-Build/7427/artifact/patchprocess/diff-patch-shellcheck.txt
            Max memory used 49MB
            Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7427/console

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s precommit patch detected. +1 site 0m 0s HADOOP-12111 passed 0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched. +1 site 0m 0s the patch passed +1 asflicense 0m 20s Patch does not generate ASF License warnings. -1 shellcheck 0m 11s The applied patch generated 6 new shellcheck issues (total was 22, now 28). +1 whitespace 0m 0s Patch has no whitespace issues. 0m 37s Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12749398/HADOOP-12129.HADOOP-12111.01.patch JIRA Issue HADOOP-12129 git revision HADOOP-12111 / c393182 Optional Tests asflicense site shellcheck 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 Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build@2/patchprocess/dev-support-test/personality/hadoop.sh Default Java 1.7.0_55 Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55 shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.) shellcheck https://builds.apache.org/job/PreCommit-HADOOP-Build/7427/artifact/patchprocess/diff-patch-shellcheck.txt Max memory used 49MB Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7427/console This message was automatically generated.

            -02:

            • fix some bugs with curl's usage in smart-apply-patch
            • github support works! both basic and token for auth!
            • jira plugin now has some simple/basic support of switching to github when it detects a pull request in the comments
            • removed jira-cmd parse arg since we don't need it anymore
            • some hard-coded sed's moved to use the sed var
            • jira was forcing it's header into github comments
            aw Allen Wittenauer added a comment - -02: fix some bugs with curl's usage in smart-apply-patch github support works! both basic and token for auth! jira plugin now has some simple/basic support of switching to github when it detects a pull request in the comments removed jira-cmd parse arg since we don't need it anymore some hard-coded sed's moved to use the sed var jira was forcing it's header into github comments
            hadoopqa Hadoop QA added a comment -

            A patch to the files used for the QA process has been detected.
            Re-executing against the patched versions to perform further tests.
            The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7434/console in case of problems.

            hadoopqa Hadoop QA added a comment - A patch to the files used for the QA process has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7434/console in case of problems.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 0s precommit patch detected.
            +1 site 0m 0s HADOOP-12111 passed
            0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched.
            -1 test4tests 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 site 0m 0s the patch passed
            +1 asflicense 0m 31s Patch does not generate ASF License warnings.
            -1 shellcheck 0m 12s The applied patch generated 2 new shellcheck issues (total was 22, now 24).
            +1 whitespace 0m 0s Patch has no whitespace issues.
            0m 48s



            Subsystem Report/Notes
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12749721/HADOOP-12129.HADOOP-12111.02.patch
            JIRA Issue HADOOP-12129
            git revision HADOOP-12111 / 8726069
            Optional Tests asflicense site unit shellcheck
            uname Linux asf906.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
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh
            Default Java 1.7.0_55
            Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55
            shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.)
            shellcheck https://builds.apache.org/job/PreCommit-HADOOP-Build/7434/artifact/patchprocess/diff-patch-shellcheck.txt
            JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7434/testReport/
            Max memory used 49MB
            Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7434/console

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s precommit patch detected. +1 site 0m 0s HADOOP-12111 passed 0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched. -1 test4tests 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 site 0m 0s the patch passed +1 asflicense 0m 31s Patch does not generate ASF License warnings. -1 shellcheck 0m 12s The applied patch generated 2 new shellcheck issues (total was 22, now 24). +1 whitespace 0m 0s Patch has no whitespace issues. 0m 48s Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12749721/HADOOP-12129.HADOOP-12111.02.patch JIRA Issue HADOOP-12129 git revision HADOOP-12111 / 8726069 Optional Tests asflicense site unit shellcheck uname Linux asf906.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 Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh Default Java 1.7.0_55 Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55 shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.) shellcheck https://builds.apache.org/job/PreCommit-HADOOP-Build/7434/artifact/patchprocess/diff-patch-shellcheck.txt JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7434/testReport/ Max memory used 49MB Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7434/console This message was automatically generated.

            -03:

            • many, many, many bug fixes
            • moar documentation!
            • jira<->github bridges work much better now and truly are bi
            • branch detection now done on a per-bug system basis
            • basic branch detection on github PRs
            • plugins can now write comments to bug systems that have review capabilities. shellcheck and github are only ones that currently support this
            • github pull request links on the command line is much smarter
            • better authentication done
            • a few more globals killed to clean things up more
            aw Allen Wittenauer added a comment - -03: many, many, many bug fixes moar documentation! jira<->github bridges work much better now and truly are bi branch detection now done on a per-bug system basis basic branch detection on github PRs plugins can now write comments to bug systems that have review capabilities. shellcheck and github are only ones that currently support this github pull request links on the command line is much smarter better authentication done a few more globals killed to clean things up more

            On the testing front, after -02, I've been using this patch to develop against github directly (using an oauth token to write back to my PR!) without any interaction with JIRA at all, except for an occasional "I didn't break anything right?". I'm much more confident now that the interfaces make sense and work. Still need to do more documentation here, esp since I know we need to write a plugin for reviewboard at some point.

            That said, this is a BIG patch (as expected) and it'd be good for folks to start digging into it. Most of the major work is done and everything should be relatively small bug fixes from here on out!

            aw Allen Wittenauer added a comment - On the testing front, after -02, I've been using this patch to develop against github directly (using an oauth token to write back to my PR!) without any interaction with JIRA at all, except for an occasional "I didn't break anything right?". I'm much more confident now that the interfaces make sense and work. Still need to do more documentation here, esp since I know we need to write a plugin for reviewboard at some point. That said, this is a BIG patch (as expected) and it'd be good for folks to start digging into it. Most of the major work is done and everything should be relatively small bug fixes from here on out!
            hadoopqa Hadoop QA added a comment -

            A patch to the files used for the QA process has been detected.
            Re-executing against the patched versions to perform further tests.
            The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7458/console in case of problems.

            hadoopqa Hadoop QA added a comment - A patch to the files used for the QA process has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7458/console in case of problems.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 0s precommit patch detected.
            +1 site 0m 0s HADOOP-12111 passed
            0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched.
            -1 test4tests 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 site 0m 0s the patch passed
            +1 asflicense 0m 22s Patch does not generate ASF License warnings.
            +1 shellcheck 0m 13s There were no new shellcheck issues.
            +1 whitespace 0m 0s Patch has no whitespace issues.
            0m 41s



            Subsystem Report/Notes
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12750217/HADOOP-12129.HADOOP-12111.03.patch
            JIRA Issue HADOOP-12129
            git revision HADOOP-12111 / 96f2745
            Optional Tests asflicense site unit shellcheck
            uname Linux asf906.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
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh
            Default Java 1.7.0_55
            Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55
            shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.)
            JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7458/testReport/
            Max memory used 49MB
            Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7458/console

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s precommit patch detected. +1 site 0m 0s HADOOP-12111 passed 0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched. -1 test4tests 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 site 0m 0s the patch passed +1 asflicense 0m 22s Patch does not generate ASF License warnings. +1 shellcheck 0m 13s There were no new shellcheck issues. +1 whitespace 0m 0s Patch has no whitespace issues. 0m 41s Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12750217/HADOOP-12129.HADOOP-12111.03.patch JIRA Issue HADOOP-12129 git revision HADOOP-12111 / 96f2745 Optional Tests asflicense site unit shellcheck uname Linux asf906.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 Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh Default Java 1.7.0_55 Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55 shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.) JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7458/testReport/ Max memory used 49MB Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7458/console This message was automatically generated.
            aw Allen Wittenauer added a comment - - edited

            -04:

            • some doc fixes/additions/cleanup
            • fix some leaky variables
            • point personalities to their github equivalents and set more reasonable defaults
            • added some code comments
            • fixed a minor bug with GH pull requests given on the command line
            • force GH to use v3 API
            • reformat the REST conversion bits to be a bit cleaner
            • whitespace now writes line comments
            • fix a bug where if the JIRA issue that was given as input was also a branch name, we don't switch to that branch
            • if we can't write to a system because of creds, report it.
            • guess_patch_file would throw errors if the input file didn't exist. the new locate_patch code can trigger this condition whereas before it didn't.
            • bugysstem line comments now take a header so that plugins can report which plugin actually generated the message
            • renamed bugsystem_output to bugsystem_finalreport to better reflect reality
            aw Allen Wittenauer added a comment - - edited -04: some doc fixes/additions/cleanup fix some leaky variables point personalities to their github equivalents and set more reasonable defaults added some code comments fixed a minor bug with GH pull requests given on the command line force GH to use v3 API reformat the REST conversion bits to be a bit cleaner whitespace now writes line comments fix a bug where if the JIRA issue that was given as input was also a branch name, we don't switch to that branch if we can't write to a system because of creds, report it. guess_patch_file would throw errors if the input file didn't exist. the new locate_patch code can trigger this condition whereas before it didn't. bugysstem line comments now take a header so that plugins can report which plugin actually generated the message renamed bugsystem_output to bugsystem_finalreport to better reflect reality
            hadoopqa Hadoop QA added a comment -

            A patch to the files used for the QA process has been detected.
            Re-executing against the patched versions to perform further tests.
            The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7462/console in case of problems.

            hadoopqa Hadoop QA added a comment - A patch to the files used for the QA process has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7462/console in case of problems.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 0s precommit patch detected.
            +1 site 0m 0s HADOOP-12111 passed
            0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched.
            -1 test4tests 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 site 0m 0s the patch passed
            +1 asflicense 0m 15s Patch does not generate ASF License warnings.
            -1 shellcheck 0m 11s The applied patch generated 1 new shellcheck issues (total was 22, now 23).
            +1 whitespace 0m 0s Patch has no whitespace issues.
            0m 32s



            Subsystem Report/Notes
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12750358/HADOOP-12129.HADOOP-12111.04.patch
            JIRA Issue HADOOP-12129
            git revision HADOOP-12111 / 13da896
            Optional Tests asflicense site unit shellcheck
            uname Linux asf906.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
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh
            Default Java 1.7.0_55
            Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55
            shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.)
            shellcheck https://builds.apache.org/job/PreCommit-HADOOP-Build/7462/artifact/patchprocess/diff-patch-shellcheck.txt
            JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7462/testReport/
            Max memory used 49MB
            Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7462/console

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s precommit patch detected. +1 site 0m 0s HADOOP-12111 passed 0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched. -1 test4tests 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 site 0m 0s the patch passed +1 asflicense 0m 15s Patch does not generate ASF License warnings. -1 shellcheck 0m 11s The applied patch generated 1 new shellcheck issues (total was 22, now 23). +1 whitespace 0m 0s Patch has no whitespace issues. 0m 32s Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12750358/HADOOP-12129.HADOOP-12111.04.patch JIRA Issue HADOOP-12129 git revision HADOOP-12111 / 13da896 Optional Tests asflicense site unit shellcheck uname Linux asf906.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 Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh Default Java 1.7.0_55 Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55 shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.) shellcheck https://builds.apache.org/job/PreCommit-HADOOP-Build/7462/artifact/patchprocess/diff-patch-shellcheck.txt JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7462/testReport/ Max memory used 49MB Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7462/console This message was automatically generated.
            aw Allen Wittenauer added a comment - - edited

            So I've found an interesting bug, but frankly, it's too hard to fix and a lot of it goes back to the brokenness of the github API.

            If a file has been modified in two different commits to the point that they show up as different entries even in the github UI, then our comment on the line is going to be probably on the wrong line, if the github API even accepts it. The only way to fix this is to figure out which commit id hash applies to which line change.

            A good workaround is really to recommend people to rebase the changes to be in as few commits as possible.

            aw Allen Wittenauer added a comment - - edited So I've found an interesting bug, but frankly, it's too hard to fix and a lot of it goes back to the brokenness of the github API. If a file has been modified in two different commits to the point that they show up as different entries even in the github UI, then our comment on the line is going to be probably on the wrong line, if the github API even accepts it. The only way to fix this is to figure out which commit id hash applies to which line change. A good workaround is really to recommend people to rebase the changes to be in as few commits as possible.
            busbey Sean Busbey added a comment -
            +
            +* pluginname_writecomment
            +
            +    - Given text input, write this output to the bug system as a comment.  NOTE: It is the bug system's responsibility to format appropriately.
            +
            +* pluginname\_linecomments
            +
            +    - This function allows for the system to write specific comments on specific lines if the bug system supports code review comments.
            +
            +* pluginname_finalreport
            +
            

            the first and second bullet points should use \ to escape the _

             
            -* ISSUE\_RE is to help test-patch when talking to JIRA.  It helps determine if the given project is appropriate for the given JIRA issue.
            +* JIRA\_ISSUE\_RE is to help test-patch when talking to JIRA.  It helps determine if the given project is appropriate for the given JIRA issue.  There are similar variables for GITHUB.
             

            Give a pointer to the variable or say what it is? It's GITHUB_ISSUE, and without other context this paragraph would have made me presume GITHUB_ISSUE_RE.

            -* wget
            +* curl
            

            yay!

            +
            +A new practice within the ASF is to use a service such as GitHub and its Pull Request (PR) feature.  test-patch supports many forms of providing PR support.
            +
            

            nit: I don't think we need to mention the ASF use here, since our goal is larger.

            +      ${CURL} --silent \
            +              --output "${PATCH_DIR}/jira" \
            +              --location \
            +             "http://issues.apache.org/jira/browse/${PATCH_OR_ISSUE}"
            

            nit: maybe different issue. can we just start at the https url? also maybe we need some link about setting up https support for curl. and maybe a warning that you should trust your issue tracker since we use --location (though this is very minor, given our broader context).

            +# personalities can override the following settings:
            +
            +# Web interface URL.
            +GITHUB_BASE_URL="https://github.com"
            +
            +# API interface URL.
            +GITHUB_API_URL="https://api.github.com"
            +
            

            does this mean we can support github enterprise installations?!

            (looks like barring the note about jira issues that point at github. sweet)

            busbey Sean Busbey added a comment - + +* pluginname_writecomment + + - Given text input, write this output to the bug system as a comment. NOTE: It is the bug system's responsibility to format appropriately. + +* pluginname\_linecomments + + - This function allows for the system to write specific comments on specific lines if the bug system supports code review comments. + +* pluginname_finalreport + the first and second bullet points should use \ to escape the _ -* ISSUE\_RE is to help test-patch when talking to JIRA. It helps determine if the given project is appropriate for the given JIRA issue. +* JIRA\_ISSUE\_RE is to help test-patch when talking to JIRA. It helps determine if the given project is appropriate for the given JIRA issue. There are similar variables for GITHUB. Give a pointer to the variable or say what it is? It's GITHUB_ISSUE, and without other context this paragraph would have made me presume GITHUB_ISSUE_RE. -* wget +* curl yay! + +A new practice within the ASF is to use a service such as GitHub and its Pull Request (PR) feature. test-patch supports many forms of providing PR support. + nit: I don't think we need to mention the ASF use here, since our goal is larger. + ${CURL} --silent \ + --output "${PATCH_DIR}/jira" \ + --location \ + "http://issues.apache.org/jira/browse/${PATCH_OR_ISSUE}" nit: maybe different issue. can we just start at the https url? also maybe we need some link about setting up https support for curl. and maybe a warning that you should trust your issue tracker since we use --location (though this is very minor, given our broader context). +# personalities can override the following settings: + +# Web interface URL. +GITHUB_BASE_URL="https://github.com" + +# API interface URL. +GITHUB_API_URL="https://api.github.com" + does this mean we can support github enterprise installations?! (looks like barring the note about jira issues that point at github. sweet)
            busbey Sean Busbey added a comment -
            -## @returns ${JIRACLI} exit code
            +## @returns ${CURL} exit code
            

            so long as we're changing, replace with "exit code from posting to jira" so that we don't leak how we talk to jira?

            +  for bug in ${BUGSYSTEMS}; do
            +    if declare -f ${bug}_write_comment >/dev/null; then
            +       "${bug}_write_comment" "${commentfile}"
            +    fi
            +  done
            

            How will this play out for ASF projects that have github integration in place? I think we'll write to github and jira, and then github will write to jira. can we add a flag that picks one and defaults to "all"?

            busbey Sean Busbey added a comment - -## @returns ${JIRACLI} exit code +## @returns ${CURL} exit code so long as we're changing, replace with "exit code from posting to jira" so that we don't leak how we talk to jira? + for bug in ${BUGSYSTEMS}; do + if declare -f ${bug}_write_comment >/dev/null; then + "${bug}_write_comment" "${commentfile}" + fi + done How will this play out for ASF projects that have github integration in place? I think we'll write to github and jira, and then github will write to jira. can we add a flag that picks one and defaults to "all"?
            busbey Sean Busbey added a comment -
            +  while read -r line; do
            +    {
            +      printf "%s" $(echo "${line}" | cut -f1-2 -d:)
            +      echo "${comment}"
            +    } >> "${tmpfile}"
            

            As of shellcheck 3.3.7 that precommit flag still shows up:

            In dev-support/test-patch.d/whitespace.sh line 29:
                  printf "%s" $(echo "${line}" | cut -f1-2 -d:)
                              ^-- SC2046: Quote this to prevent word splitting.
            
            
            busbey Sean Busbey added a comment - + while read -r line; do + { + printf "%s" $(echo "${line}" | cut -f1-2 -d:) + echo "${comment}" + } >> "${tmpfile}" As of shellcheck 3.3.7 that precommit flag still shows up: In dev-support/test-patch.d/whitespace.sh line 29: printf "%s" $(echo "${line}" | cut -f1-2 -d:) ^-- SC2046: Quote this to prevent word splitting.

            Give a pointer to the variable or say what it is? It's GITHUB_ISSUE, and without other context this paragraph would have made me presume GITHUB_ISSUE_RE.

            Good catch, especially since GITHUB_ISSUE_RE doesn't exist anymore. (It did when I wrote that.) Specifically called out GITHUB_REPO which is the functional replacement.

            maybe different issue. can we just start at the https url?

            Yes, we should.

            also maybe we need some link about setting up https support for curl. and maybe a warning that you should trust your issue tracker since we use --location (though this is very minor, given our broader context).

            Well, that particular code is for smart-apply-patch and specifically hard-coded to the ASF jira. De-Apache-ing smart-apply-patch to point to another JIRA system and add GH support, etc, is a different project for sure. At some point, we need to tie smart-apply-patch into test-patch's plugin code. As long as we keep teasing code out of test-patch, this will get easier and easier to do.

            does this mean we can support github enterprise installations?! (looks like barring the note about jira issues that point at github. sweet)

            Someone needs to test it, but yes, I think I've gotten the necessary plumbing in place to support it.

            can we add a flag that picks one and defaults to "all"?

            Add --bugcomments. It takes a list but one always gets the console.

            aw Allen Wittenauer added a comment - Give a pointer to the variable or say what it is? It's GITHUB_ISSUE, and without other context this paragraph would have made me presume GITHUB_ISSUE_RE. Good catch, especially since GITHUB_ISSUE_RE doesn't exist anymore. (It did when I wrote that.) Specifically called out GITHUB_REPO which is the functional replacement. maybe different issue. can we just start at the https url? Yes, we should. also maybe we need some link about setting up https support for curl. and maybe a warning that you should trust your issue tracker since we use --location (though this is very minor, given our broader context). Well, that particular code is for smart-apply-patch and specifically hard-coded to the ASF jira. De-Apache-ing smart-apply-patch to point to another JIRA system and add GH support, etc, is a different project for sure. At some point, we need to tie smart-apply-patch into test-patch's plugin code. As long as we keep teasing code out of test-patch, this will get easier and easier to do. does this mean we can support github enterprise installations?! (looks like barring the note about jira issues that point at github. sweet) Someone needs to test it, but yes, I think I've gotten the necessary plumbing in place to support it. can we add a flag that picks one and defaults to "all"? Add --bugcomments. It takes a list but one always gets the console.

            -05:

            aw Allen Wittenauer added a comment - -05: address busbey 's comments
            hadoopqa Hadoop QA added a comment -

            A patch to the files used for the QA process has been detected.
            Re-executing against the patched versions to perform further tests.
            The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7483/console in case of problems.

            hadoopqa Hadoop QA added a comment - A patch to the files used for the QA process has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7483/console in case of problems.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 0s precommit patch detected.
            +1 site 0m 0s HADOOP-12111 passed
            0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched.
            -1 test4tests 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 site 0m 0s the patch passed
            +1 asflicense 0m 19s Patch does not generate ASF License warnings.
            +1 shellcheck 0m 12s There were no new shellcheck issues.
            +1 whitespace 0m 0s Patch has no whitespace issues.
            0m 37s



            Subsystem Report/Notes
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12750663/HADOOP-12129.HADOOP-12111.05.patch
            JIRA Issue HADOOP-12129
            git revision HADOOP-12111 / f64f68a
            Optional Tests asflicense site unit shellcheck
            uname Linux asf906.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
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh
            Default Java 1.7.0_55
            Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55
            shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.)
            JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7483/testReport/
            Max memory used 49MB
            Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7483/console

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s precommit patch detected. +1 site 0m 0s HADOOP-12111 passed 0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched. -1 test4tests 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 site 0m 0s the patch passed +1 asflicense 0m 19s Patch does not generate ASF License warnings. +1 shellcheck 0m 12s There were no new shellcheck issues. +1 whitespace 0m 0s Patch has no whitespace issues. 0m 37s Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12750663/HADOOP-12129.HADOOP-12111.05.patch JIRA Issue HADOOP-12129 git revision HADOOP-12111 / f64f68a Optional Tests asflicense site unit shellcheck uname Linux asf906.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 Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh Default Java 1.7.0_55 Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55 shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.) JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7483/testReport/ Max memory used 49MB Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7483/console This message was automatically generated.

            -06:

            • fix issue with --bugcomments default
            • add --linecomments to turn those on and off as well
            aw Allen Wittenauer added a comment - -06: fix issue with --bugcomments default add --linecomments to turn those on and off as well
            hadoopqa Hadoop QA added a comment -

            A patch to the files used for the QA process has been detected.
            Re-executing against the patched versions to perform further tests.
            The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7487/console in case of problems.

            hadoopqa Hadoop QA added a comment - A patch to the files used for the QA process has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/7487/console in case of problems.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 0s precommit patch detected.
            +1 site 0m 0s HADOOP-12111 passed
            0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched.
            -1 test4tests 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 site 0m 0s the patch passed
            +1 asflicense 0m 15s Patch does not generate ASF License warnings.
            +1 shellcheck 0m 12s There were no new shellcheck issues.
            +1 whitespace 0m 0s Patch has no whitespace issues.
            0m 33s



            Subsystem Report/Notes
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12750717/HADOOP-12129.HADOOP-12111.06.patch
            JIRA Issue HADOOP-12129
            git revision HADOOP-12111 / 565d9bf
            Optional Tests asflicense site unit shellcheck
            uname Linux asf906.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
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh
            Default Java 1.7.0_55
            Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55
            shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.)
            JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7487/testReport/
            Max memory used 49MB
            Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7487/console

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s precommit patch detected. +1 site 0m 0s HADOOP-12111 passed 0 @author 0m 0s Skipping @author checks as test-patch.sh has been patched. -1 test4tests 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 site 0m 0s the patch passed +1 asflicense 0m 15s Patch does not generate ASF License warnings. +1 shellcheck 0m 12s There were no new shellcheck issues. +1 whitespace 0m 0s Patch has no whitespace issues. 0m 33s Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12750717/HADOOP-12129.HADOOP-12111.06.patch JIRA Issue HADOOP-12129 git revision HADOOP-12111 / 565d9bf Optional Tests asflicense site unit shellcheck uname Linux asf906.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 Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/dev-support-test/personality/hadoop.sh Default Java 1.7.0_55 Multi-JDK versions /home/jenkins/tools/java/jdk1.8.0:1.8.0 /home/jenkins/tools/java/jdk1.7.0_55:1.7.0_55 shellcheck v0.3.3 (This is an old version that has serious bugs. Consider upgrading.) JDK v1.7.0_55 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7487/testReport/ Max memory used 49MB Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7487/console This message was automatically generated.
            busbey Sean Busbey added a comment -

            +1

            busbey Sean Busbey added a comment - +1

            Thanks for the review!

            Committed.

            I'll write a release note later today.

            aw Allen Wittenauer added a comment - Thanks for the review! Committed. I'll write a release note later today.

            People

              aw Allen Wittenauer
              aw Allen Wittenauer
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: