Pre-commit hook to prevent large file commits

Question

I've written the below bash script to run as a pre-commit hook. The intention is to check the git staging area for any files larger than 1mb, and prevent the commit if any are present.

#!/bin/sh
too_big() {
 bytez=$(cat "$(git rev-parse --show-toplevel)/1ドル" | wc -c)
 if [ "$bytez" -gt 1000000 ] ; then
 cat <<EOF
Error: Attempting to commit a file larger than approximately 1mb.
Commiting large files slows jenkins builds, clones, and other operations we'd rather not slow down.
Consider generating, downloading, zipping, etc these files.
Offending file - 1ドル
EOF
 exit 1
 fi
}
# If you want to allow large files to be committed set this variable to true.
allowbigfiles=$(git config --bool hooks.allowbigfiles)
# Redirect output to stderr.
exec 1>&2
if [ "$allowbigfiles" != "true" ]
then
 set -e
 git diff --name-only --cached 1ドル | while read x; do too_big $x; done
fi

Edit: The final script ended up as part of a library of client side Git Hooks

Answer 1

Although described as a Bash script, this appears to be a portable shell script that can be run by any POSIX-conformant shell. That's a good thing, as it means we can use a much smaller, leaner shell such as Dash.

If you haven't yet installed shellcheck, I recommend you do so (there's also a web version you can try). It highlights the following:

• Useless cat here:

  bytez=$(cat "$(git rev-parse --show-toplevel)/1ドル" | wc -c)
  

  That can be simplified to

  bytez=$(<"$(git rev-parse --show-toplevel)/1ドル" wc -c)
  

• Unquoted expansion of 1ドル - we really wanted to write "1ドル" there.

• Unsafe read x ought to be read -r x
• $x is unquoted

Piping the file into wc isn't an efficient way to measure size of a file; we could simply use stat:

bytez=$(stat -c %s "$(git rev-parse --show-toplevel)/1ドル")

And instead of running git rev-parse for every file in the changeset, run it once and remember the value in a variable.

(削除) The error message should go to the standard error stream (削除ここまで) (I see the whole script is redirected to &2)

It's not obvious why set -e is right down inside the if - I'd normally put that immediately after the shebang.

Consider also set -u to help detect a likely cause of errors.

Spelling: unless you really mean "1 millibit", that should be "1MB".

A suggestion that might fall into the "too cute" category: since git config --bool always produces true or false as output, we can simply execute that as a command:

if ! $(git config --bool hooks.allowbigfiles)
then

Line-based reading (i.e. git diff --name-only | while read) isn't totally robust; there's a -z option provided to produce NUL-separated output. This will require Bash, though, in order to read -d.

---

Improved code

#!/bin/bash
set -e
too_big() {
 if [ "$(stat -c %s "$toplevel/1ドル")" -gt 1000000 ] ; then
 cat <<EOF
Error: Attempting to commit a file larger than approximately 1MB.
Commiting large files slows jenkins builds, clones, and other
operations we would rather not slow down.
Consider generating, downloading, zipping, etc these files.
Offending file - 1ドル
EOF
 exit 1
 fi
}
# If you want to allow large files to be committed set this variable to true.
allowbigfiles=$(git config --bool hooks.allowbigfiles)
# Redirect output to stderr.
exec >&2
if ! "$allowbigfiles"
then
 toplevel=$(git rev-parse --show-toplevel)
 git diff --name-only -z --cached "1ドル" | 
 while IFS= read -d '' -r x; do too_big "$x"; done
fi

• chicks: Some people can't imagine cuteness in a shell script. I love putting commands in if statements. This is a fabulous twist.