From: Chandra <chandrakr@pm.me>
Date: Wed, 11 Feb 2026 15:16:30 GMT
Subject: [PATCH] RE: add: support pre-add hook
Message-ID: <LQvDJSyBjBqEvnPMAY4rI3oAHHSNnBZmHV9vfKbeSFeLEIO7kAFMg8CWM9e81_QKCuoL0_a7qMMUwmcLK9Aut9abH0fqpKB5g6KOEya-n7I=@pm.me>
In-Reply-To: <xmqq8qd0zan1.fsf@gitster.g>

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:

> 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.
> 
>