Web lists-archives.com

Re: [PATCH 2/2] fsmonitor: Store fsmonitor bitmap before splitting index

On 11/9/2017 2:58 PM, Alex Vandiver wrote:
ba1b9caca6 resolved the problem of the fsmonitor data being applied to
the non-base index when reading; however, a similar problem exists
when writing the index.  Specifically, writing of the fsmonitor
extension happens only after the work to split the index has been
applied -- as such, the information in the index is only for the
non-"base" index, and thus the extension information contains only
partial data.

When saving, compute the ewah bitmap before the index is split, and
store it in the fsmonitor_dirty field, mirroring the behavior that
occurred during reading.  fsmonitor_dirty is kept from being leaked by
being freed when the extension data is written -- which always happens
precisely once, no matter the split index configuration.

Signed-off-by: Alex Vandiver <alexmv@xxxxxxxxxxx>

The patch looks like a reasonable fix to make fsmonitor work correctly with split index. I also did manual testing to verify it was working as expected.

Thanks for adding this additional test case to ensure we don't have any regressions with the interactions between fsmonitor and split-index. While the test does correctly fail before the patch and pass after the patch, I had a question about the test-dump-fsmonitor lines.

Why do you redirect stdout to stderr and then and perform an "echo" afterwards? I don't understand what benefit that provides. I removed this logic and the test still passes so am confused as to what its purpose is.

+# test that splitting the index dosn't interfere
+test_expect_success 'splitting the index results in the same state' '
+	write_integration_script &&
+	dirty_repo &&
+	git update-index --fsmonitor  &&
+	git ls-files -f >expect &&
+	test-dump-fsmonitor >&2 && echo &&
+	git update-index --fsmonitor --split-index &&
+	test-dump-fsmonitor >&2 && echo &&
+	git ls-files -f >actual &&
+	test_cmp expect actual