On Mon, Nov 08 2021, samuelyvon9@xxxxxxxxx wrote: > From: Samuel Yvon <samuelyvon9@xxxxxxxxx> > >> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: >> The code you're moving around has a comment which seems to suggest that >> what you want *is* the desired behavior, i.e. we re-read it before >> invoking the editor, so we should have the updated diff, but just don't? > > My understanding is that it was once the behaviour and has changed over time. > I am saying this based on > https://lore.kernel.org/git/xmqqk0yripca.fsf@xxxxxxxxxxxxxxxxxxxxxx/t/#u > > Specifically, > >> Junio C Hamano <gitster@xxxxxxxxx> writes: >> Even before ec84bd00 (git-commit: Refactor creation of log message., >> 2008-02-05), the code anticipated that pre-commit may touch the index >> and tried to cope with it. >> However, ec84bd00 moved the place where we re-read the on-disk index >> in the sequence, and updated a message that used to read: >> >> - /* >> - * Re-read the index as pre-commit hook could have updated it, >> - * and write it out as a tree. >> - */ >> >> to: >> >> + /* >> + * Re-read the index as pre-commit hook could have updated it, >> + * and write it out as a tree. We must do this before we invoke >> + * the editor and after we invoke run_status above. >> + */ >> >> Unfortunately there is no mention of the reason why we "must" here. > > Looking at ec84bd00 (git-commit: Refactor creation of log message., 2008-02-05), > we can see that the editor is launched after the cache has been reset. The only > part that troubles me is the line in the comment that specifies that "we must do > this ... after we invoke run_status above". I have tested (with my limited knowledge > of the internals of git) and it seems to be of no consequence of flushing before > the call to run_status, but I might be missing something. > >> The code you're moving around has a comment which seems to suggest that >> what you want *is* the desired behavior, i.e. we re-read it before >> invoking the editor, so we should have the updated diff, but just don't? > > I think this is the case (based on the previously linked conversation). *nod*, the implicit suggestion here being: Let's put more of that summary into the commit message. It helps when looking up/discovering older behavior. The comment was first added in 2888605c649 (builtin-commit: fix partial-commit support, 2007-11-18), and quite suspicuous in timing is f5bbc3225c4 (Port git commit to C., 2007-11-08) where we moved from git-commit.sh. It's a bit of a pain to build git that old, but my hunch is that perhaps this was tested with git-commit.sh, where the reading of the index would be another process.
