Re: [PATCH v2 0/5] Sparse Index: Integrate with 'git add'

On Thu, Jul 29, 2021 at 8:49 AM Derrick Stolee [off-list ref] wrote:

On 7/28/2021 10:57 PM, Elijah Newren wrote:

quoted

On Wed, Jul 28, 2021 at 8:03 PM Derrick Stolee [off-list ref] wrote:

quoted

On 7/28/2021 7:13 PM, Elijah Newren wrote:

quoted

On Mon, Jul 26, 2021 at 9:18 AM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

...

quoted

quoted

I agree about ignored files, and that is true whether or not they
are in the sparse cone.

Yes, and...

quoted

quoted

I think the same logic holds for adding untracked files
outside the sparsity cone.

In my opinion, "outside the sparsity cone" is another form of "being
ignored", and in my mind should be treated similarly -- it should
generally require an override to add such files.  (Case (c) possibly
being an exception, though maybe even it shouldn't be.)

I don't hold that same interpretation. I think of it instead as
"hidden" files, but they still matter. I also think that advising
one to adjust their sparsity patterns might be dangerous because
not all users know the ramifications of doing that. They might
accidentally download an enormous amount of data to correct a
single file.

Having an override seems like the best option, and we can hopefully
make it consistent across all the cases and commands.

I think we might be arguing two sides of the same coin at this point.
We don't have a more general term for special in a way that shouldn't
be included by default with git-add, and I couldn't think of a good
synonym, so I used the words "another form of being ignored" (not
trying to imply that it was the same as .gitignored, but just that the
two were special in a very similar way) while you tried to highlight
the differences using "hidden" but agreed they were similar in that
they should have an override.

Fair point on adjusting sparsity patterns and the data download it can cause.

...

quoted

Trying to get out of a corner we paint ourselves into with
sparse-checkout would be massively harder, which is why I keep harping
on this kind of thing.  I'm very concerned it's happening even despite
my numerous comments and worries about it.

...

quoted

I'm totally fine with such changes not being part of this series.  I
just don't want a test_expect_success that checks for behavior that I
consider buggy unless it comes with a disclaimer that it's checking
for existing rather than expected behavior.

I understand your perspective. I'll send a v3 soon that adds a
comment on top of the entire test signalling the things we talked
about here: this is a documentation of behavior, not an endorsement,
and we should probably change it because users can get confused.

Thanks for doing that.  :-)