Re: [PATCH v3 0/5] Sparse index: fetch, pull, ls-files

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 10, 2021, 16:16 UTC
Message-ID: <211210.86v8zwbev9.gmgdl@evledraar.gmail.com>
In-Reply-To: <pull.1080.v3.git.1639149192.gitgitgadget@gmail.com>

On Fri, Dec 10 2021, Derrick Stolee via GitGitGadget wrote:

Show 6 quoted lines

> Updates in v3
> =============
>
>  * Fixed typo in commit message.
>  * Added comments around doing strange things in an ls-files test.
>  * Fixed adjacent typo in a test comment.

Yay, I'm happy to see 5/5. Not because I didn't like the helper, but that sparse is getting mature enough that we're getting ls-files to emit information about it. Thanks.

There's the small "diff -u" portability issue noted in my just-sent <211210.86zgp8bi48.gmgdl@evledraar.gmail.com>.

Other than that 2/5 adds this documentation about ls-files --sparse:

	If the index is sparse, show the sparse directories without expanding
	to the contained files.

Shouldn't we at least add:

	Sparse directories will be shown with a trailing slash,
	e.g. "x/" for a sparse directory "x".q

In addition to that I think this may have a buggy/unexpected interaction with the --eol option:

    040000 aaff74984cccd156a469afa7d9ab10e4777beb24 0       i/      w/      attr/                   x/

I.e. should we be saying anything about the EOL state of these? OTOHO I tried adding a submodule and it says the same, which seems similarly odd, so maybe it's either correct, or this isn't updated for those either.

Is the behavior of:

    $ git -C sparse-index ls-files --stage --sparse -- 'folder2/a'
    $ echo $?
    0

Expected? I.e. accepting /a when we'd just print "folder2/" and not e.g. erroring (probably, just asking)?

How about:

    $ ls -l sparse-index/x
    ls: cannot access 'sparse-index/x': No such file or directory
    $ git -C sparse-index ls-files --stage 'x/*'
    100644 78981922613b2afb6025042ff6bd878ac1994e85 0       x/a
    $ git -C sparse-index ls-files --stage --no-empty-directory 'x/*' 
    100644 78981922613b2afb6025042ff6bd878ac1994e85 0       x/a
    $ git -C sparse-index ls-files --stage --no-empty-directory --sparse 'x/*' 
    040000 aaff74984cccd156a469afa7d9ab10e4777beb24 0       x/

The answer is probably "yes that's fine" because I've got no idea how sparse really works, but just checking..

So it's very nice to have the new diff test in 2/5, but would be much nicer/assuring to have that split into a trivial function followed by seeing how the diff looked in combination with each of the other option that "ls-files" accepts.