Thank you for the thorough feedback -- v2 follows the architecture you outlined.
The hook now runs after staging is computed, receiving two positional arguments: a temporary copy of the original index ($1) and the lockfile containing the proposed index ($2). Hook authors inspect the computed result directly.
One trade-off: since staging must complete to produce $2, blobs are written to the object store before the hook fires. I think it's worth it though for the reasons you mentioned re: hook authors.
The CI failure is a result of an ar/parallel-hooks conflict.
Appreciate the time, effort, and thought you put into review and feedback. Excited to hear back
Chandra Kethi-Reddy
Sent with Proton Mail secure email.
On Wednesday, February 11th, 2026 at 12:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 65 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
> > The hook takes no clue from anything derived from the command line,
> > not even the pathspec (or list of individual paths computed using
> > the pathspec by the command) or the mode of operation like '-u' or
> > '--renormalize'. I am not sure how effective a decision the invoked
> > hook can make to approve or deny in this lack of information.
>
> And I do not necessarily suggest passing the pathspec arguments or
> command line options that the "git add" command received from its
> caller down to the hook, which will force hook authors to emulate
> what "git add" would do to these arguments and options, and they
> will certainly get it wrong.
>
> I wonder if we can split write_locked_index() into two so that
> writing out the in-core index to the temporary/lockfile can happen
> separately from the call to commit_locked_index(). If we can do so,
> then the following would become a viable and better implementation
> of this new feature to run the "pre-add" hook:
>
> * Determine if we will need to run this "pre-add" hook, at the
> location in the code you addded the run_hooks_opt() invocation,
> but do *NOT* run any hook there yet.
>
> * Instead, create a temporary copy of the index file if the above
> says "Yes, we are going to run the hook".
>
> * Let the code path to update the in-core index, i.e., letting
> everythning up to the "finish:" label to run normally.
>
> * Perform the first-half of the write_locked_index(), writing the
> new index contents into the lockfile, but stopping before
> committing it to the final name.
>
> * If we are running the hook, run it with two arguments, the name
> of the temporary copy of the original index we created earlier,
> and the name of this lockfile that has the proposed contents of
> the index if the hook allowed "git add" to proceed.
>
> * If we ran the hook and hook succeeded, or if we did not have to
> run the hook at all, then commit the lockfile. Otherwise abort
> the "git add" command and rollback_lock_file().
>
> * Remove the temporary file we created earlier (if any).
>
> Your hooks can "GIT_INDEX_FILE=$1 git diff --cached --name-only" to
> find out which paths already had changes added before this
> invocation of "git add", and similarly using $2 get the list of
> paths that will add further changes with this invocation. The
> latter set of paths you can inspect to see if you like the
> additional changes brought in, perhaps like
>
> #!/bin/sh
> paths=$(GIT_INDEX_FILE=$2 git diff --cached --name-only)
> GIT_INDEX_FILE=$1 git diff $paths >patch.txt
>
> if grep "^+.*secret" patch.txt
> then
> echo "do not divulge company secret!" >&2
> exit 1
> fi
>
> or something.
>
>