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