Thread (88 messages) flat view 88 messages, 6 authors, 2021-09-08

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

From: Derrick Stolee <hidden>
Date: 2021-08-05 01:55:34

On 8/2/2021 12:17 PM, Elijah Newren wrote:
On Mon, Aug 2, 2021 at 8:34 AM Derrick Stolee [off-list ref] wrote:
quoted
On 7/30/2021 9:52 AM, Elijah Newren wrote:
quoted
On Thu, Jul 29, 2021 at 11:27 AM Derrick Stolee via GitGitGadget
[off-list ref] wrote:
...
quoted
quoted
quoted
+                */
+               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.
This is _not_ true, and I can document it with a test.

Having untracked files outside of the sparse cone is just as bad as
ignored files, so I want to ensure that these get cleaned up, too.

The correct thing would be to prevent the 'git sparse-checkout
(set|add|reapply)' command from making any changes to the sparse-checkout
cone or the worktree if there are untracked files that would be deleted.
(Right? Or is there another solution that I'm missing here?)
quoted
quoted
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.
Good point. I'd like to avoid using `git clean` as a subcommand, if
possible, that way we have one fewer thing to do before integrating
the `git sparse-checkout` builtin with the sparse index.

Thanks,
-Stolee
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help