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

[Ben Peart <peartben@gmail.com>]

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@dropbox.com>
> ---

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
> +'
> +
>   test_done
>