fix(ci): let validate-source's RAT check fail on unlicensed files outside Maven modules#19275
fix(ci): let validate-source's RAT check fail on unlicensed files outside Maven modules#19275wombatu-kun wants to merge 2 commits into
Conversation
…side Maven modules Inside a Maven module RAT works: it is declared in ~55 module poms and bound to the compile phase, so an ordinary build job fails on a missing ASF header. Outside a module nothing checks. Those runs are each scoped to their own module's basedir, and the root aggregator does not declare the plugin, so the only run that scans the repository root is the direct `mvn apache-rat:check` goal in validate_source_rat.sh. That script always exits 0: (bash -c "mvn apache-rat:check") || (echo "Rat Check Failed. [ERROR]" && exit 1) echo "RAT Check Passed [OK]" The `exit 1` runs in a subshell, so it exits the subshell rather than the script, and with no set -e control falls through to the success message. On master the job prints BUILD FAILURE, then "Too many files with unapproved license: 19", then "Rat Check Failed. [ERROR]", then "RAT Check Passed [OK]", and stays green. Propagate the failure with an if-block instead of the subshell, and point the error at target/rat.txt rather than at a --verbose flag this script does not accept. 17 of those 19 violations are phantom. The job stages a copy of the tree into hudi-tmp-repo before the RAT step, and the root-anchored exclude `hudi-trino-plugin/**` does not cover `hudi-tmp-repo/hudi-trino-plugin/**`, so RAT scans the copy. Match at any depth instead. The intent of the exclusion, that hudi-trino-plugin's licensing is handled by airlift, is unchanged. The remaining 2 are real: rfc/rfc-99/variant-appendix.md and rfc/rfc-99/vector-appendix.md are the only 2 of the 54 rfc markdown files with no ASF header. Add it, copied verbatim from the sibling rfc-99.md. Closes apache#19274
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the PR! This tightens the RAT license check so it can actually fail CI for files outside Maven modules, fixes the phantom-violation count from the scratch-directory copy, and adds the two missing ASF headers under rfc/rfc-99/. The core fix looks sound; one inline note on the scope of the pom.xml exclude change. A Hudi committer or PMC member can take it from here.
| <exclude>hudi-trino-plugin/**</exclude> | ||
| <!-- Match at any depth: the source-release checks copy the tree into a scratch | ||
| directory, and a root-anchored pattern would not exclude the copy. --> | ||
| <exclude>**/hudi-trino-plugin/**</exclude> |
There was a problem hiding this comment.
🤖 Broadening hudi-trino-plugin/** to **/hudi-trino-plugin/** fixes the scratch-copy case, but it also permanently excludes a hudi-trino-plugin directory at any depth from RAT going forward. Since the real problem is the copied tree, would excluding the scratch directory directly (e.g. hudi-tmp-repo/**) be a more targeted fix that keeps RAT coverage tighter for the real source tree? Curious whether you considered that alternative.
There was a problem hiding this comment.
Done 847b1e8. The scratch copy is now staged in ${{ runner.temp }} instead of inside the workspace, so RAT no longer scans it, and the pom exclude is reverted to hudi-trino-plugin/**.
…AT scans `mvn apache-rat:check` scans the filesystem from the repository root, so the full copy of the tree that the validate-source job stages into `hudi-tmp-repo` is scanned as well, and every root-anchored exclude in the root pom silently stops applying to it. That is where the 17 phantom violations came from: all of them sit under `hudi-tmp-repo/hudi-trino-plugin`. Broadening the exclude to `**/hudi-trino-plugin/**` treated the symptom and loosened the pattern for good. Stage the copy in the runner temp directory instead, so RAT sees only the real tree: no exclude has to change and license coverage stays exactly as it is on master. The release path already behaves this way, since validate_source_rat.sh runs inside the extracted source tarball, which contains no nested copy. The pom exclude is reverted to `hudi-trino-plugin/**`.
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the RFC! This change adds the standard ASF license headers to the rfc-99 variant and vector appendix files as part of a CI RAT-check fix; there's no design content being modified here, so nothing stands out at the design level. A Hudi committer or PMC member can take it from here for a final look.
cc @yihua
3 participants
Describe the issue this Pull Request addresses
Closes #19274
Inside a Maven module, RAT works. It is declared in the
<build><plugins>of ~55 module poms and bound to thecompilephase, so an ordinary build job does fail on a missing ASF header (verified: a header-less.javaunderhudi-io/src/main/java/makesmvn -pl hudi-io compileexit 1).Outside a module there is no working gate. Each of those RAT runs is scoped to its own module's
basedir, and the root aggregator does not declare the plugin in<build><plugins>, so the only run that ever scans the repository root is the directmvn apache-rat:checkgoal invocation - and that lives in exactly one place,scripts/release/validate_source_rat.sh, which always exits 0 no matter what RAT reports (verified: a header-less file at the root is missed bymvn -pl hudi-io compileand caught bymvn apache-rat:check). Sorfc/,scripts/,docker/, the root-level files, and any new non-Maven top-level directory are effectively unchecked.The failure does not propagate. The script is
(mvn apache-rat:check) || (echo "Rat Check Failed. [ERROR]" && exit 1)followed by the success message. Theexit 1runs in a( ... )subshell, so it exits the subshell rather than the script, and with noset -econtrol falls straight through toRAT Check Passed [OK]and a 0 exit status. On current master thevalidate-sourcejob printsBUILD FAILURE, thenToo many files with unapproved license: 19, thenRat Check Failed. [ERROR], thenRAT Check Passed [OK], and the job is green.17 of those 19 violations are phantom, because the job asks RAT to scan a copy of the tree.
apache-rat:checkis a filesystem scan rooted at the repository root, and the precedingCheck Copyrightstep runscreate_source_directory.sh hudi-tmp-repo, which rsyncs the whole tree intohudi-tmp-repo/inside the workspace. RAT therefore audits the real tree and the copy. Nothing in the root pom's exclude list applies to the copy the way it does to the original: every root-anchored exclude (hudi-trino-plugin/**among them) silently stops matching once the same directory sits one level deeper. All 17 phantom entries arehudi-tmp-repo/hudi-trino-plugin/...test resources. On a plain checkout the count is exactly 2.The remaining 2 are real.
rfc/rfc-99/variant-appendix.mdandrfc/rfc-99/vector-appendix.mdcarry no ASF header. They are the only 2 of the 54rfc/**/*.mdfiles without one, and being outside any Maven module, nothing has ever flagged them.Found while fixing #19266, which is the same class of silently-inert guard in the sibling binary-file check in the same job.
Summary and Changelog
The license gate for everything outside a Maven module can now actually fail CI, and it no longer reports violations that do not exist.
scripts/release/validate_source_rat.sh: replace the subshell failure branch withif ! mvn apache-rat:check ...; then ... exit 1; fi, so a RAT failure fails the script. The error message now points attarget/rat.txt, which lists the offending files, instead of suggesting a--verboseflag this script does not accept..github/workflows/bot.yml: stage the source-directory copy in${{ runner.temp }}instead of inside the workspace, so theRAT checkstep scans the repository and not a duplicate of it. The release path already works this way:validate_staged_release.shuntars the source release,cds into it and runs the same RAT script there, with no nested copy in sight. No RAT exclude needs to change, and license coverage of the real tree is exactly what it is on master.rfc/rfc-99/variant-appendix.md,rfc/rfc-99/vector-appendix.md: add the ASF header, copied verbatim from the siblingrfc/rfc-99/rfc-99.md.Verified locally:
RAT Check Passed [OK]RAT Check Passed [OK]RAT Check Passed [OK]Rat Check Failed. [ERROR]mvn compile(exit 1)The copyright check itself is unaffected by the move: it is invoked from inside the copy, which still contains
scripts/, and it passes on a pristine export of this branch.Impact
CI and release tooling only. No runtime, public API, config, storage format or user-facing change.
The
validate-sourcejob becomes able to go red on a missing ASF header outside a Maven module, which is its purpose. It passes on this branch because the only two genuine violations in the tree are fixed here.Risk Level
low
Confined to one release-validation script, the location of a scratch copy in one CI job, and two RFC markdown files. The root pom is untouched, so RAT's exclude list and the compile-phase RAT executions in the module poms behave exactly as they do on master.
mvn apache-rat:checkreportsUnapproved: 0on this branch, and theCheck Copyrightstep still runs against the same pruned copy of the tree, only from${{ runner.temp }}rather than from a subdirectory of the workspace.Documentation Update
none
Contributor's checklist