From f206a8732f9093f061862a5d89209f3852bb493b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 6 Aug 2025 19:43:52 +0000 Subject: [PATCH 1/4] fix(shell): Prevent 'argument list too long' error Set the IFS variable to a newline in base-template.sh. This prevents the 'argument list too long' error when hook scripts iterate over the STAGED_FILES environment variable with a large number of files. This change is made in the central `base-template.sh` script to avoid modifying numerous hook scripts, and to be robust for future hook scripts. --- base-template.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/base-template.sh b/base-template.sh index da0b84fc..8346105b 100755 --- a/base-template.sh +++ b/base-template.sh @@ -112,6 +112,9 @@ set_main_variables() { # Global IFS for loops IFS_NEWLINE=" " + # Set IFS to newline, to handle filenames with spaces correctly + # and to prevent argument list too long errors in for loops over STAGED_FILES. + IFS="$IFS_NEWLINE" # Fail if the shared root is not available (if enabled) FAIL_ON_NOT_EXISTING_SHARED_HOOK=$(git config --get githooks.failOnNonExistingSharedHooks) From e36d8eadd874f1005f2be584f15460fdaccedaf8 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 6 Aug 2025 20:04:36 +0000 Subject: [PATCH 2/4] fix(shell): Prevent 'argument list too long' error I've set the IFS variable to a newline in base-template.sh. This will prevent the 'argument list too long' error when hook scripts iterate over the STAGED_FILES environment variable with a large number of files. I made this change in the central `base-template.sh` script to avoid modifying numerous hook scripts, and to be robust for future hook scripts. I've also added a new test case (tests/step-101.sh) to verify the fix by committing a large number of files. --- tests/step-101.sh | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 tests/step-101.sh diff --git a/tests/step-101.sh b/tests/step-101.sh new file mode 100644 index 00000000..d15fcafd --- /dev/null +++ b/tests/step-101.sh @@ -0,0 +1,38 @@ +#!/bin/sh +# Test: +# Test that a large number of staged files does not cause an +# "argument list too long" error. + +# run the default install +sh /var/lib/githooks/install.sh --non-interactive || exit 1 + +mkdir -p /tmp/test101 && cd /tmp/test101 || exit 1 +git init || exit 1 + +# set up a pre-commit hook +mkdir -p .githooks/pre-commit +cp /var/lib/githooks/.githooks/pre-commit/list-staged-files .githooks/pre-commit/ + +# Create a large number of files +for i in $(seq 1 2000); do + touch "file_$i" +done + +# Stage the files +git add . + +# Commit the files +# This should not fail with "argument list too long" +OUTPUT=$(git commit -m "Test commit with a large number of files." 2>&1) +if echo "$OUTPUT" | grep -q "Argument list too long"; then + echo "ERROR: The commit failed with 'Argument list too long'" + exit 1 +fi + +# Check that the commit was successful +if ! git log -1 | grep -q "Test commit with a large number of files."; then + echo "ERROR: The commit was not successful" + exit 1 +fi + +exit 0 From 92346ebb3ee1816264abc847f91bf558039c7fde Mon Sep 17 00:00:00 2001 From: Viktor Adam Date: Thu, 7 Aug 2025 09:27:01 +1000 Subject: [PATCH 3/4] Introduce STAGED_FILES_REFERENCE when the list of changed files is too large :star: --- .githooks/pre-commit/list-staged-files | 5 ++ .githooks/pre-commit/no-set-x | 2 + .githooks/pre-commit/no-tabs | 2 + .githooks/pre-commit/no-todo-or-fixme | 2 + .githooks/pre-commit/shellcheck | 2 + .githooks/pre-commit/shellcheck-ignore-format | 2 + .githooks/pre-commit/shfmt | 2 + base-template.sh | 30 +++++++++-- tests/step-101.sh | 52 ++++++++++++++----- 9 files changed, 81 insertions(+), 18 deletions(-) diff --git a/.githooks/pre-commit/list-staged-files b/.githooks/pre-commit/list-staged-files index bcdade84..9c130c4f 100644 --- a/.githooks/pre-commit/list-staged-files +++ b/.githooks/pre-commit/list-staged-files @@ -1,4 +1,9 @@ #!/bin/sh + +if [ -n "${STAGED_FILES_REFERENCE}" ]; then + STAGED_FILES="$(cat "${STAGED_FILES_REFERENCE}")" +fi + # shellcheck disable=SC2153 if [ -n "${STAGED_FILES}" ]; then echo "* Staged files:" diff --git a/.githooks/pre-commit/no-set-x b/.githooks/pre-commit/no-set-x index 1481c385..1b3fb0fb 100644 --- a/.githooks/pre-commit/no-set-x +++ b/.githooks/pre-commit/no-set-x @@ -17,6 +17,8 @@ checkSetX() { if [ -n "$GITHOOKS_ON_DEMAND_EXEC" ]; then STAGED_FILES=$(find . -name '*.sh') +elif [ -n "${STAGED_FILES_REFERENCE}" ]; then + STAGED_FILES="$(cat "${STAGED_FILES_REFERENCE}")" fi for FILE in $STAGED_FILES; do diff --git a/.githooks/pre-commit/no-tabs b/.githooks/pre-commit/no-tabs index 3575d7b5..b9b84df8 100644 --- a/.githooks/pre-commit/no-tabs +++ b/.githooks/pre-commit/no-tabs @@ -17,6 +17,8 @@ checkTab() { if [ -n "$GITHOOKS_ON_DEMAND_EXEC" ]; then STAGED_FILES=$(find . -name '*.sh') +elif [ -n "${STAGED_FILES_REFERENCE}" ]; then + STAGED_FILES="$(cat "${STAGED_FILES_REFERENCE}")" fi for FILE in $STAGED_FILES; do diff --git a/.githooks/pre-commit/no-todo-or-fixme b/.githooks/pre-commit/no-todo-or-fixme index 45b1788e..64869195 100644 --- a/.githooks/pre-commit/no-todo-or-fixme +++ b/.githooks/pre-commit/no-todo-or-fixme @@ -18,6 +18,8 @@ checkTodo() { if [ -n "$GITHOOKS_ON_DEMAND_EXEC" ]; then STAGED_FILES=$(find . -type f) +elif [ -n "${STAGED_FILES_REFERENCE}" ]; then + STAGED_FILES="$(cat "${STAGED_FILES_REFERENCE}")" fi for FILE in $STAGED_FILES; do diff --git a/.githooks/pre-commit/shellcheck b/.githooks/pre-commit/shellcheck index a71060fb..55206a9d 100644 --- a/.githooks/pre-commit/shellcheck +++ b/.githooks/pre-commit/shellcheck @@ -12,6 +12,8 @@ fi SUCCESS=0 if [ -n "$GITHOOKS_ON_DEMAND_EXEC" ]; then STAGED_FILES=$(find . -name '*.sh') +elif [ -n "${STAGED_FILES_REFERENCE}" ]; then + STAGED_FILES="$(cat "${STAGED_FILES_REFERENCE}")" fi for FILE in $STAGED_FILES; do diff --git a/.githooks/pre-commit/shellcheck-ignore-format b/.githooks/pre-commit/shellcheck-ignore-format index f40720bd..c46eb454 100644 --- a/.githooks/pre-commit/shellcheck-ignore-format +++ b/.githooks/pre-commit/shellcheck-ignore-format @@ -7,6 +7,8 @@ assertStaged SUCCESS=0 if [ -n "$GITHOOKS_ON_DEMAND_EXEC" ]; then STAGED_FILES=$(find . -name '*.sh') +elif [ -n "${STAGED_FILES_REFERENCE}" ]; then + STAGED_FILES="$(cat "${STAGED_FILES_REFERENCE}")" fi for FILE in $STAGED_FILES; do diff --git a/.githooks/pre-commit/shfmt b/.githooks/pre-commit/shfmt index 60370ec1..1765aaa1 100644 --- a/.githooks/pre-commit/shfmt +++ b/.githooks/pre-commit/shfmt @@ -12,6 +12,8 @@ fi SUCCESS=0 if [ -n "$GITHOOKS_ON_DEMAND_EXEC" ]; then STAGED_FILES=$(find . -name '*.sh') +elif [ -n "${STAGED_FILES_REFERENCE}" ]; then + STAGED_FILES="$(cat "${STAGED_FILES_REFERENCE}")" fi for FILE in $STAGED_FILES; do diff --git a/base-template.sh b/base-template.sh index 8346105b..f802e35d 100755 --- a/base-template.sh +++ b/base-template.sh @@ -33,6 +33,7 @@ process_git_hook() { execute_old_hook_if_available "$@" || return 1 execute_all_shared_hooks "$@" || return 1 execute_all_hooks_in "$(pwd)/.githooks" "$@" || return 1 + cleanup_staged_files_reference_if_exists } ##################################################### @@ -112,9 +113,6 @@ set_main_variables() { # Global IFS for loops IFS_NEWLINE=" " - # Set IFS to newline, to handle filenames with spaces correctly - # and to prevent argument list too long errors in for loops over STAGED_FILES. - IFS="$IFS_NEWLINE" # Fail if the shared root is not available (if enabled) FAIL_ON_NOT_EXISTING_SHARED_HOOK=$(git config --get githooks.failOnNonExistingSharedHooks) @@ -170,7 +168,8 @@ register_repo() { # when available, so hooks can use it if # they want to. # -# Sets the ${STAGED_FILES} variable +# Sets the ${STAGED_FILES} or the +# ${STAGED_FILES_REFERENCE} variable. # # Returns: # None @@ -184,7 +183,28 @@ export_staged_files() { # shellcheck disable=SC2181 if [ $? -eq 0 ]; then - export STAGED_FILES="$CHANGED_FILES" + # if the changed files list is over 100k then write it into a temporary file instead + # to avoid "Argument list too long" errors + if [ "${#CHANGED_FILES}" -gt 100000 ]; then + STAGED_FILES_TMP=$(mktemp) + echo "$CHANGED_FILES" >"$STAGED_FILES_TMP" + export STAGED_FILES_REFERENCE="$STAGED_FILES_TMP" + else + export STAGED_FILES="$CHANGED_FILES" + fi + fi +} + +##################################################### +# Deletes the temporary file that references +# the staged files list when it was too big. +# +# Returns: +# None +##################################################### +cleanup_staged_files_reference_if_exists() { + if [ -n "$STAGED_FILES_REFERENCE" ] && [ -f "$STAGED_FILES_REFERENCE" ]; then + rm "$STAGED_FILES_REFERENCE" fi } diff --git a/tests/step-101.sh b/tests/step-101.sh index d15fcafd..4947b1c9 100644 --- a/tests/step-101.sh +++ b/tests/step-101.sh @@ -1,38 +1,64 @@ #!/bin/sh # Test: -# Test that a large number of staged files does not cause an -# "argument list too long" error. +# Test that a large number of staged files does not cause an "argument list too long" error. # run the default install sh /var/lib/githooks/install.sh --non-interactive || exit 1 -mkdir -p /tmp/test101 && cd /tmp/test101 || exit 1 -git init || exit 1 +mkdir -p /tmp/test101 && cd /tmp/test101 || exit 2 +git init || exit 3 # set up a pre-commit hook -mkdir -p .githooks/pre-commit -cp /var/lib/githooks/.githooks/pre-commit/list-staged-files .githooks/pre-commit/ +# shellcheck disable=SC2016 +mkdir -p .githooks/pre-commit && + echo 'echo "RefFile: $STAGED_FILES_REFERENCE" >> /tmp/test101.out; +if [ -n "$STAGED_FILES_REFERENCE" ]; then + export STAGED_FILES="$(cat $STAGED_FILES_REFERENCE)"; +fi; +echo "Hook executed for $STAGED_FILES" >> /tmp/test101.out +' >.githooks/pre-commit/test && + git hooks accept test || exit 4 # Create a large number of files -for i in $(seq 1 2000); do - touch "file_$i" +mkdir -p some/quite/long/directory/to/put/these/test/files/so/that/our/test/here/can/verify/lengths/better/and/again/some/quite/long/directory/to/put/these/test/files/so/that/our/test/here/can/verify/lengths/better/and/again/some/quite/long/directory/to/put/these/test/files/so/that/our/test/here/can/verify/lengths/better/and/again +for i in $(seq 1 5000); do + touch "some/quite/long/directory/to/put/these/test/files/so/that/our/test/here/can/verify/lengths/better/and/again/some/quite/long/directory/to/put/these/test/files/so/that/our/test/here/can/verify/lengths/better/and/again/some/quite/long/directory/to/put/these/test/files/so/that/our/test/here/can/verify/lengths/better/and/again/some_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_long_file_name_$i.txt" done # Stage the files -git add . +git add . || exit 5 # Commit the files # This should not fail with "argument list too long" OUTPUT=$(git commit -m "Test commit with a large number of files." 2>&1) if echo "$OUTPUT" | grep -q "Argument list too long"; then - echo "ERROR: The commit failed with 'Argument list too long'" - exit 1 + echo "! $OUTPUT" + echo "! The commit failed with 'Argument list too long'" + exit 6 fi # Check that the commit was successful if ! git log -1 | grep -q "Test commit with a large number of files."; then - echo "ERROR: The commit was not successful" - exit 1 + echo "! $OUTPUT" + echo "! The commit was not successful" + exit 7 +fi + +# Check that the hook was executed +if ! grep -q 'Hook executed for' /tmp/test101.out || ! grep -q '_long_file_name_1012.txt' /tmp/test101.out; then + echo "! $OUTPUT" + echo "! Could not verify the hook execution" + exit 8 +fi + +# Make sure the temporary staged files reference file was deleted +REF_FILE=$(grep 'RefFile:' /tmp/test101.out | awk '{print $2}') +if [ -z "$REF_FILE" ]; then + echo "! Staged files reference file not found in the output" + exit 9 +elif [ -f "$REF_FILE" ]; then + echo "! Staged files reference file was not cleaned up" + exit 9 fi exit 0 From c969f486b3b0b5a3ca1f0d24e2d2dfdf9ea3a5d1 Mon Sep 17 00:00:00 2001 From: Viktor Adam Date: Thu, 7 Aug 2025 09:33:32 +1000 Subject: [PATCH 4/4] ${STAGED_FILES_REFERENCE} updates in the README :sparkles: --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index 9090de0c..99388fd0 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,18 @@ done The `ACMR` filter in the `git diff` will include staged files that are added, copied, modified or renamed. +__Note__: if the list of changes is over 100k characters, then instead of `${STAGED_FILES}` you will get the `${STAGED_FILES_REFERENCE}` variable set instead which will point to a temporary file containing this list. This is to avoid `Argument list too long` errors when executing hooks and other parts of the framework. If you have a large enough repository where this is a concern, you should probably start your hook files by examining if this reference is set, like shown below. + +```shell +if [ -n "${STAGED_FILES_REFERENCE}" ]; then + STAGED_FILES="$(cat "${STAGED_FILES_REFERENCE}")" +fi + +for STAGED in ${STAGED_FILES}; do + ... +done +``` + ## Supported hooks The supported hooks are listed below. Refer to the [Git documentation](https://git-scm.com/docs/githooks) for information on what they do and what parameters they receive.