git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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?

Previous: Nguyễn Thái Ngọc DuyNext: Junio C Hamano
Message 13 of 15 in “i-t-a entries again...”
  1. 0/4 i-t-a entries again...Nguyễn Thái Ngọc Duy, Jun 6, 2016
  2. 1/4 diff.h: extend "flags" field to 64 bits because we're out of bitsNguyễn Thái Ngọc Duy, Jun 6, 2016
  3. Junio C HamanoJun 6, 2016
  4. Duy NguyenJun 7, 2016
  5. stefan.naewe@atlas-elektronik.comJun 7, 2016
  6. 2/4 Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"Nguyễn Thái Ngọc Duy, Jun 6, 2016
  7. Junio C HamanoJun 6, 2016
  8. Duy NguyenJun 7, 2016
  9. Johannes SchindelinJun 9, 2016
  10. Duy NguyenSep 27, 2016
  11. Duy NguyenSep 27, 2016
  12. 3/4 commit: don't count i-t-a entries when checking if the new commit is emptyNguyễn Thái Ngọc Duy, Jun 6, 2016
  13. Junio C HamanoJun 6, 2016
  14. Junio C HamanoJun 6, 2016
  15. 4/4 commit: reinstate commit behavior before 3f6d56d via a config optionNguyễn Thái Ngọc Duy, Jun 6, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.