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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jul 4, 2019, 20:01 UTC
Message-ID: <20190704200133.GD20404@szeder.dev>
In-Reply-To: <20190624130226.17293-6-pclouds@gmail.com>

On Mon, Jun 24, 2019 at 08:02:21PM +0700, Nguyễn Thái Ngọc Duy wrote:

Show 68 quoted lines

> 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...