Skip to content

Commit 3389473

Browse files
committed
default base set to apache/datafusion, fails test on git failures, non-number values rejected
1 parent c83f7cd commit 3389473

2 files changed

Lines changed: 39 additions & 10 deletions

File tree

‎ci/scripts/check_large_files.sh‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ usage() {
3838
cat >&2 <<USAGE
3939
Usage: $SCRIPT_NAME [--base <ref>] [--head <ref>] [--max-bytes <n>]
4040
Fails if any file committed between <base> and <head> is larger than <n> bytes.
41-
--base <ref> Start of the commit range, exclusive. Defaults to the merge base of <head> and origin/main.
41+
--base <ref> Start of the commit range, exclusive. Defaults to the merge base of <head> and main on the
42+
remote that points at apache/datafusion, or on origin when there is no such remote.
4243
--head <ref> End of the commit range, inclusive. Defaults to HEAD.
4344
--max-bytes <n> Size limit in bytes. Defaults to ${DEFAULT_MAX_FILE_SIZE_BYTES} (1.5 MB).
4445
USAGE
@@ -72,39 +73,66 @@ while [[ $# -gt 0 ]]; do
7273
shift
7374
done
7475

76+
# A leading zero is rejected too: bash would read the value as octal.
77+
if ! [[ "${MAX_FILE_SIZE_BYTES}" =~ ^(0|[1-9][0-9]*)$ ]]; then
78+
echo "[${SCRIPT_NAME}] --max-bytes must be a whole number of bytes, got \"${MAX_FILE_SIZE_BYTES}\"" >&2
79+
exit 1
80+
fi
81+
7582
cd "${ROOT_DIR}"
7683

84+
# The remote that points at apache/datafusion, or origin when there is no such remote.
85+
upstream_remote() {
86+
local remote
87+
for remote in $(git remote); do
88+
if [[ "$(git remote get-url "${remote}")" =~ github\.com[:/]apache/datafusion(\.git)?/?$ ]]; then
89+
echo "${remote}"
90+
return
91+
fi
92+
done
93+
echo "origin"
94+
}
95+
7796
if ! git rev-parse --verify --quiet "${HEAD_REF}^{commit}" > /dev/null; then
7897
echo "[${SCRIPT_NAME}] ${HEAD_REF} is not a commit" >&2
7998
exit 1
8099
fi
81100

82101
if [[ -z "${BASE_REF}" ]]; then
83-
if ! git rev-parse --verify --quiet "origin/main^{commit}" > /dev/null; then
84-
echo "[${SCRIPT_NAME}] origin/main is not available; pass --base <ref> to choose the start of the range" >&2
102+
upstream_main="$(upstream_remote)/main"
103+
if ! git rev-parse --verify --quiet "${upstream_main}^{commit}" > /dev/null; then
104+
echo "[${SCRIPT_NAME}] ${upstream_main} is not available; fetch it or pass --base <ref> to choose the start of the range" >&2
85105
exit 1
86106
fi
87-
BASE_REF="$(git merge-base "${HEAD_REF}" origin/main)"
107+
BASE_REF="$(git merge-base "${HEAD_REF}" "${upstream_main}")"
88108
elif ! git rev-parse --verify --quiet "${BASE_REF}^{commit}" > /dev/null; then
89109
echo "[${SCRIPT_NAME}] ${BASE_REF} is not a commit" >&2
90110
exit 1
91111
fi
92112

93113
echo "[${SCRIPT_NAME}] Checking files committed in ${BASE_REF}..${HEAD_REF} against the ${MAX_FILE_SIZE_BYTES} byte limit"
94114

115+
# Listing the objects into a variable, rather than piping them into the loop, makes a failing
116+
# git command fail the check instead of leaving the loop with nothing to read.
117+
objects="$(
118+
git rev-list --objects "${BASE_REF}..${HEAD_REF}" \
119+
| git cat-file --batch-check='%(objectname) %(objecttype) %(objectsize) %(rest)'
120+
)"
121+
95122
exit_code=0
96123
# Only blobs are files. Commits and trees are listed too, and are skipped.
97124
while read -r id type size path; do
125+
if [[ "${type}" == "missing" ]]; then
126+
echo "[${SCRIPT_NAME}] Object ${id} is not in this clone, so its size cannot be checked. Fetch the full history and retry." >&2
127+
exit 1
128+
fi
98129
if [[ "${type}" == "blob" && "${size}" -gt "${MAX_FILE_SIZE_BYTES}" ]]; then
99130
exit_code=1
100131
echo "Object ${id} [${path}] has size ${size}, exceeding ${MAX_FILE_SIZE_BYTES} limit." >&2
101132
if [[ -n "${GITHUB_ACTIONS:-}" ]]; then
102133
echo "::error file=${path}::File ${path} has size ${size}, exceeding ${MAX_FILE_SIZE_BYTES} limit."
103134
fi
104135
fi
105-
done < <(
106-
git rev-list --objects "${BASE_REF}..${HEAD_REF}" \
107-
| git cat-file --batch-check='%(objectname) %(objecttype) %(objectsize) %(rest)'
108-
)
136+
done <<< "${objects}"
109137

110138
exit "${exit_code}"

‎docs/source/contributor-guide/testing.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,9 @@ The audit fetches the RustSec advisory database. A new advisory or a different
260260
`ci/scripts/check_large_files.sh` fails if any file committed between a base
261261
ref and a head ref is larger than 1.5 MB, the same check the "Large files PR
262262
check" workflow runs on pull requests. `./dev/rust_lint.sh` runs it against
263-
the merge base of `HEAD` and `origin/main`. To run the check on its own, or
264-
against a different range:
263+
the merge base of `HEAD` and `main` on the remote that points at
264+
`apache/datafusion`, or on `origin` when there is no such remote. To run the
265+
check on its own, or against a different range:
265266

266267
```shell
267268
./ci/scripts/check_large_files.sh

0 commit comments

Comments
 (0)