wombatu-kun opened a new pull request, #19267:
URL: https://github.com/apache/hudi/pull/19267
### Describe the issue this Pull Request addresses
Closes #19266
`scripts/release/validate_source_binary_files.sh` guards the Apache source
release against binary files. It has never fired on Linux, which is every CI
runner and most release managers' machines, for two independent reasons.
**The flag is BSD-only.** The check pipes every file through `file -I`,
which is the macOS spelling of the MIME option. GNU `file` rejects it with
`file: invalid option -- 'I'`, prints usage to stderr and nothing to stdout, so
`wc -l` counts zero and `numBinaryFiles` is always `0`. `set -o errexit` does
not catch this: the pipeline sits inside a command substitution in an
assignment, and without `pipefail` the assignment takes the exit status of the
last stage (`sed`), which is 0. Running the current script unmodified against a
clean checkout of master, whose tree holds 105 images outside `src/test/`,
exits 0 and prints `No Binary Files in the source files? - [OK]`.
**The check runs in the wrong directory.** Its contract is "run me from the
root of a source release tree" - that is how `validate_staged_release.sh` calls
it, after `cd`-ing into the extracted tarball. The `validate-source` job in
`bot.yml` runs it against the raw git checkout, which still contains `rfc/`,
`docker/images/`, `hudi-notebooks/notebooks/common/images/` and `.idea/`, all
of which `create_source_directory.sh` deliberately excludes from the release.
So repairing the flag alone would turn CI red on 105 files that are not in the
source release at all.
Found while reviewing #19265, which adds the first binary asset that reaches
the source tarball outside `release/` and drew no red check.
### Summary and Changelog
The guard now actually guards, and it can no longer fail silently.
- `scripts/release/validate_source_binary_files.sh`: ask libmagic for the
**charset** instead of the MIME type. `file --mime` is the long option for both
GNU's `-i` and BSD's `-I`, so it is the one spelling that works on Linux CI and
on a release manager's macOS, and its `charset=binary` field answers the
question directly. The previous approach - an allowlist of permitted MIME types
(`text/`, `application/json`, `application/xml`) - is brittle against
libmagic's growing type vocabulary: on `file-5.45` it produces five false
positives on master alone, because libmagic now labels the JSON test data
`application/x-ndjson` and misdetects `SparkIndexerSupport.java` as
`application/javascript`. Empty files (`inode/x-empty`, which libmagic also
reports as `charset=binary`) are exempted, and the existing `release/` and
`src/test/` exemptions are kept.
- Same file: add `set -o pipefail`, so a broken `file` invocation fails the
job loudly instead of yielding an empty pipeline that reads as "no binary files
found". This is the regression guard for the bug itself.
- Same file: run `file` once over a batched `find -print0 | xargs -0`
instead of spawning one `file` process per path with `xargs -I {}`, and build
the report from the single pass rather than re-running the whole pipeline to
print it. This also removes an inconsistency between the two runs, which
excluded `release/` and `release/release_guide` respectively.
- `.github/workflows/bot.yml`: create the source release directory once,
then run **both** the binary check and the copyright check inside it, mirroring
what `validate_staged_release.sh` already does with an extracted tarball. The
copyright check already ran there; only its `create_source_directory.sh` call
moves out into its own step.
Verified locally against a clean checkout of master:
| Scenario | Before | After |
| --- | --- | --- |
| Fixed script, source release directory | n/a | exit 0, `[OK]` |
| Fixed script, raw git checkout | n/a | exit 1, 105 files (100 under
`rfc/`, 3 under `hudi-notebooks/`, 1 each under `.idea/` and `docker/`) |
| Fixed script, a PNG planted in a source path | n/a | exit 1, path named |
| Fixed script with a deliberately broken `file` flag | n/a | exit 123 |
| **Current script, tree containing 105 binaries** | **exit 0, `[OK]`** |
n/a |
### Impact
CI and release tooling only. No runtime, public API, config, storage format
or user-facing change.
One cross-PR interaction is worth calling out: once this merges, the guard
catches `hudi-agent-gateway/src/hudi_agent_gateway/ui/hudi-logo.png` from
#19265 (`image/png; charset=binary`), because `create_source_directory.sh` does
not exclude that path. That is the guard doing its job, and it is independent
of merge order - whichever lands second sees the red check. #19265 can clear it
by inlining the logo as SVG or as a data URI in `style.css`, which also removes
the need for the `.png` exemption that PR adds to
`validate_source_copyright.sh`.
### Risk Level
low
The change is confined to a release-validation script and the
`validate-source` CI job. The new check passes on current master inside the
source release directory with zero files flagged, and `pipefail` makes any
future breakage loud rather than silent. The one behaviour that gets stricter
is the intended one.
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
