Thread (5 messages) 5 messages, 3 authors, 2022-06-17

Re: [WIP v2 5/5] mv: use update_sparsity() after touching sparse contents

From: Victoria Dye <hidden>
Date: 2022-06-16 16:42:16

Shaoxuan Yuan wrote:
On Sat, May 28, 2022 at 5:24 AM Victoria Dye [off-list ref] wrote:
quoted
Junio C Hamano wrote:
quoted
Victoria Dye [off-list ref] writes:
quoted
Note that you'll also probably need to check out the file(s) (if moving into
the cone) or remove them from disk (if moving out of cone). If you don't,
files moved into cone will appear "deleted" on-disk, and files moved
out-of-cone that still appear on disk will have 'SKIP_WORKTREE'
automatically disabled (see [1]).
Does it also imply that we should forbid "git mv" of a dirty path
out of the cone?  Or is that too draconian and it suffices to tweak
the rule slightly to "remove from the worktree when moving a clean
path out of cone", perhaps?  When a dirty path is moved out of cone,
we would trigger the "SKIP_WORKTREE automatically disabled" behaviour
and that would be a good thing, I imagine?
I like the idea of the modified rule as an option since it *does* complete
the move in accordance with '--force', but doesn't result in silently lost
information.

An alternative might be 'mv' refusing to move a modified file out-of-cone
(despite '--force'), printing something like
'WARNING_SPARSE_NOT_UPTODATE_FILE' ("Path 'x' not uptodate; will not remove
from working tree").

I'm not sure which would provide a more vs. less frustrating experience, but
both are at least safe in terms of preserving unstaged changes.
For me, the alternative provides a less frustrating experience.

Since it is more explicit (giving a message and directly saying NO).
quoted
Also, the `sparse-checkout` users should expect the moved file to be
missing in the working tree, as opposed to being present.
Good point, since the sparseness of the destination file would be different
depending on whether it had local modifications or not (with no indication
from 'mv' of the different treatment).

If you're interested, maybe there's a middle-ground option? Suppose you want
to move a file 'file1' to an out-of-cone location:

1. If 'file1' is clean, regardless of use of '--force', move the file & make
   it sparse.
2. If 'file1' is *not* clean and '--force' is *not* used, refuse to move the
   file (with a "Path 'file1' not uptodate; will not move. Use '--force' to
   override." type of error).
3. If 'file1' is *not* clean and '--force' is used, move the file but do not
   make it sparse.

That way, '--force' really does force the move to happen, but users are
generally warned against it. I'm still not sure what the "right" approach
is, but to your point I think it should err on the side of not surprising
the user.
And the tweaked rule suggested by Junio [1] might need an extra
 `git sparse-checkout reapply` to re-sparsify the file that moved out-of-cone
after staging its change?
Just so I understand correctly, do you mean 'git sparse-checkout reapply'
*as part of* the 'mv' operation? Or are you thinking that a user might want
to manually run 'git sparse-checkout reapply' after running 'mv'? 

If it's the former (internally calling 'git sparse-checkout reapply' in
'mv'), then no, you wouldn't want to do that. In Junio's suggestion, he said
(emphasis mine):
When a dirty path is moved out of cone, we would trigger the
"SKIP_WORKTREE automatically disabled" behaviour" *and that would be a
good thing, I imagine?*
We don't want the file moved out-of-cone to be sparse again because it has
local (on-disk) modifications that would disappear (since a file needs to be
removed from disk to be "sparse" in the eyes of 'sparse-checkout'). It's
*completely valid* behavior to have an out-of-cone file become non-sparse if
a user does something to cause that; it doesn't cause any bugs/corruption
with the repo. And, even if you did want to make the file sparse, it should
be done by manually setting 'SKIP_WORKTREE' and individually removing the
file from disk (for all the reasons I mentioned in my upthread comment [1]).

On the other hand, if you're talking about a user manually running 'git
sparse-checkout reapply' after the fact, that wouldn't work either - they'd
get an error:

warning: The following paths are not up to date and were left despite sparse patterns:
        <out-of-cone modified file>

[1] https://lore.kernel.org/git/077a0579-903e-32ad-029c-48572d471c84@github.com/ (local)
[1] https://lore.kernel.org/git/xmqq8rqm3fxa.fsf@gitster.g/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help