Re: [PATCH 3/4] commit: don't count i-t-a entries when checking if the new commit is empty

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

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> i-t-a entries are excluded from tree building. Relying solely on active_nr
> (or diff without --shift-ita) may lead to empty commits sometimes, when
> i-t-a entries are the only ones "changed" in the index.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/commit.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index fcfaa2b..e98ca8a 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -894,9 +894,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  		if (amend)
>  			parent = "HEAD^1";
>  
> -		if (get_sha1(parent, sha1))
> -			commitable = !!active_nr;
> -		else {
> +		if (get_sha1(parent, sha1)) {
> +			int i, ita_nr = 0;
> +
> +			for (i = 0; i < active_nr; i++)
> +				if (ce_intent_to_add(active_cache[i]))
> +					ita_nr++;
> +			commitable = active_nr - ita_nr == 0;
> +		} else {
>  			/*
>  			 * Unless the user did explicitly request a submodule
>  			 * ignore mode by passing a command line option we do
> @@ -910,6 +915,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  			if (ignore_submodule_arg &&
>  			    !strcmp(ignore_submodule_arg, "all"))
>  				diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
> +			diff_flags |= DIFF_OPT_SHIFT_INTENT_TO_ADD;
>  			commitable = index_differs_from(parent, diff_flags);

This feels like an ugly hack.  Doesn't wt-status do a separate
"diff" that enumerates what's different between the index and the
HEAD?

I am wondering if this "we do not include status and do not ask
run_status() about commitable bit" codepath should share more with
the other side, which you do not touch at all with this series,
which in turn must be doing the right thing already, apparently
without having to know anything about the SHIFT_INTENT_TO_ADD
hackery.

Or does "git commit" that uses run_status() still allow an empty
commit after this patch?