Re: [PATCH 3/4] commit: don't count i-t-a entries when checking if the new commit is empty
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 6, 2016, 19:58 UTC
- Message-ID
- <xmqqbn3ew0mf.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160606111643.7122-4-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 37 quoted lines
> 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?