Re: [PATCH v5] add: support pre-add hook

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mar 5, 2026, 14:37 UTC
Message-ID: <98531f78-cf04-4e64-ac7c-6a13e52aee54@gmail.com>
In-Reply-To: <pull.2045.v5.git.1772714253412.gitgitgadget@gmail.com>

On 05/03/2026 12:37, Chandra Kethi-Reddy via GitGitGadget wrote:

Show 11 quoted lines

> 
> diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc
> index 6192daeb03..a3ff4ced83 100644
> --- a/Documentation/git-add.adoc
> +++ b/Documentation/git-add.adoc
> [...]   
> @@ -42,10 +42,11 @@ use the `--force` option to add ignored files. If you specify the exact
>   filename of an ignored file, `git add` will fail with a list of ignored
>   files. Otherwise it will silently ignore the file.
>   
> +A `pre-add` hook can be used to reject `git add` (see linkgit:githooks[5]).

git-commit.adoc has a separate section for HOOKS, perhaps we should do the same here. It would be clearer to say the that the proposed changes are rejected rather than `git add` itself.

Show 14 quoted lines

> diff --git a/Documentation/githooks.adoc b/Documentation/githooks.adoc
> index 056553788d..90945a590e 100644
> --- a/Documentation/githooks.adoc
> +++ b/Documentation/githooks.adoc
> @@ -94,6 +94,36 @@ and is invoked after the patch is applied and a commit is made.
>   This hook is meant primarily for notification, and cannot affect
>   the outcome of `git am`.
>   
> +pre-add
> +~~~~~~~
> +
> +This hook is invoked by linkgit:git-add[1], and can be bypassed with the
> +`--no-verify` option. It is not invoked for `--interactive`, `--patch`,
> +`--edit`, or `--dry-run`.

I'm struggling to see how it is helpful to the user for "git add --dry-run $path" to succeed when "git add $path" will be rejected by the "pre-add" hook.

The other options all use "git apply" to apply a diff to the index so they could apply the patch to a temporary index which is then passed to the "pre-add" hook. If the hook fails the user should be given the option to re-edit the patch or re-select the hunks so that their work is not wasted.

To me this hook would be much more useful if it also checked changes staged by "git commit" - it is still staging changes after all.

> +It takes two arguments: the path to the index file for this invocation
> +of `git add`, and the path to the lockfile containing the proposed

Calling it a lockfile is rather confusing - it is just second index file that contains the changes that would be staged.

Show 10 quoted lines

> +index after staging. If no index exists yet, the first argument names
> +a path that does not exist and should be treated as an empty index.
> +
> +The hook is invoked after the index has been updated in memory and
> +written to the lockfile, but before it is committed to the final index
> +path. Exiting with a non-zero status causes `git add` to reject the
> +proposed state, roll back the lockfile, and leave the index unchanged.
> +Exiting with zero status allows the index update to be committed. The
> +hook accepts or rejects the entire proposed update; per-path filtering
> +is not supported. Both files should be treated as read-only by the hook.

If we don't enforce them being read-only people will write hooks that update them just as they do for "pre-commit" hooks. Once they start relying on that they will complain if we stop supporting it. If we lock both index files before running the hook I think that will prevent the hook from being able to update them.

> +Hook authors may set `GIT_INDEX_FILE="$1"` to inspect the current index
> +state and `GIT_INDEX_FILE="$2"` to inspect the proposed index state.

We should be explicit that the proposed index state contains all the changes that would be committed so staging changes incrementally will check them multiple times.

> +This hook can be used to prevent staging of files based on names, content,
> +or sizes (e.g., to block `.env` files, secret keys, or large files).
> +
> +This hook is not invoked by `git commit -a` or `git commit --include`

I would be more accurate to say that it is not invoked by `git commit` at all as there are several ways of staging changes including `git commit $path`. We should also be explicit that in order to ensure that all staged changes are checked the checks in the "pre-add" hook must be duplicated by the "pre-commit" hook.

> +which still can run the `pre-commit` hook, providing a control point at
> +commit time.

While I've commented on the documentation, I think it is really the design that needs working on. I like the idea of giving feedback earlier when staging changes rather than waiting for the user to run "git commit" but I think we need a more coherent approach to when the hook is run.

Thanks

Phillip