Re: [PATCH 2/2] sparse-checkout: clear tracked sparse dirs

From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Matheus Tavares Bernardino <matheus.bernardino@usp.br>,
	Derrick Stolee <derrickstolee@github.com>,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH 2/2] sparse-checkout: clear tracked sparse dirs
Date: Mon, 2 Aug 2021 10:17:17 -0600	[thread overview]
Message-ID: <CABPp-BE1aKAsp6yKAzM-djRHuiP=GHC_Q7xZxMWPJ=f28bOeUg@mail.gmail.com> (raw)
In-Reply-To: <76639e16-204d-7812-d4c5-56c70e280bed@gmail.com>

On Mon, Aug 2, 2021 at 8:34 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 7/30/2021 9:52 AM, Elijah Newren wrote:
> > On Thu, Jul 29, 2021 at 11:27 AM Derrick Stolee via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
...
> >> +                */
> >> +               if (S_ISSPARSEDIR(ce->ce_mode) &&
> >> +                   repo_file_exists(r, ce->name)) {
> >> +                       strbuf_setlen(&path, pathlen);
> >> +                       strbuf_addstr(&path, ce->name);
> >> +
> >> +                       /*
> >> +                        * Removal is "best effort". If something blocks
> >> +                        * the deletion, then continue with a warning.
> >> +                        */
> >> +                       if (remove_dir_recursively(&path, 0))
> >> +                               warning(_("failed to remove directory '%s'"), path.buf);
> >
> > Um, doesn't this delete untracked files that are not ignored as well
> > as the ignored files?  If so, was that intentional?  I'm fully on
> > board with removing the gitignore'd files, but I'm worried removing
> > other untracked files is dangerous.
>
> I believe that 'git sparse-checkout (set|add|reapply)' will fail before
> reaching this method if there are untracked files that could potentially
> be removed. I will double-check to ensure this is the case. It is
> definitely my intention to protect any untracked, non-ignored files in
> these directories by failing the sparse-checkout modification.
>
> > My implementation of this concept (in an external tool) was more along
> > the lines of
> >
> >   * Get $LIST_OF_NON_SPARSE_DIRECTORIES by walking `git ls-files -t`
> > output and finding common fully-sparse directories
> >   * git clean -fX $LIST_OF_NON_SPARSE_DIRECTORIES
>
> I initially was running 'git clean -dfx -- <dir> ...' but that also
> requires parsing and expanding the index (or being very careful with
> the sparse index).

`git clean -dfx -- <dir> ...` could also be very dangerous because
it'd delete untracked non-ignored files.  You want -X rather than -x.
One of those cases where capitalization is critical.