Re: [PATCH 05/13] add: fail when adding an untracked sparse file

On Tue, Aug 24, 2021 at 6:54 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

From: Derrick Stolee <redacted>

The add_files() method in builtin/add.c takes a set of untracked files
that are being added by the input pathspec and inserts them into the
index. If these files are outside of the sparse-checkout cone, then they
gain the SKIP_WORKTREE bit at some point. However, this was not checked
before inserting into the index, so these files are added even though we
want to avoid modifying the index outside of the sparse-checkout cone.

Add a check within add_files() for these files and write the advice
about files outside of the sprase-checkout cone.

s/sprase/sparse/

This behavior change modifies some existing tests within t1092. These
tests intended to document how a user could interact with the existing
behavior in place. Many of these tests need to be marked as expecting
failure. A future change will allow these tests to pass by adding a flag
to 'git add' that allows users to modify index entries outside of the
sparse-checkout cone.

The 'submodule handling' test is intended to document what happens to
directories that contain a submodule when the sparse index is enabled.
It is not trying to say that users should be able to add submodules
outside of the sparse-checkout cone, so that test can be modified to
avoid that operation.

While I was playing with this patch, I did the following:

echo a >a
echo b >b
git add .
git commit -m files
git sparse-checkout set a
echo c >c
git add c

And the last `git add` was successful in adding the untracked `c` file
which is outside the sparse checkout. I'm not sure if I'm doing
something wrong, but it seems that `path_in_sparse_checkout()` returns
UNDECIDED for `c`. Is it because there was no pattern in the list
explicitly excluding it? And if so, should we consider UNDECIDED as
NOT_MATCHED for `path_in_sparse_checkout()`?

I see this reuses the logic from cmd_add() and refresh(). But since we
are operating on untracked files here, perhaps we could replace
"skip_worktree" by "sparse_paths" or something similar?

Hmm, advise_on_updating_sparse_paths() takes a list of pathspecs that
only matched sparse paths, but here we are passing a list of actual
pathnames... Well, these are technically pathspecs too, but the advice
message may be confusing.

For example, if we ran `git add *.c` on a repo with the untracked
files `d1/file.c` and `d2/file.c`, we will get:

The following pathspecs didn't match any eligible path, but they do match index
entries outside the current sparse checkout:
d1/file.c
d2/file.c

However, `d1/file.c` and `d2/file.c` are neither index entries nor the
pathspecs that the user has given to `git add`. So perhaps we need to
change the error/advice message?

+               exit_status = 1;
+       }
+       string_list_clear(&only_match_skip_worktree, 0);
+
        return exit_status;
 }