[0/4] Fix a few split-index bugs

Message

Philippe Blain via GitGitGadget March 22, 2023, 4 p.m. UTC

I received an internal bug report that after upgrading from v2.39.2 to
v2.40.0, some users ran into the following error message:

BUG: fsmonitor.c:21: fsmonitor_dirty has more entries than the index (57 > 0)


It sounds very much like the report we received in
https://lore.kernel.org/git/[e-mail]/,
but sadly that thread petered out when the reporter stopped being able to
reproduce the problem.

After a few days of investigating, I am convinced that this is due to some
old bugs, and not actually a regression in v2.40.0 (although I can believe
that some improvements in v2.40.0 might make it easier to run into these
bugs).

This patch series addresses those bugs.

Note: While the Git maintainer has stated a strong preference to introduce
regression tests in the same patch that fixes the corresponding regression,
this patch series starts with a stand-alone patch that demonstrates a
problematic scenario via a new test_expect_failure test case. The reason why
I specifically split out the test into its own commit is that there is a lot
of information to unpack in the commit message that is larger than any of
the subsequent bug fixes. Besides, it motivates not only the second patch
(which marks the test case as test_expect_success) but paints the larger
picture necessary to understand also the need for the remaining two patches.

This patch series is based on maint-2.37, the oldest maintenance branch it
applies without merge conflicts. When merging with next, there are only
trivial conflicts in unpack-trees.c due to en/dir-api-cleanup where
o->result is now o->internal.result.

Johannes Schindelin (4):
  split-index & fsmonitor: demonstrate a bug
  split-index; stop abusing the `base_oid` to strip the "link" extension
  fsmonitor: avoid overriding `cache_changed` bits
  unpack-trees: take care to propagate the split-index flag

 fsmonitor.h                  |  2 +-
 read-cache.c                 | 37 ++++++++++++++++++++++--------------
 t/t7527-builtin-fsmonitor.sh | 37 ++++++++++++++++++++++++++++++++++++
 unpack-trees.c               |  2 ++
 4 files changed, 63 insertions(+), 15 deletions(-)


base-commit: eb88fe1ff5ceb34845f0919b8bdc60d8a1703cf6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1497%2Fdscho%2Ffix-split-index-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1497/dscho/fix-split-index-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1497

Comments

Junio C Hamano March 22, 2023, 4:22 p.m. UTC | #1

"Johannes Schindelin via GitGitGadget" <[e-mail]>
writes:

> Note: While the Git maintainer has stated a strong preference to introduce
> regression tests in the same patch that fixes the corresponding regression,
> this patch series starts with a stand-alone patch that demonstrates a
> problematic scenario via a new test_expect_failure test case.

It is fine, especially to show existing/old bugs that need extensive
explanation.

> This patch series is based on maint-2.37, the oldest maintenance branch it
> applies without merge conflicts. When merging with next, there are only
> trivial conflicts in unpack-trees.c due to en/dir-api-cleanup where
> o->result is now o->internal.result.

Thanks for digging into old and important case.  Maintenance of the
index data structure is a crucial part of the health of the system.