[
https://issues.apache.org/jira/browse/HADOOP-20017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18126521#comment-18126521
]
ASF GitHub Bot commented on HADOOP-20017:
-----------------------------------------
joseluisll opened a new pull request, #8818:
URL: https://github.com/apache/hadoop/pull/8818
### Description of PR
JIRA: [HADOOP-20017](https://issues.apache.org/jira/browse/HADOOP-20017)
The nightly qbt builds (JDK 17 and JDK 21) report 19 SpotBugs warnings in
`hadoop-tools/hadoop-distcp`, which make the spotbugs vote -1 for
`hadoop-distcp`, `hadoop-tools` and root, and turn the precommit of other open
PRs red. This PR fixes 10 of them in code and adds narrow exclusions for the
other 9. Each exclusion matches class, method and bug pattern, and the
`EI_EXPOSE_REP2` ones also match the field. The module had no SpotBugs exclude
file before, so this adds
`hadoop-tools/hadoop-distcp/dev-support/findbugs-exclude.xml` and points
`spotbugs-maven-plugin` at it in the module pom. I did not add the module to
`dev-support/findbugs-exclude-global.xml`, because that file turns these
patterns off for the whole module.
| # | Class | Warning | Resolution |
|---|---|---|---|
| 1 | `SimpleCopyListing` | SS_SHOULD_BE_STATIC `maxRetries` | **Code**:
`private static final int MAX_RETRIES = 3` |
| 2 | `DiffInfo` | CT_CONSTRUCTOR_THROW `<init>` | **Code**: package-private
class made `final` |
| 3 | `DynamicInputChunk` | CT_CONSTRUCTOR_THROW `<init>` | **Code**:
package-private class made `final` |
| 4 | `DynamicInputChunkContext` | CT_CONSTRUCTOR_THROW `<init>` | **Code**:
package-private class made `final` |
| 5 | `CopyListingFileStatus` | EI_EXPOSE_REP2 `setAclEntries` | **Code**:
copies the list, keeping null as null |
| 6 | `CopyListingFileStatus` | EI_EXPOSE_REP2 `setXAttrs` | **Code**:
copies into a `LinkedHashMap`, keeping null as null and the iteration order |
| 7 | `DistCpContext` | EI_EXPOSE_REP2 `setSourcePaths` | **Code**: copies
the list, keeping null as null |
| 8 | `DistCpContext` | EI_EXPOSE_REP `getSourcePaths` | **Code**: returns
an unmodifiable view, or null |
| 9 | `DistCpOptions$Builder` | EI_EXPOSE_REP2 `<init>` | **Code**: copies
the source path list. `DistCpOptions` is documented as immutable, and
`getSourcePaths()` already returns an unmodifiable copy |
| 10 | `DistCpOptionSwitch` | EI_EXPOSE_REP `getOption` | **Code**: returns
a clone of the enum's shared, JVM-wide CLI `Option`. The only caller is
`OptionsParser`, which just adds it to `Options` |
| 11 | `CopyListingFileStatus` | EI_EXPOSE_REP `getPermission` |
**Excluded**: the `FsPermission` is shared on purpose. One instance is created
per copied file, so copying would add an allocation per file and per access |
| 12 | `CopyListingFileStatus` | EI_EXPOSE_REP2 `<init>` (field
`permission`) | **Excluded**: same reason as #11 |
| 13 | `DistCpContext` | EI_EXPOSE_REP2 `<init>` (field `options`) |
**Excluded**: `DistCpOptions` is immutable once built |
| 14 | `DistCp` | CT_CONSTRUCTOR_THROW `<init>` | **Excluded**: the class is
`@InterfaceAudience.Public` and non-final, so making it final would break
subclasses |
| 15 | `ThrottledInputStream` | CT_CONSTRUCTOR_THROW `<init>` |
**Excluded**: public, non-final class; the constructor only `assert`s that the
bandwidth is positive |
| 16 | `CopyCommitter` | EI_EXPOSE_REP2 `<init>` (field
`taskAttemptContext`) | **Excluded**: the MapReduce framework passes this
context in and the committer has to keep the same instance |
| 17 | `DynamicRecordReader` | EI_EXPOSE_REP2 `initialize` (field
`taskAttemptContext`) | **Excluded**: same reason as #16 |
| 18 | `WorkReport` | EI_EXPOSE_REP2 `<init>` (field `exception`) |
**Excluded**: the report carries the original exception back to the caller on
purpose |
| 19 | `WorkReport` | EI_EXPOSE_REP `getException` | **Excluded**: same
reason as #18 |
No public method signature changes. There are two small behaviour changes
for callers that kept and later changed a collection they passed in. Changes
made to the list after `new DistCpOptions.Builder(list, target)` are no longer
seen, which matches the documented immutability of `DistCpOptions`. Changing
the list returned by `DistCpContext.getSourcePaths()` now throws
`UnsupportedOperationException`; when the paths came from `DistCpOptions` it
already did. Nothing in the repository depends on either behaviour.
**Expected Yetus result:** the spotbugs vote on the trunk (base) column will
still be `-1` ("hadoop-distcp in trunk has 21 extant spotbugs warnings"),
because Yetus scores the base branch before applying the patch. Those 21 are
the warnings fixed here. The patch column reports `0 new + 0 unchanged - 21
fixed = 0 total`.
### How was this patch tested?
- SpotBugs on `hadoop-distcp` (`mvn -pl hadoop-tools/hadoop-distcp
spotbugs:spotbugs` / `spotbugs:check`):
- before: **21** BugInstances. These are the 19 warnings above;
`DynamicInputChunk` and `ThrottledInputStream` each have a second constructor
that is reported separately.
- after: **0** (`BugInstance size is 0`, `spotbugs:check` passes).
- With an empty filter file, the build reports exactly the 10 BugInstances
that the 9 entries in the exclude file are meant to cover, so the exclusions
don't hide anything else.
- Checkstyle (`checkstyle:checkstyle`): no new violations. The module total
is unchanged, and the changed test files have no issues.
- New unit tests cover the behaviour changes. They all fail against the
pre-fix code and pass with it:
- `TestCopyListingFileStatus#testAclEntriesAndXAttrsAreCopied`
- `TestDistCpOptions#testBuilderCopiesSourcePaths`
- `TestDistCpOptions#testContextSourcePaths`
- `TestDistCpOptions#testOptionSwitchReturnsCopyOfOption`
- The full `hadoop-distcp` unit test suite passes locally (Maven 3.9.15, JDK
21), with 0 failures and 0 errors. The 2 skips are the large-file tests in
`TestHDFSContractDistCp`, which are skipped by default.
- Yetus precommit (JDK 17 and 21) on this change gives `+1` for test4tests,
compile, javac, checkstyle, javadoc, mvnsite, shadedclient, asflicense and the
hadoop-distcp unit tests; the only `-1` is the base-branch spotbugs vote
described above.
### For code changes:
- [x] Does the title of this PR start with the corresponding JIRA issue id
(e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: Have the integration tests been executed and the
endpoint
declared according to the connector-specific documentation? *Not
applicable: no object store connector changes.*
- [x] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)? *No new
dependencies.*
- [x] If applicable, have you updated the `LICENSE`, `LICENSE-binary`,
`NOTICE-binary` files? *Not applicable.*
### AI Tooling
If an AI tool was used:
- [x] The PR includes the phrase "Contains content generated by Claude Code"
where <tool> is the name of the AI tool used.
- [X ] My use of AI contributions follows the ASF legal policy
https://www.apache.org/legal/generative-tooling.html
Contains content generated by Claude Code.
> Fix the trunk SpotBugs warnings in hadoop-distcp
> ------------------------------------------------
>
> Key: HADOOP-20017
> URL: https://issues.apache.org/jira/browse/HADOOP-20017
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: Jose Luis López
> Assignee: Jose Luis López
> Priority: Minor
>
> The nightly trunk build (qbt) reports 19 spotbugs warning(s) in
> {{hadoop-tools/hadoop-distcp}}:
> * {{org.apache.hadoop.tools.CopyListingFileStatus}}: EI_EXPOSE_REP in
> CopyListingFileStatus.getPermission() line 217 (malicious_code)
> * {{org.apache.hadoop.tools.CopyListingFileStatus}}: EI_EXPOSE_REP2 in
> CopyListingFileStatus.<init>() line 152 (malicious_code)
> * {{org.apache.hadoop.tools.CopyListingFileStatus}}: EI_EXPOSE_REP2 in
> CopyListingFileStatus.setAclEntries() line 240 (malicious_code)
> * {{org.apache.hadoop.tools.CopyListingFileStatus}}: EI_EXPOSE_REP2 in
> CopyListingFileStatus.setXAttrs() line 258 (malicious_code)
> * {{org.apache.hadoop.tools.DiffInfo}}: CT_CONSTRUCTOR_THROW in
> DiffInfo.<init>() line 60 (experimental)
> * {{org.apache.hadoop.tools.DistCp}}: CT_CONSTRUCTOR_THROW in DistCp.<init>()
> line 117 (experimental)
> * {{org.apache.hadoop.tools.DistCpContext}}: EI_EXPOSE_REP in
> DistCpContext.getSourcePaths() line 63 (malicious_code)
> * {{org.apache.hadoop.tools.DistCpContext}}: EI_EXPOSE_REP2 in
> DistCpContext.<init>() line 50 (malicious_code)
> * {{org.apache.hadoop.tools.DistCpContext}}: EI_EXPOSE_REP2 in
> DistCpContext.setSourcePaths() line 55 (malicious_code)
> * {{org.apache.hadoop.tools.DistCpOptionSwitch}}: EI_EXPOSE_REP in
> DistCpOptionSwitch.getOption() line 281 (malicious_code)
> * {{org.apache.hadoop.tools.DistCpOptions$Builder}}: EI_EXPOSE_REP2 in
> DistCpOptions$Builder.<init>() line 540 (malicious_code)
> * {{org.apache.hadoop.tools.SimpleCopyListing}}: SS_SHOULD_BE_STATIC in
> SimpleCopyListing.maxRetries line 80 (performance)
> * {{org.apache.hadoop.tools.mapred.CopyCommitter}}: EI_EXPOSE_REP2 in
> CopyCommitter.<init>() line 95 (malicious_code)
> * {{org.apache.hadoop.tools.mapred.lib.DynamicInputChunk}}:
> CT_CONSTRUCTOR_THROW in DynamicInputChunk.<init>() line 56 (experimental)
> * {{org.apache.hadoop.tools.mapred.lib.DynamicInputChunkContext}}:
> CT_CONSTRUCTOR_THROW in DynamicInputChunkContext.<init>() line 47
> (experimental)
> * {{org.apache.hadoop.tools.mapred.lib.DynamicRecordReader}}: EI_EXPOSE_REP2
> in DynamicRecordReader.initialize() line 74 (malicious_code)
> * {{org.apache.hadoop.tools.util.ThrottledInputStream}}: CT_CONSTRUCTOR_THROW
> in ThrottledInputStream.<init>() line 48 (experimental)
> * {{org.apache.hadoop.tools.util.WorkReport}}: EI_EXPOSE_REP in
> WorkReport.getException() line 76 (malicious_code)
> * {{org.apache.hadoop.tools.util.WorkReport}}: EI_EXPOSE_REP2 in
> WorkReport.<init>() line 51 (malicious_code)
> They turn the spotbugs vote of these module runs -1: hadoop-tools,
> hadoop-tools/hadoop-distcp, root.
> Seen in:
> * JDK 17: 6 of the last 6 builds, latest
> [#189|https://ci-hadoop.apache.org/job/hadoop-qbt-trunk-java17-linux-x86_64/189/]
> (2026-10-11, trunk 16aa23404b)
> * JDK 21: 7 of the last 7 builds, latest
> [#118|https://ci-hadoop.apache.org/job/hadoop-qbt-trunk-java21-linux-x86_64/118/]
> (2026-10-11, trunk 16aa23404b)
> It turns the precommit of these open pull requests red:
> * [PR #8811|https://github.com/apache/hadoop/pull/8811] - clears the spotbugs
> -1 on hadoop-tools/hadoop-distcp
> * [PR #8749|https://github.com/apache/hadoop/pull/8749] - 19 of the 77
> warnings of the spotbugs -1 on root
> * [PR #8725|https://github.com/apache/hadoop/pull/8725] - 19 of the 77
> warnings of the spotbugs -1 on root
> * [PR #8704|https://github.com/apache/hadoop/pull/8704] - 19 of the 77
> warnings of the spotbugs -1 on root
> * [PR #8703|https://github.com/apache/hadoop/pull/8703] - 19 of the 77
> warnings of the spotbugs -1 on root
> * [PR #8699|https://github.com/apache/hadoop/pull/8699] - 19 of the 77
> warnings of the spotbugs -1 on root
> No open pull request or JIRA issue that fixes it was found.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
