Skip to content

Conversation

@myskov
Copy link
Contributor

@myskov myskov commented Mar 9, 2024

To improve Apache Rat configuration and Ozone maven config, I have done:

  • Added Rat plugin to the root module (ozone-main) to cover all files in a repo.
  • Removed Rat plugin declarations from other modules to make maven and rat configs more readable.
  • Moved exclusions to a separate file to make the Maven config more maintainable.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10498

How was this patch tested?

  1. I did a manual test by adding a "file.txt" without a license inside to hadoop-hdds-client project and running apache-rat:check goal. The output of the build:
[INFO] --- apache-rat:0.16.1:check (default-cli) @ hdds-client ---
[INFO] Rat check: Summary over all files. Unapproved: 1, unknown: 1, generated: 0, approved: 68 licenses.
[WARNING] Files with unapproved licenses:
  /Users/maximmyskov/Projects/github/myskov-ozone/hadoop-hdds/client/file.txt
  1. As the CI script is also modified, I committed the same "file.txt" to the branch and got the following output from RAT workflow:
Run hadoop-ozone/dev-support/checks/_summary.sh target/rat/summary.txt
[10](https://github.com/myskov/ozone/actions/runs/8205987100/job/22444088879#step:7:11)
hadoop-hdds/client/target/rat.txt: !????? /home/runner/work/ozone/ozone/hadoop-hdds/client/file.txt
[11](https://github.com/myskov/ozone/actions/runs/8205987100/job/22444088879#step:7:12)
Error: Process completed with exit code 1.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @myskov for working on this.

Comment on lines 18 to 55
.github/*
CONTRIBUTING.md
README.md
SECURITY.md
.gitattributes
src/main/resources/proto.lock
**/output.xml
**/log.html
**/report.html
**/.ssh/id_rsa*
src/test/resources/*.log
src/test/resources/ssl/*
**/dependency-reduced-pom.xml
**/pnpm-lock.yaml
src/test/resources/prometheus-test-response.txt
src/main/license/**
src/main/resources/proto.lock
src/test/resources/test.db.ini
tools/fault-injection-service/README.md
**/webapps/static/angular-nvd3-1.0.9.min.js
**/webapps/static/angular-route-1.8.0.min.js
**/webapps/static/bootstrap-3.4.1/**
**/webapps/static/d3-3.5.17.min.js
**/webapps/static/jquery-3.5.1.min.js
**/webapps/static/nvd3-1.8.5.min.css.map
**/webapps/static/nvd3-1.8.5.min.css
**/webapps/static/nvd3-1.8.5.min.js.map
**/webapps/static/nvd3-1.8.5.min.js
**/webapps/static/angular-1.8.0.min.js
src/test/resources/additionalfields.container
src/test/resources/incorrect.checksum.container
src/test/resources/incorrect.container
src/test/resources/test.db.ini
src/test/resources/123-dn-container.db/**
src/test/resources/123.container
static/slides/*
**/themes/ozonedoc/**
**/*.json No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to sort these.

To make the script useful for local run by devs, we should also exclude:

dev-support/ci/bats-assert/**
dev-support/ci/bats-support/**
.dev-tools/**

(Would be useful if we could exclude everything in .gitignore by default...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be useful if we could exclude everything in .gitignore by default...

As it's in the root folder now, it behaves exactly as you suggested.( parseSCMIgnoresAsExcludes is enabled by default https://creadur.apache.org/rat/apache-rat-plugin/check-mojo.html)

Copy link
Contributor

@adoroszlai adoroszlai Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @myskov for the info and updating the patch.

It seems Rat 0.13 does not use .gitignore correctly, the check fails due to the items I suggested (when not included in rat-exclusions.txt). Upgrading to latest Rat 0.16.1 solves false positive for those.

The only item it still reports is .hugo_build.lock. The problem seems to be that it looks for .gitignore only in the module directory tree:

[INFO] --- apache-rat-plugin:0.16.1:check (default-cli) @ hdds-docs ---
...
[DEBUG] Recursively loading .gitignore files in hadoop-hdds/docs

So I suggest:

  1. upgrade to 0.16.1 (adoroszlai@301b56b)
  2. split some items from .gitignore to module-level files (adoroszlai@82f0a4b)

BTW, steps that create the files reported by Rat as false positives:

  • build docs: mvn -pl :hdds-docs clean package (for .hugo_build.lock)
  • run bats check: hadoop-ozone/dev-support/checks/bats.sh (for dev-support/ci/bats-*)

@myskov myskov requested a review from adoroszlai March 12, 2024 06:06
@adoroszlai adoroszlai merged commit 71e4ff3 into apache:master Mar 14, 2024
@adoroszlai
Copy link
Contributor

Thanks @myskov for the patch. I'll post a follow-up PR with the suggested changes.

@myskov
Copy link
Contributor Author

myskov commented Mar 14, 2024

Thank you @adoroszlai

@myskov myskov deleted the HDDS-10498_improve_rat_config branch July 6, 2024 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants