Re: [PATCH v2 2/5] ls-files: add --sparse option

From: Derrick Stolee <stolee@gmail.com>
To: Elijah Newren <newren@gmail.com>,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Victoria Dye" <vdye@github.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Derrick Stolee" <derrickstolee@github.com>,
	"Derrick Stolee" <dstolee@microsoft.com>
Subject: Re: [PATCH v2 2/5] ls-files: add --sparse option
Date: Fri, 10 Dec 2021 08:51:53 -0500	[thread overview]
Message-ID: <e2a39196-5ee6-fbe7-a24c-c14c809f707d@gmail.com> (raw)
In-Reply-To: <CABPp-BF5kw69ioheMvRPoVso8FJf45Fdm_Cf-P7PfqWsqHz9Gw@mail.gmail.com>

On 12/9/2021 12:08 AM, Elijah Newren wrote:
> On Wed, Dec 8, 2021 at 11:39 AM Derrick Stolee via GitGitGadget

>> +       # With --sparse and no sparse index, nothing changes.
>> +       git -C sparse-checkout ls-files >dense &&
>> +       git -C sparse-checkout ls-files --sparse >sparse &&
>> +       test_cmp dense sparse &&
>> +
> 
> I still believe this next section deserves a comment; something like
> 
> # Set up an erroneous condition: folder1/a present despite SKIP_WORKTREE.
> # We do this just to verify consistency between sparse-index and
> non-sparse-index
> # with such files.
> 
>> +       write_script edit-content <<-\EOF &&
>> +       mkdir folder1 &&
>> +       echo content >>folder1/a
>> +       EOF
>> +       run_on_sparse ../edit-content &&
>> +
>> +       # ls-files does not notice modified files whose
>> +       # cache entries are marked SKIP_WORKTREE.
> 
> I also believe this still deserves a slight change, e.g.
> 
> # ls-files does not currently notice modified files whose
> # cache entries are marked SKIP_WORKTREE; this
> # may change in the future, but we test here that
> # sparse-index and non-sparse-index at least match.
> 
> 
>> +       test_sparse_match git ls-files --modified &&
>> +       test_must_be_empty sparse-checkout-out &&
>> +       test_must_be_empty sparse-index-out &&

Ok. Sorry I missed these in the gap between v1 and v2.

Thanks,
-Stolee