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

[Junio C Hamano <gitster@pobox.com>]

"Chandra Kethi-Reddy via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> @@ -576,6 +579,17 @@ int cmd_add(int argc,
>  		string_list_clear(&only_match_skip_worktree, 0);
>  	}
>  
> +	if (!show_only && !no_verify) {
> +		struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
> +
> +		strvec_pushf(&opt.env, "GIT_INDEX_FILE=%s",
> +			     repo_get_index_file(repo));
> +		if (run_hooks_opt(repo, "pre-add", &opt)) {
> +			exit_status = 1;
> +			goto finish;
> +		}
> +	}
> +
>  	transaction = odb_transaction_begin(repo->objects);
>  
>  	ps_matched = xcalloc(pathspec.nr, 1);

Hmph, unless I am confused, I am a bit disappointed.  The code
snippet whose beginning we can see in the post context is
preparation for determining which paths are going to be updated, and
this new code happens before anything is added to the in-core index.

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.

Also I am not sure what good it is doing to pass GIT_INDEX_FILE as
an environment variable.  If this were a hook that is invoked by
"git commit", which may be doing a partial commit "git commit [-o]
path", the command involves multiple on-disk index files to allow
the changes to named paths jump over already added changes to other
paths, but "git add path" is always inclusive of already added
changes, and does not use anything but the main index file being
used.

So,...