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.