Details
-
Type:
New Feature
-
Status: Resolved
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 3.0.0-alpha1
-
Fix Version/s: 2.8.0, 3.0.0-alpha1
-
Component/s: scripts
-
Labels:None
-
Hadoop Flags:Incompatible change
-
Release Note:
Description
Now that Yetus has had a release, we should rip out the components that make it up from dev-support and replace them with wrappers. The wrappers should:
- default to a sane version
- allow for version overrides via an env var
- download into patchprocess
- execute with the given parameters
Marking this as an incompatible change, since we should also remove the filename extensions and move these into a bin directory for better maintenance towards the future.
Issue Links
- is blocked by
-
MAPREDUCE-6536
hadoop-pipes doesn't use maven properties for openssl
-
- Patch Available
-
- relates to
-
HADOOP-12192
update releasedocmaker commands
-
- Resolved
-
-
HADOOP-12798
Update changelog and release notes (2016-03-04)
-
- Resolved
-
-
HADOOP-11792
Remove all of the CHANGES.txt files
-
- Resolved
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
(argh. BUILDING.TXT triggers the world. sigh)
-01:
- fix the shell check errors
 -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 0s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| -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 | mvninstall | 8m 51s | trunk passed |
| +1 | compile | 10m 26s | trunk passed with JDK v1.8.0_66 |
| +1 | compile | 10m 25s | trunk passed with JDK v1.7.0_91 |
| +1 | mvnsite | 11m 28s | trunk passed |
| +1 | mvneclipse | 0m 46s | trunk passed |
| +1 | javadoc | 7m 10s | trunk passed with JDK v1.8.0_66 |
| +1 | javadoc | 11m 13s | trunk passed with JDK v1.7.0_91 |
| +1 | mvninstall | 9m 6s | the patch passed |
| +1 | compile | 10m 28s | the patch passed with JDK v1.8.0_66 |
| +1 | javac | 10m 28s | the patch passed |
| +1 | compile | 10m 12s | the patch passed with JDK v1.7.0_91 |
| +1 | javac | 10m 12s | the patch passed |
| +1 | mvnsite | 10m 58s | the patch passed |
| +1 | mvneclipse | 0m 43s | the patch passed |
| +1 | pylint | 0m 3s | There were no new pylint issues. |
| -1 | shellcheck | 0m 12s | The applied patch generated 4 new shellcheck issues (total was 139, now 100). |
| -1 | whitespace | 0m 0s | The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. |
| +1 | xml | 0m 0s | The patch has no ill-formed XML file. |
| +1 | javadoc | 6m 46s | the patch passed with JDK v1.8.0_66 |
| +1 | javadoc | 10m 50s | the patch passed with JDK v1.7.0_91 |
| -1 | unit | 14m 4s | root in the patch failed with JDK v1.8.0_66. |
| -1 | unit | 93m 20s | root in the patch failed with JDK v1.7.0_91. |
| +1 | asflicense | 0m 32s | Patch does not generate ASF License warnings. |
| 228m 51s |
| Reason | Tests |
|---|---|
| JDK v1.8.0_66 Failed junit tests | hadoop.metrics2.impl.TestGangliaMetrics |
| JDK v1.7.0_91 Failed junit tests | hadoop.hdfs.server.datanode.TestBlockScanner |
| hadoop.hdfs.server.namenode.TestFileTruncate | |
| hadoop.hdfs.TestFileAppend2 | |
| hadoop.hdfs.TestDFSStripedOutputStreamWithFailure030 |
This message was automatically generated.
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 0s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| -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 | mvninstall | 7m 28s | trunk passed |
| +1 | compile | 7m 34s | trunk passed with JDK v1.8.0_66 |
| +1 | compile | 8m 28s | trunk passed with JDK v1.7.0_91 |
| +1 | mvnsite | 9m 20s | trunk passed |
| +1 | mvneclipse | 0m 38s | trunk passed |
| +1 | javadoc | 6m 3s | trunk passed with JDK v1.8.0_66 |
| +1 | javadoc | 9m 3s | trunk passed with JDK v1.7.0_91 |
| +1 | mvninstall | 7m 20s | the patch passed |
| +1 | compile | 7m 33s | the patch passed with JDK v1.8.0_66 |
| +1 | javac | 7m 33s | the patch passed |
| +1 | compile | 8m 26s | the patch passed with JDK v1.7.0_91 |
| +1 | javac | 8m 26s | the patch passed |
| +1 | mvnsite | 9m 15s | the patch passed |
| +1 | mvneclipse | 0m 38s | the patch passed |
| +1 | pylint | 0m 2s | There were no new pylint issues. |
| +1 | shellcheck | 0m 11s | There were no new shellcheck issues. |
| -1 | whitespace | 0m 0s | The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. |
| +1 | xml | 0m 1s | The patch has no ill-formed XML file. |
| +1 | javadoc | 5m 10s | the patch passed with JDK v1.8.0_66 |
| +1 | javadoc | 9m 5s | the patch passed with JDK v1.7.0_91 |
| -1 | unit | 81m 48s | root in the patch failed with JDK v1.8.0_66. |
| -1 | unit | 87m 18s | root in the patch failed with JDK v1.7.0_91. |
| +1 | asflicense | 0m 24s | Patch does not generate ASF License warnings. |
| 266m 55s |
| Reason | Tests |
|---|---|
| JDK v1.8.0_66 Failed junit tests | hadoop.hdfs.server.namenode.TestNNThroughputBenchmark |
| hadoop.hdfs.server.namenode.TestNameNodeMetricsLogger | |
| hadoop.hdfs.TestWriteReadStripedFile | |
| JDK v1.7.0_91 Failed junit tests | hadoop.hdfs.server.datanode.TestBlockReplacement |
This message was automatically generated.
-02:
- white space issues fixed
- add ability to use YETUS_HOME/bin/x
- add some short cutting when already downloaded
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 0s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| -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 | mvninstall | 7m 30s | trunk passed |
| +1 | compile | 7m 32s | trunk passed with JDK v1.8.0_66 |
| +1 | compile | 8m 26s | trunk passed with JDK v1.7.0_91 |
| +1 | mvnsite | 9m 12s | trunk passed |
| +1 | mvneclipse | 0m 48s | trunk passed |
| -1 | javadoc | 3m 34s | root in trunk failed with JDK v1.8.0_66. |
| +1 | javadoc | 8m 59s | trunk passed with JDK v1.7.0_91 |
| +1 | mvninstall | 7m 22s | the patch passed |
| +1 | compile | 7m 32s | the patch passed with JDK v1.8.0_66 |
| +1 | javac | 7m 32s | the patch passed |
| +1 | compile | 8m 25s | the patch passed with JDK v1.7.0_91 |
| +1 | javac | 8m 25s | the patch passed |
| +1 | mvnsite | 9m 6s | the patch passed |
| +1 | mvneclipse | 0m 38s | the patch passed |
| +1 | pylint | 0m 2s | There were no new pylint issues. |
| +1 | shellcheck | 0m 11s | There were no new shellcheck issues. |
| +1 | whitespace | 0m 0s | Patch has no whitespace issues. |
| +1 | xml | 0m 1s | The patch has no ill-formed XML file. |
| -1 | javadoc | 3m 35s | root in the patch failed with JDK v1.8.0_66. |
| +1 | javadoc | 8m 56s | the patch passed with JDK v1.7.0_91 |
| -1 | unit | 25m 41s | root in the patch failed with JDK v1.8.0_66. |
| -1 | unit | 79m 45s | root in the patch failed with JDK v1.7.0_91. |
| +1 | asflicense | 0m 23s | Patch does not generate ASF License warnings. |
| 198m 47s |
| Reason | Tests |
|---|---|
| JDK v1.8.0_66 Timed out junit tests | org.apache.hadoop.http.TestHttpServerLifecycle |
| JDK v1.7.0_91 Failed junit tests | hadoop.hdfs.server.datanode.TestBlockReplacement |
| hadoop.hdfs.server.namenode.TestFsck |
This message was automatically generated.
Since this is targeting trunk, it probably isn't necessary to redirect the old scripts to invoke the new ones, even to remain compatible with existing tooling/automation. A README under dev-support would be helpful, though. To upgrade to new versions of Yetus, one bumps the version in yetus-wrapper?
Random:
- Downloading YETUS_KEYS and the .asc could probably be put within the check for $GPGBIN. The keys file may change between versions, but wouldn't be refetched, which could cause validation to fail (for obvious and easy to fix reasons, but still).
- If the tarball exists but the result of extracting it doesn't, should this attempt to re-download the tarball (and .asc), assuming the last one was corrupt? The -f check that avoids the download seems more likely to wedge in this case than avoid skippable work.
- Is the "z" option nonstandard for tar?
Thanks for the review and feedback!
Since this is targeting trunk, it probably isn't necessary to redirect the old scripts to invoke the new ones, even to remain compatible with existing tooling/automation.
Redirecting would be extremely tricky anyway: smart-apply-patch, for example, uses wholly incompatible command line options. So by not doing the redirection, we'll force folks to recognize that the world has changed, even the 90% of folks that don't read common-dev.
A README under dev-support would be helpful, though.
That's a good idea, especially given the above. Will add.
To upgrade to new versions of Yetus, one bumps the version in yetus-wrapper?
Yup. I've been debating adding the ability to fetch from the source tree. This week's thought process has mainly been: no, bad idea, because if the source tree changes form (highly likely) then this would need to get touched again. Plus there's already the ability to point to a built version somewhere else, so if someone really wants to use master, they can always go that route.
Downloading YETUS_KEYS and the .asc could probably be put within the check for $GPGBIN.
Good idea.
If the tarball exists but the result of extracting it doesn't, should this attempt to re-download the tarball (and .asc), assuming the last one was corrupt? The -f check that avoids the download seems more likely to wedge in this case than avoid skippable work.
Hmm. I see it the potential issue. Let me think about how to fix this. I definitely need to add more error checking there as well as rm the tarball if extract fails.
Is the "z" option nonstandard for tar?
Yes, it's nonstandard. It's a GNU extension.
-03:
- address feedback
- increase error checking to the extreme
In README.md:
- ASL header?
- s/wrappers scripts/wrapper scripts/
- Unescaped underscore in HADOOP_PATCHPROCESS
- Link to Yetus?
It'd be nice if the README or each wrapper included a brief description. Usage would come from Yetus, but someone poking around might appreciate a one-liner that doesn't require them to invoke anything to understand what's in dev-support.
Otherwise +1, pending jenkins. Speaking of jenkins, it won't be a problem for trunk/future branches and branch-2 to invoke different dev-support scripts? I assume it's diverged already, but thought I'd ask.
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 0s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| -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. |
| 0 | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 | mvninstall | 8m 1s | trunk passed |
| +1 | compile | 6m 55s | trunk passed with JDK v1.8.0_66 |
| +1 | compile | 8m 1s | trunk passed with JDK v1.7.0_91 |
| +1 | mvnsite | 10m 20s | trunk passed |
| +1 | mvneclipse | 0m 40s | trunk passed |
| +1 | javadoc | 6m 8s | trunk passed with JDK v1.8.0_66 |
| +1 | javadoc | 10m 0s | trunk passed with JDK v1.7.0_91 |
| 0 | mvndep | 0m 23s | Maven dependency ordering for patch |
| +1 | mvninstall | 8m 8s | the patch passed |
| +1 | compile | 9m 11s | the patch passed with JDK v1.8.0_66 |
| +1 | javac | 9m 11s | the patch passed |
| +1 | compile | 9m 2s | the patch passed with JDK v1.7.0_91 |
| +1 | javac | 9m 2s | the patch passed |
| +1 | mvnsite | 11m 28s | the patch passed |
| +1 | mvneclipse | 0m 45s | the patch passed |
| +1 | pylint | 0m 3s | The applied patch generated 0 new + 0 unchanged - 449 fixed = 0 total (was 449) |
| +1 | shellcheck | 0m 12s | The applied patch generated 0 new + 98 unchanged - 43 fixed = 98 total (was 141) |
| +1 | whitespace | 0m 0s | Patch has no whitespace issues. |
| +1 | xml | 0m 1s | The patch has no ill-formed XML file. |
| +1 | javadoc | 7m 25s | the patch passed with JDK v1.8.0_66 |
| +1 | javadoc | 11m 6s | the patch passed with JDK v1.7.0_91 |
| -1 | unit | 14m 17s | root in the patch failed with JDK v1.8.0_66. |
| -1 | unit | 14m 25s | root in the patch failed with JDK v1.7.0_91. |
| -1 | asflicense | 0m 24s | Patch generated 1 ASF License warnings. |
| 138m 32s |
| Reason | Tests |
|---|---|
| JDK v1.8.0_66 Failed junit tests | hadoop.security.ssl.TestReloadingX509TrustManager |
| JDK v1.7.0_91 Failed junit tests | hadoop.ipc.TestIPC |
This message was automatically generated.
(various readme)
All reasonable. Also: Doh! on the license header. Clearly I didn't test my own patch before submitting (only manually ran shellcheck haha).
Speaking of jenkins, it won't be a problem for trunk/future branches and branch-2 to invoke different dev-support scripts?
Hadoop precommit, at least for the time being, is pulling directly from yetus master via git at runtime. The bundled test-patch hasn't been in use for months.
-04:
- a better readme
Thanks!
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | reexec | 0m 0s | Docker mode activated. |
| +1 | @author | 0m 0s | The patch does not contain any @author tags. |
| -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. |
| 0 | mvndep | 1m 13s | Maven dependency ordering for branch |
| +1 | mvninstall | 7m 43s | trunk passed |
| +1 | compile | 6m 44s | trunk passed with JDK v1.8.0_66 |
| +1 | compile | 7m 25s | trunk passed with JDK v1.7.0_91 |
| +1 | mvnsite | 9m 51s | trunk passed |
| +1 | mvneclipse | 0m 48s | trunk passed |
| +1 | javadoc | 6m 6s | trunk passed with JDK v1.8.0_66 |
| +1 | javadoc | 10m 8s | trunk passed with JDK v1.7.0_91 |
| 0 | mvndep | 0m 22s | Maven dependency ordering for patch |
| +1 | mvninstall | 8m 22s | the patch passed |
| +1 | compile | 7m 11s | the patch passed with JDK v1.8.0_66 |
| +1 | javac | 7m 11s | the patch passed |
| +1 | compile | 7m 26s | the patch passed with JDK v1.7.0_91 |
| +1 | javac | 7m 26s | the patch passed |
| +1 | mvnsite | 9m 43s | the patch passed |
| +1 | mvneclipse | 0m 40s | the patch passed |
| +1 | pylint | 0m 4s | The applied patch generated 0 new + 0 unchanged - 449 fixed = 0 total (was 449) |
| +1 | shellcheck | 0m 12s | The applied patch generated 0 new + 98 unchanged - 43 fixed = 98 total (was 141) |
| +1 | whitespace | 0m 0s | Patch has no whitespace issues. |
| +1 | xml | 0m 1s | The patch has no ill-formed XML file. |
| +1 | javadoc | 5m 38s | the patch passed with JDK v1.8.0_66 |
| +1 | javadoc | 9m 35s | the patch passed with JDK v1.7.0_91 |
| -1 | unit | 11m 52s | root in the patch failed with JDK v1.8.0_66. |
| -1 | unit | 12m 19s | root in the patch failed with JDK v1.7.0_91. |
| +1 | asflicense | 0m 21s | Patch does not generate ASF License warnings. |
| 124m 52s |
| Reason | Tests |
|---|---|
| JDK v1.8.0_66 Failed junit tests | hadoop.metrics2.impl.TestGangliaMetrics |
| JDK v1.7.0_91 Failed junit tests | hadoop.metrics2.impl.TestGangliaMetrics |
| hadoop.test.TestTimedOutTestsListener |
| Subsystem | Report/Notes |
|---|---|
| Docker | Image:yetus/hadoop:0ca8df7 |
| JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12783405/HADOOP-12651.04.patch |
| JIRA Issue | |
| Optional Tests | asflicense mvnsite shellcheck pylint unit compile javac javadoc mvninstall xml |
| uname | Linux b0d34cd1719f 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 | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 7905788 |
| Default Java | 1.7.0_91 |
| Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 |
| shellcheck | v0.4.3 |
| pylint | v1.1.0 |
| unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/8444/artifact/patchprocess/patch-unit-root-jdk1.8.0_66.txt |
| unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/8444/artifact/patchprocess/patch-unit-root-jdk1.7.0_91.txt |
| unit test logs | https://builds.apache.org/job/PreCommit-HADOOP-Build/8444/artifact/patchprocess/patch-unit-root-jdk1.8.0_66.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8444/artifact/patchprocess/patch-unit-root-jdk1.7.0_91.txt |
| JDK v1.7.0_91 Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/8444/testReport/ |
| modules | C: hadoop-common-project/hadoop-common . U: . |
| Max memory used | 120MB |
| Powered by | Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org |
| Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/8444/console |
This message was automatically generated.
Assuming the +1 is still good, I'll go ahead and commit, update the wiki pages, and drop a note to common-dev so people aren't taken by surprise.
Thanks for the review!
FAILURE: Integrated in Hadoop-trunk-Commit #9156 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9156/)
HADOOP-12651. Replace dev-support with wrappers to Yetus (aw) (aw: rev 8cecad2d564d89d0f56e98de80a6f9b0bcb39d7a)
- dev-support/smart-apply-patch.sh
- hadoop-common-project/hadoop-common/pom.xml
- BUILDING.txt
- dev-support/test-patch.d/shellcheck.sh
- dev-support/test-patch.sh
- dev-support/test-patch.d/checkstyle.sh
- dev-support/test-patch.d/whitespace.sh
- dev-support/shelldocs.py
- dev-support/releasedocmaker.py
HADOOP-12651. Replace dev-support with wrappers to Yetus (aw) (aw: rev 2a867355dfce880bb82257508862ec26bc7f16b9) - dev-support/README.md
- dev-support/bin/test-patch
- dev-support/bin/releasedocmaker
- dev-support/bin/shelldocs
- dev-support/bin/yetus-wrapper
- dev-support/bin/smart-apply-patch
Anyone objections to cherry-picking this to branch-2? IIUC from the above discussion, this is marked as incompatible because of the changes to smart-apply-patch, but it sounds like branch-2 precommit is already using Yetus already.
Motivation is to pull in the new releasedocmaker, so we can finally nuke CHANGES.txt (see HADOOP-11792).
Thanks, Andrew Wang. I'm +1 for backporting to branch-2. I understand the motivation for flagging this as backwards-incompatible, but as a practical matter I think the probability of actually impacting anyone is miniscule. The benefits of eliminating CHANGES.txt outweigh the risks.
Thanks Chris. I just pulled the two relevant commits back to branch-2 and branch-2.8. Tested by running "mvn site -Preleasedocs" and spot checking the generated html files.
One conflict was that I did not include the new "shelldocs" profile, which only applies to the rewritten shell scripts in trunk.
Thanks of course also to Allen for the original patch!
I think branch-2 lacks the copies the previous versions. Then there is HADOOP-12798 which probably needs a refresh now.
Thanks for the pointer Allen, let's continue the discussion over on HADOOP-12798.
-00:
This forces a few parameters to smart-apply-patch and test-patch to be mostly equivalent to the previous versions. (they can be overridden on the command line, however)