Re: [PATCH v2 05/10] split-index.c: dump "link" extension as json

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Mon, Jun 24, 2019 at 08:02:21PM +0700, Nguyễn Thái Ngọc Duy wrote:
> diff --git a/t/t3011-ls-files-json.sh b/t/t3011-ls-files-json.sh
> index 082fe8e966..dbb572ce9d 100755
> --- a/t/t3011-ls-files-json.sh
> +++ b/t/t3011-ls-files-json.sh
> @@ -44,4 +44,18 @@ test_expect_success 'ls-files --json, main entries, UNTR and TREE' '
>  	compare_json basic
>  '
>  
> +test_expect_success 'ls-files --json, split index' '
> +	git init split &&
> +	(
> +		cd split &&
> +		echo one >one &&
> +		git add one &&
> +		git update-index --split-index &&
> +		echo updated >>one &&
> +		test_must_fail git -c splitIndex.maxPercentChange=100 update-index --refresh &&
> +		cp ../filter.sed . &&
> +		compare_json split-index
> +	)
> +'
> +
>  test_done
> diff --git a/t/t3011/split-index b/t/t3011/split-index
> new file mode 100644
> index 0000000000..cdcc4ddded
> --- /dev/null
> +++ b/t/t3011/split-index
> @@ -0,0 +1,39 @@
> +{
> +  "version": 2,
> +  "oid": <string>,
> +  "mtime_sec": <number>,
> +  "mtime_nsec": <number>,
> +  "entries": [
> +    {
> +      "id": 0,
> +      "name": "",
> +      "mode": "100644",
> +      "flags": 0,
> +      "oid": <string>,
> +      "stat": {
> +        "ctime_sec": <number>,
> +        "ctime_nsec": <number>,
> +        "mtime_sec": <number>,
> +        "mtime_nsec": <number>,
> +        "device": <number>,
> +        "inode": <number>,
> +        "uid": <number>,
> +        "gid": <number>,
> +        "size": 4
> +      },
> +      "file_offset": <number>
> +    }
> +  ],
> +  "extensions": {
> +    "link": {
> +      "file_offset": <number>,
> +      "ext_size": <number>,
> +      "oid": <string>,
> +      "delete_bitmap": [
> +      ],
> +      "replace_bitmap": [
> +        0
> +      ]
> +    }
> +  }
> +}

This test is flaky, as reported in:

  https://public-inbox.org/git/xmqqftno2mku.fsf@gitster-ct.c.googlers.com/

This is because it relies on racy behaviour, namely that the following
three commands

    echo one >one &&
    git add one &&
    git update-index --split-index &&

are executed within the same second, leaving 'one' racily clean.  To
deal with the racily clean file, 5581a019ba (split-index: smudge and
add racily clean cache entries to split index, 2018-10-11) kicks in,
and 'one's smudged index entry is stored both in the shared index and
in the split index.  That's why this test expects the offset 0 in the
"replace_bitmap" array.

However, it's possible that a second boundary is crossed between
writing to 'one' and splitting the index, and then 'one' is not racily
clean, and its index entry is only stored in the shared index.
Consequently, there are no index entries in the split index, so the
"replace_bitmap" array ends up being empty, ultimately failing the
test.

A 'test-tool chmtime' invocation or two could make the test
deterministic (i.e it could make sure that 'one' is either always
racily clean or it never is, whichever is preferred).

What I still don't understand, however, is that when the test fails
this way, then the "entries" array ends up being empty as well.  It
looks as if the JSON dump included only index entries that were
actually stored in '.git/index', but omitted entries that were only
present in the shared index.  I think this is wrong, and it should
dump the unified view of the split and shared indexes.  Or include all
entries from the shared index as well.  Or perhaps I'm completely
missing something...