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

Re: [PATCH 3/3] commit -a -m: allow the top-level tree to become empty again

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 29, 2023, 19:17 UTC
Message-ID
<xmqq5y769iyl.fsf@gitster.g>
In-Reply-To
<08c50b64e2a93300eed196505936e58ce8bb639b.1688044991.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 9 quoted lines
> That logic was introduced to add a shortcut when committing without
> editing the commit message interactively. A part of that logic was to
> ensure that the index was read into memory:
>
> 	if (!active_nr && read_cache() < 0)
> 		die(...)
>
> Translation to English: If the index has not yet been read, read it, and
> if that fails, error out.

Well described. It does make sense to turn !active_nr used here into a check on the .initialized member.

Show 6 quoted lines
> And it was natural to do it this way because at the time that condition
> was introduced, the `index_state` structure had no explicit flag to
> indicate that it was initialized: This flag was only introduced in
> 913e0e99b6a (unpack_trees(): protect the handcrafted in-core index from
> read_cache(), 2008-08-23), but that commit did not adjust the code path
> where no index file was found and a new, pristine index was initialized.

My mistake, but after 15 years it probably is beyond statute of limitations ;-)

Show 6 quoted lines
> Using the `initialized` flag instead, we avoid that mistake, and as a
> bonus we can fix a bug at the same time that was introduced by the
> memory leak fix: When deleting all tracked files and then asking `git
> commit -a -m ...` to commit the result, Git would internally update the
> index, then discard and re-read the index undoing the update, and fail
> to commit anything.

That does sound like the primary bug fixed with this change, not a bonus, but anyway, the change is very sensible and clearly described with a good test. Will queue.

Thanks.
Previous: Johannes Schindelin via GitGitGadget
Message 6 of 6 in “commit -a -m: allow the top-level tree to become empty again”
  1. 0/3 commit -a -m: allow the top-level tree to become empty againJohannes Schindelin via GitGitGadget, Jun 29, 2023
  2. 1/3 do_read_index(): always mark index as initialized unless erroring outJohannes Schindelin via GitGitGadget, Jun 29, 2023
  3. 2/3 split-index: accept that a base index can be emptyJohannes Schindelin via GitGitGadget, Jun 29, 2023
  4. Junio C HamanoJun 29, 2023
  5. 3/3 commit -a -m: allow the top-level tree to become empty againJohannes Schindelin via GitGitGadget, Jun 29, 2023
  6. Junio C HamanoJun 29, 2023

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.