voonhous commented on code in PR #19267:
URL: https://github.com/apache/hudi/pull/19267#discussion_r3585826509
##########
scripts/release/validate_source_binary_files.sh:
##########
@@ -19,16 +19,38 @@
# under the License.
#
-# fail immediately
+# Run this from the root of a source release tree, i.e. the output of
+# create_source_directory.sh (see the validate-source job in bot.yml) or an
+# extracted source tarball (see validate_staged_release.sh). Paths that never
+# reach the source release are excluded by create_source_directory.sh, not
here.
+
+# fail immediately; pipefail so that a broken `file` invocation can never again
+# yield an empty pipeline that reads as "no binary files found"
set -o errexit
set -o nounset
+set -o pipefail
echo "Checking for binary files in the source files"
-numBinaryFiles=`find . -iname '*' | xargs -I {} file -I {} | grep -va
directory | grep -v "release/" | grep -v "/src/test/" | grep -va
'application/json' | grep -va 'text/' | grep -va 'application/xml' | grep -va
'application/json' | wc -l | sed -e s'/ //g'`
-if [ "$numBinaryFiles" -gt "0" ]; then
+# `--mime` is the long option for both GNU file's `-i` and BSD file's `-I`, so
it
+# is the only spelling that works on Linux CI and on a release manager's macOS.
+# Its charset field answers the question directly: libmagic reports
+# charset=binary for anything that is not text. Matching on the charset rather
+# than on a list of permitted MIME types keeps this robust against libmagic's
+# growing type vocabulary, which already labels the JSON test data
+# application/x-ndjson and misdetects at least one .java source as
+# application/javascript.
+mimeTypes=$(find . -type f -print0 | xargs -0 file --mime)
+
+# An empty file reports as inode/x-empty; charset=binary, and is neither
binary nor
+# a licensing concern. release/ holds the release guide's screenshot;
src/test/ holds
+# the binary fixtures (parquet, avro, hfile) that tests read.
+binaryFiles=$(echo "$mimeTypes" | grep -a 'charset=binary' | grep -v
'inode/x-empty' \
Review Comment:
nit: the `-v` stages lack `-a`, so a non-UTF-8 filename can flip a later
grep into binary mode. `-a` on all stages (or `LC_ALL=C` up top) keeps the
chain byte-safe.
##########
scripts/release/validate_source_binary_files.sh:
##########
@@ -19,16 +19,38 @@
# under the License.
#
-# fail immediately
+# Run this from the root of a source release tree, i.e. the output of
+# create_source_directory.sh (see the validate-source job in bot.yml) or an
+# extracted source tarball (see validate_staged_release.sh). Paths that never
+# reach the source release are excluded by create_source_directory.sh, not
here.
+
+# fail immediately; pipefail so that a broken `file` invocation can never again
+# yield an empty pipeline that reads as "no binary files found"
set -o errexit
set -o nounset
+set -o pipefail
echo "Checking for binary files in the source files"
-numBinaryFiles=`find . -iname '*' | xargs -I {} file -I {} | grep -va
directory | grep -v "release/" | grep -v "/src/test/" | grep -va
'application/json' | grep -va 'text/' | grep -va 'application/xml' | grep -va
'application/json' | wc -l | sed -e s'/ //g'`
-if [ "$numBinaryFiles" -gt "0" ]; then
+# `--mime` is the long option for both GNU file's `-i` and BSD file's `-I`, so
it
+# is the only spelling that works on Linux CI and on a release manager's macOS.
+# Its charset field answers the question directly: libmagic reports
+# charset=binary for anything that is not text. Matching on the charset rather
+# than on a list of permitted MIME types keeps this robust against libmagic's
+# growing type vocabulary, which already labels the JSON test data
+# application/x-ndjson and misdetects at least one .java source as
+# application/javascript.
+mimeTypes=$(find . -type f -print0 | xargs -0 file --mime)
+
+# An empty file reports as inode/x-empty; charset=binary, and is neither
binary nor
+# a licensing concern. release/ holds the release guide's screenshot;
src/test/ holds
+# the binary fixtures (parquet, avro, hfile) that tests read.
+binaryFiles=$(echo "$mimeTypes" | grep -a 'charset=binary' | grep -v
'inode/x-empty' \
+ | grep -v 'release/' | grep -v '/src/test/' || true)
Review Comment:
nit: substring match - this also exempts any nested `*/release/` path (or a
hypothetical `prerelease/`). `grep -v '^\./release/'` pins it to the
release-guide dir it is meant for.
--
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]
