akashjainn opened a new pull request, #25440:
URL: https://github.com/apache/datafusion/pull/25440

   ## Which issue does this PR close?
   
   - Part of #21048. The issue stays open for the other checklist items.
   
   ## Rationale for this change
   
   The "Large files PR check" workflow rejects pull requests that commit a file 
larger than 1.5 MB, but its logic lives inline in the workflow YAML, so a 
contributor cannot run it before pushing and only learns about an oversized 
file from CI. This moves the check into a script that CI and the local lint 
suite share, so the "Large file check" item in #21048 is covered the same way 
#25359 covered the security audit.
   
   ## What changes are included in this PR?
   
   - `ci/scripts/check_large_files.sh` is a new executable script. It lists the 
objects committed between a base ref and a head ref and fails if any blob is 
larger than the limit. `--base` defaults to the merge base of the head and 
`origin/main`, `--head` defaults to `HEAD`, and `--max-bytes` defaults to the 
1.5 MB the workflow used. Both refs are validated before the range is listed. 
The `::error` annotation is only printed when `GITHUB_ACTIONS` is set, so local 
runs get the plain message on stderr.
   - `.github/workflows/large_files.yml` keeps the same job, step name, 
triggers, permissions, runner and checkout, and its run step now passes the PR 
or merge-queue SHAs it already computed to the script. The size limit and its 
comment move from the step's `env:` block into the script.
   - `dev/rust_lint.sh` runs the script as a read-only step after the ASF 
status check validation. The `--write` and `--allow-dirty` flags never reach it.
   - `docs/source/contributor-guide/testing.md` documents the standalone 
command and the range flags.
   
   One difference from the inline version: the script sizes objects with a 
single `git cat-file --batch-check` instead of one `git cat-file -s` per 
object, and skips commits and trees by object type rather than by an empty 
path. On a 2000 commit range with 29,214 objects, the inline loop took 11 min 
20 s locally and the script took 0.8 s, with identical output.
   
   ## What is the testing strategy for this PR?
   
   - The old inline loop and the new script were run on the same four ranges of 
this repository's history, including the commit that added the 3.7 MB 
`docs/logos/Datafusion_Branding_Guideline.pdf`. Exit codes and stderr were 
identical for all four: three passing ranges and the PDF commit failing with 
the same message.
   - In a throwaway worktree, a 2 MB file was committed on a branch off 
`origin/main` and the script was run with no arguments. It picked the merge 
base as the default and failed on the new file. `--max-bytes 20000` on a small 
range fails on the expected files, and an unknown `--base` or `--head` exits 1 
with a message naming the ref.
   - With `GITHUB_ACTIONS=true` the `::error file=` line is printed; without 
it, nothing is printed to stdout.
   - The script runs under bash 3.2 (macOS `/bin/bash`) and bash 5, and passes 
`shellcheck`. `typos`, `prettier`, `check_asf_yaml_status_checks.py` and 
`check_no_cargo_install_in_workflows.sh` pass on the changed files. The 
workflow file parses and still has the single `check-files` job with the `Check 
size of new Git objects` step.
   
   ## Are there any user-facing changes?
   
   No. Contributors get a new local check via `./dev/rust_lint.sh` or 
`./ci/scripts/check_large_files.sh`.
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to