{"thread":{"id":"58554","subject":"Bug report: `git restore --source --staged` deals poorly with sparse-checkout","startedAt":"2022-10-03T22:06:07Z","lastAt":"2022-10-06T19:38:42Z","messageCount":12,"participants":["Glen Choo","Victoria Dye","Martin von Zweigbergk","Elijah Newren","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"464100","messageId":"kl6l35c4mukf.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"58554","inReplyTo":null,"subject":"Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-10-03T22:05:52Z","receivedAt":"2022-10-03T22:06:07Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Filing a `git bugreport` on behalf of a user at $DAYJOB. I'm also pretty\nsurprised by this behavior, perhaps someone who knows more could shed\nsome light?\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n\n git clone git@github.com:git/git.git . &&\n git sparse-checkout set t &&\n git restore --source v2.38.0-rc1 --staged Documentation &&\n git status\n\nWhat did you expect to happen? (Expected behavior)\n\nI expected to see staged changes only, since I restored only paths\noutside of my sparse spec (which was t/, plus the implicit root\ndirectory).\n\nWhat happened instead? (Actual behavior)\n\nI saw a staged modification (Documentation/cmd-list.perl) and the same\nfile reported as deleted in the working copy. Specifically,\n\n $ git status\n\n On branch master\n Your branch is up to date with 'origin/master'.\n\n You are in a sparse checkout with 64% of tracked files present.\n\n Changes to be committed:\n (use \"git restore --staged ...\" to unstage)\n modified: Documentation/cmd-list.perl\n\n Changes not staged for commit:\n (use \"git add/rm ...\" to update what will be committed)\n (use \"git restore ...\" to discard changes in working directory)\n deleted: Documentation/cmd-list.perl\n\nWhat's different between what you expected and what actually happened?\n\ngit status should not have said that the file was deleted in the\nworking copy\n\n[System Info]\ngit version: git version 2.37.3.998.g577e59143f-goog\ncpu: x86_64 no commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Linux 5.17.11-1rodete2-amd64 #1 SMP PREEMPT Debian\n5.17.11-1rodete2 (2022-06-09) x86_64\ncompiler info: gnuc: 12.2\nlibc info: glibc: 2.33\n$SHELL (typically, interactive shell): /bin/bash\n"},{"id":"464194","messageId":"54ee4a2a-1937-8640-9297-8ad1516596cc@github.com","threadId":"58554","inReplyTo":"kl6l35c4mukf.fsf@chooglen-macbookpro.roam.corp.google.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2022-10-04T16:34:49Z","receivedAt":"2022-10-04T16:34:58Z","isPatch":false,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Glen Choo wrote:\n> Filing a `git bugreport` on behalf of a user at $DAYJOB. I'm also pretty\n> surprised by this behavior, perhaps someone who knows more could shed\n> some light?\n> \n> What did you do before the bug happened? (Steps to reproduce your issue)\n> \n> git clone git@github.com:git/git.git . &&\n> git sparse-checkout set t &&\n> git restore --source v2.38.0-rc1 --staged Documentation &&\n> git status\n> ...> \n> What happened instead? (Actual behavior)\n> \n> I saw a staged modification (Documentation/cmd-list.perl) and the same\n> file reported as deleted in the working copy. Specifically,\n> \n> $ git status\n> \n> On branch master\n> Your branch is up to date with 'origin/master'.\n> \n> You are in a sparse checkout with 64% of tracked files present.\n> \n> Changes to be committed:\n> (use \"git restore --staged ...\" to unstage)\n> modified: Documentation/cmd-list.perl\n> \n> Changes not staged for commit:\n> (use \"git add/rm ...\" to update what will be committed)\n> (use \"git restore ...\" to discard changes in working directory)\n> deleted: Documentation/cmd-list.perl\n> \n\nThanks for reporting this! There are a few confusing things going on with\n'restore' here.\n\nFirst is that the out-of-cone was even restored in the first place.\nTheoretically, 'restore' (like 'checkout') should be limited to pathspecs\ninside the sparse-checkout patterns (per the documentation of\n'--ignore-skip-worktree-bits'), but 'Documentation' does not match them.\nThen, there's a difference between 'restore' and 'checkout' that doesn't\nseem intentional; both remove the 'SKIP_WORKTREE' flag from the file, but\nonly 'checkout' creates the file on-disk (therefore avoiding the \"deleted\"\nstatus).\n\nElijah's WIP design doc [1] describes 'restore' as one of:\n\n> commands that restore files to the working tree that match sparsity\n> patterns, and remove unmodified files that don't match those patterns\n\nalbeit with other (probably related?) bugs. Given that, I think the correct\nbehavior would be:\n\n1. if '--ignore-skip-worktree-bits' *is not* specified, do not restore any\n files outside of the sparse-checkout patterns.\n2. if '--ignore-skip-worktree-bits' *is* specified, remove the\n 'SKIP_WORKTREE' bit & check out all entries matching the pathspec to\n disk.\n\nFixing this should probably wait until the design doc is finalized (I've\nmeant to follow up on it for a while now, but I should have some time to do\nthat this week). In the meantime, 'checkout' will at least allow you to (for\nbetter or for worse) avoid the \"deleted\" status.\n\nHope that helps!\n- Victoria\n\n[1] https://lore.kernel.org/git/pull.1367.v2.git.1664353951797.gitgitgadget@gmail.com/\n"},{"id":"464208","messageId":"96c4f52e-bc66-f4ee-f4f6-d22da579858e@github.com","threadId":"58554","inReplyTo":"CAESOdVAh68HoQoyicfZn4XbjGfiRFCu1zFQmUjMcSAg3tUzr4Q@mail.gmail.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2022-10-04T20:34:59Z","receivedAt":"2022-10-04T20:35:06Z","isPatch":false,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Hi Martin!\n\nPlease make sure you respond in plaintext - it looks like your message didn't\nmake it to the mailing list. I've reformatted it to render nicely in this \nreply.\n\nMartin von Zweigbergk wrote:\n>> On Tue, Oct 4, 2022 at 9:34 AM Victoria Dye > wrote:\n>> \n>> Glen Choo wrote:\n>>> Filing a `git bugreport` on behalf of a user at $DAYJOB. I'm also pretty\n>>> surprised by this behavior, perhaps someone who knows more could shed\n>>> some light?\n>>>\n>>> What did you do before the bug happened? (Steps to reproduce your issue)\n>>>\n>>>   git clone git@github.com:git/git.git . &&\n>>>   git sparse-checkout set t &&\n>>>   git restore --source v2.38.0-rc1 --staged Documentation &&\n>>>   git status\n>>> ...>\n>>> What happened instead? (Actual behavior)\n>>>\n>>> I saw a staged modification (Documentation/cmd-list.perl) and the same\n>>> file reported as deleted in the working copy. Specifically,\n>>>\n>>>   $ git status\n>>>\n>>>   On branch master\n>>>   Your branch is up to date with 'origin/master'.\n>>>\n>>>   You are in a sparse checkout with 64% of tracked files present.\n>>>\n>>>   Changes to be committed:\n>>>     (use \"git restore --staged ...\" to unstage)\n>>>           modified:   Documentation/cmd-list.perl\n>>>\n>>>   Changes not staged for commit:\n>>>     (use \"git add/rm ...\" to update what will be committed)\n>>>     (use \"git restore ...\" to discard changes in working directory)\n>>>           deleted:    Documentation/cmd-list.perl\n>>>\n>> \n>> Thanks for reporting this! There are a few confusing things going on with\n>> 'restore' here.\n>> \n>> First is that the out-of-cone was even restored in the first place.\n>> \n> \n> I was actually happy that the out-of-cone paths were restored. I ran that\n> command as an experiment while reading Elijah's doc because I was curious\n> what would happen. The reason I think it should restore out-of-cone paths is\n> so you can do `git restore --staged --source && git commit -m\n> \"restore to old commit\"` without caring about the sparse spec.\n\nConversely, that's behavior a user *wouldn't* want if they want to keep\ntheir sparse cone intact (not to mention the performance impact of checking\nout the entire worktree). I think it does more harm to those users than it\nwould benefit the ones that want to checkout out-of-cone files.\n\nThe use-case you're describing should be served by the\n'--ignore-skip-worktree-bits' option (not the most intuitive name,\nunfortunately). Luckily, there's an increasing desire to improve the naming\nof sparse-related options, so the UX situation should improve in the future.\n\n> \n>> Theoretically, 'restore' (like 'checkout') should be limited to pathspecs\n>> inside the sparse-checkout patterns (per the documentation of\n>> '--ignore-skip-worktree-bits'), but 'Documentation' does not match them.\n>> Then, there's a difference between 'restore' and 'checkout' that doesn't\n>> seem intentional; both remove the 'SKIP_WORKTREE' flag from the file, but\n>> only 'checkout' creates the file on-disk (therefore avoiding the \"deleted\"\n>> status).\n> \n> Restoring only into the index (as I think `git restore --staged` is supposed\n> to do) is weird. \n\n'git restore --staged' is intended to restore to both the worktree and index\n(per 183fb44fd2 (restore: add --worktree and --staged, 2019-04-25)). The bug\nyou've identified is that it's not restoring to the worktree.\n\nAssuming everything was working properly, users could still choose to\nrestore only to the index (using the '--no-worktree' option).\n\n> Let's say we do a clean checkout of a commit with tree A\n> (i.e. the root tree's hash is A). If we do `git sparse-checkout set\n> non-existent`, the index and the working copy still logically contain state\n> A, right? \n\nThe index will, but the working tree will be empty because all index entries\nnot matching 'non-existent' will have SKIP_WORKTREE applied.\n\n> If we now do `git restore --staged --source HEAD^` and that\n> command doesn't remove the `SKIP_WORKTREE` flag on any paths, that logically\n> means that we have modified the working copy, and I think `git\n> sparse-checkout disable` would agree with me. \n\nIf you aren't using '--ignore-skip-worktree-bits', the entries with\nSKIP_WORKTREE shouldn't be touched in the first place. If you *do* specify\nit, by virtue of restoring to the working tree, SKIP_WORKTREE must be\nremoved.\n\nBut suppose you're doing something like 'git restore --staged --no-worktree\n--ignore-skip-worktree-bits --source HEAD^'. In that case:\n\n- you are restoring to the index\n- you are *not* restoring to the worktree\n- you're restoring files with SKIP_WORKTREE applied\n\nRight now, SKIP_WORKTREE is removed from the matched entries, but suppose\n(per your comment) it wasn't. That wouldn't mean that we've \"modified the\nworking copy\"; the working tree is defined with respect to the index, and if\nthe index entry says \"I don't care about the worktree\", then there are no\ndifferences to reflect. \n\nThis raises an interesting question about the current behavior, though: if\nyou restore a SKIP_WORKTREE entry with '--staged' and '--no-worktree',\nshould we remove SKIP_WORKTREE? I'd lean towards \"no\", but I'm interested to\nhear other contributors' thoughts.\n\n> That's different from how `git\n> restore --staged` without sparse-checkout would have worked (it would not\n> have updated the working copy). So from that perspective, it might make\n> sense to remove the `SKIP_WORKTREE` and add the old file contents back in\n> the working (i.e. from state A in this example), and maybe that's why the\n> commands do that? \n\nIt's important to avoid restoring a file to the worktree when it has\nSKIP_WORKTREE enabled. See af6a51875a (repo_read_index: clear SKIP_WORKTREE\nbit from files present in worktree, 2022-01-14) and the corresponding\ndiscussion in [1].\n\n[1] https://lore.kernel.org/all/pull.1114.v2.git.1642175983.gitgitgadget@gmail.com/\n\n> Actually, `git checkout HEAD^ .` would update both the\n> index and the working copy to match\n> HEAD^, so that shouldn't have to remove the `SKIP_WORKTREE`, maybe?\n> \n\n\n"},{"id":"464213","messageId":"CAESOdVByucFm=yJn2yL1mwKGqey7tHXH4A-JM-yP125Ok+_Q+g@mail.gmail.com","threadId":"58554","inReplyTo":"96c4f52e-bc66-f4ee-f4f6-d22da579858e@github.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@google.com","sentAt":"2022-10-05T04:53:25Z","receivedAt":"2022-10-05T04:53:44Z","isPatch":false,"sender":{"key":"martinvonz@google.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Tue, Oct 4, 2022 at 1:35 PM Victoria Dye wrote:\n>\n> Hi Martin!\n>\n> Please make sure you respond in plaintext - it looks like your message didn't\n> make it to the mailing list. I've reformatted it to render nicely in this\n> reply.\n\nHi! Sorry about that, and thanks for fixing! I've switched to plain\ntext now, and I've\nmanually added newlines to keep lines short (unsure if I was supposed to).\n\n>\n> Martin von Zweigbergk wrote:\n> >> On Tue, Oct 4, 2022 at 9:34 AM Victoria Dye > wrote:\n> >>\n> >> Glen Choo wrote:\n> >>> Filing a `git bugreport` on behalf of a user at $DAYJOB. I'm also pretty\n> >>> surprised by this behavior, perhaps someone who knows more could shed\n> >>> some light?\n> >>>\n> >>> What did you do before the bug happened? (Steps to reproduce your issue)\n> >>>\n> >>> git clone git@github.com:git/git.git . &&\n> >>> git sparse-checkout set t &&\n> >>> git restore --source v2.38.0-rc1 --staged Documentation &&\n> >>> git status\n> >>> ...>\n> >>> What happened instead? (Actual behavior)\n> >>>\n> >>> I saw a staged modification (Documentation/cmd-list.perl) and the same\n> >>> file reported as deleted in the working copy. Specifically,\n> >>>\n> >>> $ git status\n> >>>\n> >>> On branch master\n> >>> Your branch is up to date with 'origin/master'.\n> >>>\n> >>> You are in a sparse checkout with 64% of tracked files present.\n> >>>\n> >>> Changes to be committed:\n> >>> (use \"git restore --staged ...\" to unstage)\n> >>> modified: Documentation/cmd-list.perl\n> >>>\n> >>> Changes not staged for commit:\n> >>> (use \"git add/rm ...\" to update what will be committed)\n> >>> (use \"git restore ...\" to discard changes in working directory)\n> >>> deleted: Documentation/cmd-list.perl\n> >>>\n> >>\n> >> Thanks for reporting this! There are a few confusing things going on with\n> >> 'restore' here.\n> >>\n> >> First is that the out-of-cone was even restored in the first place.\n> >>\n> >\n> > I was actually happy that the out-of-cone paths were restored. I ran that\n> > command as an experiment while reading Elijah's doc because I was curious\n> > what would happen. The reason I think it should restore out-of-cone paths is\n> > so you can do `git restore --staged --source && git commit -m\n> > \"restore to old commit\"` without caring about the sparse spec.\n>\n> Conversely, that's behavior a user *wouldn't* want if they want to keep\n> their sparse cone intact (not to mention the performance impact of checking\n> out the entire worktree). I think it does more harm to those users than it\n> would benefit the ones that want to checkout out-of-cone files.\n>\n> The use-case you're describing should be served by the\n> '--ignore-skip-worktree-bits' option (not the most intuitive name,\n> unfortunately). Luckily, there's an increasing desire to improve the naming\n> of sparse-related options, so the UX situation should improve in the future.\n\nI realized after sending my previous email that I might have a\ndifferent view of what\nsparse checkout is about. To me, it seems like it should be just a performance\noptimization. That's why I feel like commands should behave the same way with\nor without a sparse spec (unless that proposed `--restrict` flag is passed). I\nunderstand if that's just not feasible. Sorry about the noise in that case :)\n\n>\n> >\n> >> Theoretically, 'restore' (like 'checkout') should be limited to pathspecs\n> >> inside the sparse-checkout patterns (per the documentation of\n> >> '--ignore-skip-worktree-bits'), but 'Documentation' does not match them.\n> >> Then, there's a difference between 'restore' and 'checkout' that doesn't\n> >> seem intentional; both remove the 'SKIP_WORKTREE' flag from the file, but\n> >> only 'checkout' creates the file on-disk (therefore avoiding the \"deleted\"\n> >> status).\n> >\n> > Restoring only into the index (as I think `git restore --staged` is supposed\n> > to do) is weird.\n>\n> 'git restore --staged' is intended to restore to both the worktree and index\n> (per 183fb44fd2 (restore: add --worktree and --staged, 2019-04-25)). The bug\n> you've identified is that it's not restoring to the worktree.\n\nAh, `--worktree` is on by default even if I pass `--staged`, I see. Hmm, the\nhelp text actually says \"Specifying --staged will only restore the index.\"...\n\n>\n> Assuming everything was working properly, users could still choose to\n> restore only to the index (using the '--no-worktree' option).\n>\n> > Let's say we do a clean checkout of a commit with tree A\n> > (i.e. the root tree's hash is A). If we do `git sparse-checkout set\n> > non-existent`, the index and the working copy still logically contain state\n> > A, right?\n>\n> The index will, but the working tree will be empty because all index entries\n> not matching 'non-existent' will have SKIP_WORKTREE applied.\n>\n> > If we now do `git restore --staged --source HEAD^` and that\n> > command doesn't remove the `SKIP_WORKTREE` flag on any paths, that logically\n> > means that we have modified the working copy, and I think `git\n> > sparse-checkout disable` would agree with me.\n>\n> If you aren't using '--ignore-skip-worktree-bits', the entries with\n> SKIP_WORKTREE shouldn't be touched in the first place. If you *do* specify\n> it, by virtue of restoring to the working tree, SKIP_WORKTREE must be\n> removed.\n>\n> But suppose you're doing something like 'git restore --staged --no-worktree\n> --ignore-skip-worktree-bits --source HEAD^'. In that case:\n>\n> - you are restoring to the index\n> - you are *not* restoring to the worktree\n> - you're restoring files with SKIP_WORKTREE applied\n>\n> Right now, SKIP_WORKTREE is removed from the matched entries, but suppose\n> (per your comment) it wasn't. That wouldn't mean that we've \"modified the\n> working copy\"; the working tree is defined with respect to the index, and if\n> the index entry says \"I don't care about the worktree\", then there are no\n> differences to reflect.\n\nYes, not technically changed. I was (and still am) thinking of the working copy\nas logically containing all the files even if some of them are not written out\nto disk. I understand if that seems like an odd way of thinking about\nit. It might\nhelp to think about how it might appear in a virtual file system where clean\nfiles outside the sparse spec are presented from the index. Or it may be better\nto not try to think about it in this weird way at all :)\n\n>\n> This raises an interesting question about the current behavior, though: if\n> you restore a SKIP_WORKTREE entry with '--staged' and '--no-worktree',\n> should we remove SKIP_WORKTREE?\n\nFWIW, that's the case I meant.\n\n> I'd lean towards \"no\", but I'm interested to\n> hear other contributors' thoughts.\n>\n> > That's different from how `git\n> > restore --staged` without sparse-checkout would have worked (it would not\n> > have updated the working copy). So from that perspective, it might make\n> > sense to remove the `SKIP_WORKTREE` and add the old file contents back in\n> > the working (i.e. from state A in this example), and maybe that's why the\n> > commands do that?\n>\n> It's important to avoid restoring a file to the worktree when it has\n> SKIP_WORKTREE enabled. See af6a51875a (repo_read_index: clear SKIP_WORKTREE\n> bit from files present in worktree, 2022-01-14) and the corresponding\n> discussion in [1].\n\nMakes sense. That's why I said \"and add the old file contents back in the\nworking copy\" (if I understood correctly what that discussion was about).\n\n>\n> [1] https://lore.kernel.org/all/pull.1114.v2.git.1642175983.gitgitgadget@gmail.com/\n>\n> > Actually, `git checkout HEAD^ .` would update both the\n> > index and the working copy to match\n> > HEAD^, so that shouldn't have to remove the `SKIP_WORKTREE`, maybe?\n> >\n>\n>\n"},{"id":"464214","messageId":"CABPp-BEZeKdO_UzgO=7EX0EszVsNSiiySuGT_tuhtghLBtLWOQ@mail.gmail.com","threadId":"58554","inReplyTo":"CAESOdVAh68HoQoyicfZn4XbjGfiRFCu1zFQmUjMcSAg3tUzr4Q@mail.gmail.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-10-05T05:00:10Z","receivedAt":"2022-10-05T05:00:27Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Marin!\n\nOn Tue, Oct 4, 2022 at 10:52 AM Martin von Zweigbergk\n wrote:\n>\n> On Tue, Oct 4, 2022 at 9:34 AM Victoria Dye wrote:\n>>\n>> Glen Choo wrote:\n>> > Filing a `git bugreport` on behalf of a user at $DAYJOB. I'm also pretty\n>> > surprised by this behavior, perhaps someone who knows more could shed\n>> > some light?\n>> >\n>> > What did you do before the bug happened? (Steps to reproduce your issue)\n>> >\n>> > git clone git@github.com:git/git.git . &&\n>> > git sparse-checkout set t &&\n>> > git restore --source v2.38.0-rc1 --staged Documentation &&\n>> > git status\n>> > ...>\n>> > What happened instead? (Actual behavior)\n>> >\n>> > I saw a staged modification (Documentation/cmd-list.perl) and the same\n>> > file reported as deleted in the working copy. Specifically,\n>> >\n>> > $ git status\n>> >\n>> > On branch master\n>> > Your branch is up to date with 'origin/master'.\n>> >\n>> > You are in a sparse checkout with 64% of tracked files present.\n>> >\n>> > Changes to be committed:\n>> > (use \"git restore --staged ...\" to unstage)\n>> > modified: Documentation/cmd-list.perl\n>> >\n>> > Changes not staged for commit:\n>> > (use \"git add/rm ...\" to update what will be committed)\n>> > (use \"git restore ...\" to discard changes in working directory)\n>> > deleted: Documentation/cmd-list.perl\n>> >\n>>\n>> Thanks for reporting this! There are a few confusing things going on with\n>> 'restore' here.\n>>\n>> First is that the out-of-cone was even restored in the first place.\n>\n>\n> I was actually happy that the out-of-cone paths were restored. I ran that command as an experiment while reading Elijah's doc because I was curious what would happen. The reason I think it should restore out-of-cone paths is so you can do `git restore --staged --source && git commit -m \"restore to old commit\"` without caring about the sparse spec.\n\nI think that could lead to something that would be very dangerous or\nhighly confusing to other users. In particular, if they run\n\n `git restore --staged --source -- '*.rs'`\n\nand git changes not only all the Rust files inside the sparsity\nspecification (i.e. the files they are interested in), but all the\nones outside too, then they'll be rather unhappy. So, I think if the\npaths you specify aren't within the sparse specification, we should\nthrow an error, much like we already do with `git add` and `git rm`\nwhen in a sparse checkout. And if you don't care about the sparse\nspec despite having set up a sparse checkout, you can always specify\nthat (with --no-restrict or --scope=all or whatever).\n\n>> Theoretically, 'restore' (like 'checkout') should be limited to pathspecs\n>> inside the sparse-checkout patterns (per the documentation of\n>> '--ignore-skip-worktree-bits'), but 'Documentation' does not match them.\n>> Then, there's a difference between 'restore' and 'checkout' that doesn't\n>> seem intentional; both remove the 'SKIP_WORKTREE' flag from the file, but\n>> only 'checkout' creates the file on-disk (therefore avoiding the \"deleted\"\n>> status).\n>\n>\n> Restoring only into the index (as I think `git restore --staged` is supposed to do) is weird. Let's say we do a clean checkout of a commit with tree A (i.e. the root tree's hash is A). If we do `git sparse-checkout set non-existent`, the index and the working copy still logically contain state A, right? If we now do `git restore --staged --source HEAD^` and that command doesn't remove the `SKIP_WORKTREE` flag on any paths, that logically means that we have modified the working copy, and I think `git sparse-checkout disable` would agree with me. That's different from how `git restore --staged` without sparse-checkout would have worked (it would not have updated the working copy). So from that perspective, it might make sense to remove the `SKIP_WORKTREE` and add the old file contents back in the working (i.e. from state A in this example), and maybe that's why the commands do that? Actually, `git checkout HEAD^ .` would update both the index and the working copy to match HEAD^, so that shouldn't have to remove the `SKIP_WORKTREE`, maybe?\n\nYes, you've flagged this correctly as an issue, but I think only\ntouching files within the sparse specification is much safer and\nthrowing an error telling the user what flag to add if they specified\na path outside the sparse specification would be better.\n\nNow, if they do provide the override...then your question becomes\nvalid. My inclination there is that they provided --staged without\n--worktree and provided an override so although they'll get weird\nresults (much as they also would with git-add or git-rm when they\noverride) we just follow what they said and only update the index and\nleave the file as SKIP_WORKTREE.\n\n> I barely ever use Git, so take all that with a grain of salt.\n>\n>>\n>> Elijah's WIP design doc [1] describes 'restore' as one of:\n>>\n>> > commands that restore files to the working tree that match sparsity\n>> > patterns, and remove unmodified files that don't match those patterns\n>\n>\n> I *think* that only applies to `git restore` without `--staged`.\n\nYeah, you brought up a really good example here. I think `restore`\nand `checkout -- ` should probably be better grouped with\nadd/rm/mv\n"},{"id":"464215","messageId":"CABPp-BGeC3hXw-v3voniY5ZU2f6W8NXfXVvq0C03eGGhvSefgg@mail.gmail.com","threadId":"58554","inReplyTo":"96c4f52e-bc66-f4ee-f4f6-d22da579858e@github.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-10-05T05:22:53Z","receivedAt":"2022-10-05T05:23:11Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Oct 4, 2022 at 1:35 PM Victoria Dye wrote:\n>\n[...]\n> >> Thanks for reporting this! There are a few confusing things going on with\n> >> 'restore' here.\n> >>\n> >> First is that the out-of-cone was even restored in the first place.\n\nYep, agreed. I'll add this example to the Known bugs section.\n\n[...]\n> >> Theoretically, 'restore' (like 'checkout') should be limited to pathspecs\n> >> inside the sparse-checkout patterns (per the documentation of\n> >> '--ignore-skip-worktree-bits'), but 'Documentation' does not match them.\n> >> Then, there's a difference between 'restore' and 'checkout' that doesn't\n> >> seem intentional; both remove the 'SKIP_WORKTREE' flag from the file, but\n> >> only 'checkout' creates the file on-disk (therefore avoiding the \"deleted\"\n> >> status).\n> >\n> > Restoring only into the index (as I think `git restore --staged` is supposed\n> > to do) is weird.\n>\n> 'git restore --staged' is intended to restore to both the worktree and index\n> (per 183fb44fd2 (restore: add --worktree and --staged, 2019-04-25)). The bug\n> you've identified is that it's not restoring to the worktree.\n\nI think that commit message is easy to mis-read. --staged means\nrestore to the index, and does not restore to the working tree unless\nthe user also specifies --worktree. Duy considered adding a\n`--no-worktree` option but decided against it.\n\n(In constrast, `checkout` always restores to both places and gives no\nway to do just one. Defaulting to just the worktree and letting the\nuser pick was one of the two big warts that `restore` fixed relative\nto `checkout` -- the other being the --no-overlay mode.)\n\n[...]\n> > If we now do `git restore --staged --source HEAD^` and that\n> > command doesn't remove the `SKIP_WORKTREE` flag on any paths, that logically\n> > means that we have modified the working copy, and I think `git\n> > sparse-checkout disable` would agree with me.\n>\n> If you aren't using '--ignore-skip-worktree-bits', the entries with\n> SKIP_WORKTREE shouldn't be touched in the first place. If you *do* specify\n> it, by virtue of restoring to the working tree, SKIP_WORKTREE must be\n> removed.\n>\n> But suppose you're doing something like 'git restore --staged --no-worktree\n> --ignore-skip-worktree-bits --source HEAD^'. In that case:\n\nThat'd be `git restore --staged --source=HEAD^\n--ignore-skip-worktree-bits -- `, but I think I understand the\npoint you're conveying...\n\n> - you are restoring to the index\n> - you are *not* restoring to the worktree\n> - you're restoring files with SKIP_WORKTREE applied\n\nminor nit: you may also be restoring files that do not exist in HEAD\nor the index. So, I'd rather say here \"you're restoring files outside\nthe sparse specification\" to handle those too.\n\n> Right now, SKIP_WORKTREE is removed from the matched entries, but suppose\n\nRight, any code outside of unpack-trees.c that tweaks index entries\ntends to replace existing ones with new ones without carefully setting\nor copying various flags (like SKIP_WORKTREE) over. I suspect we just\nhave another case of that here. sparse-checkouts are forcing us to\naudit all these paths...\n\n> (per your comment) it wasn't. That wouldn't mean that we've \"modified the\n> working copy\"; the working tree is defined with respect to the index, and if\n> the index entry says \"I don't care about the worktree\", then there are no\n> differences to reflect.\n>\n> This raises an interesting question about the current behavior, though: if\n> you restore a SKIP_WORKTREE entry with '--staged' and '--no-worktree',\n> should we remove SKIP_WORKTREE? I'd lean towards \"no\", but I'm interested to\n> hear other contributors' thoughts.\n\nMy current $0.02...\n\nI'd say that, without the --ignore-skip-worktree-bits override, we\nshould behave like `git-add` and `git-rm` and not operate on paths\noutside the sparse specification. If the paths the user specified\ndon't match anything in the sparse specification, we should throw an\nerror and tell the user of the flag to use to override.\n\nWith the override...I'd agree that we should only update the index and\nstill keep (or appropriately set) the SKIP_WORKTREE bits. Yes, it has\nsome weirdness (as Martin points out), but that's part of the reason\nof requiring the --scope=all override, much as we do with `git-add`\nand `git-rm`. (The override for add/rm is currently named `--sparse`\nso it's not quite the same, but we're planning on renaming all these\nso that'll make it clearer that it's the same situation for them all.)\n"},{"id":"464218","messageId":"CABPp-BH5_=Tq9DM6iAfG3+DuzEE7dR-H8rhP34x-A5FQhLO+bg@mail.gmail.com","threadId":"58554","inReplyTo":"CAESOdVByucFm=yJn2yL1mwKGqey7tHXH4A-JM-yP125Ok+_Q+g@mail.gmail.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-10-05T07:51:43Z","receivedAt":"2022-10-05T07:52:04Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Oct 4, 2022 at 9:53 PM Martin von Zweigbergk\n wrote:\n>\n> On Tue, Oct 4, 2022 at 1:35 PM Victoria Dye wrote:\n> >\n> > Martin von Zweigbergk wrote:\n> > >> On Tue, Oct 4, 2022 at 9:34 AM Victoria Dye > wrote:\n> > >>\n[...]\n> > >> Thanks for reporting this! There are a few confusing things going on with\n> > >> 'restore' here.\n> > >>\n> > >> First is that the out-of-cone was even restored in the first place.\n> > >>\n> > >\n> > > I was actually happy that the out-of-cone paths were restored. I ran that\n> > > command as an experiment while reading Elijah's doc because I was curious\n> > > what would happen. The reason I think it should restore out-of-cone paths is\n> > > so you can do `git restore --staged --source && git commit -m\n> > > \"restore to old commit\"` without caring about the sparse spec.\n> >\n> > Conversely, that's behavior a user *wouldn't* want if they want to keep\n> > their sparse cone intact (not to mention the performance impact of checking\n> > out the entire worktree). I think it does more harm to those users than it\n> > would benefit the ones that want to checkout out-of-cone files.\n> >\n> > The use-case you're describing should be served by the\n> > '--ignore-skip-worktree-bits' option (not the most intuitive name,\n> > unfortunately). Luckily, there's an increasing desire to improve the naming\n> > of sparse-related options, so the UX situation should improve in the future.\n>\n> I realized after sending my previous email that I might have a\n> different view of what\n> sparse checkout is about. To me, it seems like it should be just a performance\n> optimization. That's why I feel like commands should behave the same way with\n> or without a sparse spec (unless that proposed `--restrict` flag is passed). I\n> understand if that's just not feasible. Sorry about the noise in that case :)\n\nThe problem I see with that definition is that I'm not even sure what\nthat means. Behaving the same way with or without a sparse\nspecification at the extreme means that switching branches should\npopulate all the files in that branch...meaning a dense checkout.\nThat kind of defeats the point. So, I'm sure you don't mean that they\nbehave the same in all cases...but where do you draw the line?\n\nIs it just switch/checkout & `reset --hard` that avoid reading and\nwriting outside the sparse specification?\n\nShould diff & status ignore files outside the sparse specification\neven if users wrote to such files? A \"performance optimization\" might\nsuggest we should, but would users get confused?\n\nWhat about merge/rebase/cherry-pick/revert? Should those write\nadditional files to the working tree or avoid it? What about if there\nare conflicts outside the sparse specification?\n\nAnd if extra files get written outside the sparse specification, are\nthere ways for this to get \"cleaned up\" where after resolving\nconflicts or changes we can again remove the file from the working\ntree?\n\nWhat about `git grep PATTERN`? That's documented to search the\ntracked files in the working tree. But should that include the files\nthat would have been there were it not for the \"performance\noptimization\" of not checking them out? (Similarly, what about `git\ngrep --cached PATTERN` or `git grep PATTERN REVISION`?) I mean, if\nthese commands should behave the same regardless of sparse\nspecification, then you should search those other files, right? But\nisn't that a nasty performance penalty if the user has a\nsparse/partial clone since Git will have to do many network operations\nto get additional blobs in order to search them? Is that really\nwanted?\n\nWhat about `git rm '*.png'` to remove all the tracked png files from\nmy working tree. Should that also remove all the files that would\nhave been there were it not for the \"performance optimization\"? Will\nthat result in very negative surprises for those with a \"I want to\nconcentrate on just this subset of files\" mental model?\n\nWhat about `git worktree add`? Should the sparse specification be the\nsame for all worktrees? Be per-worktree? Should it default to dense?\n To just top-level sparse? To the same sparsity as the worktree you\nwere in when you created a new one?\n\nThere are more questions along these lines, but perhaps this gives a\nflavor of the kinds of questions that can come up which need\nunderlying user mental models and usecases to help us answer how these\ncommands should behave.\n\n> > But suppose you're doing something like 'git restore --staged --no-worktree\n> > --ignore-skip-worktree-bits --source HEAD^'. In that case:\n> >\n> > - you are restoring to the index\n> > - you are *not* restoring to the worktree\n> > - you're restoring files with SKIP_WORKTREE applied\n> >\n> > Right now, SKIP_WORKTREE is removed from the matched entries, but suppose\n> > (per your comment) it wasn't. That wouldn't mean that we've \"modified the\n> > working copy\"; the working tree is defined with respect to the index, and if\n> > the index entry says \"I don't care about the worktree\", then there are no\n> > differences to reflect.\n>\n> Yes, not technically changed. I was (and still am) thinking of the working copy\n> as logically containing all the files even if some of them are not written out\n> to disk.\n\nGit started with a nearly identical definition and still sadly\nincludes it (though buried deep in a plumbing manual that thankfully\nvirtually no one will ever read):\n\n Skip-worktree bit can be defined in one (long) sentence: When reading\n an entry, if it is marked as skip-worktree, then Git pretends its\n working directory version is up to date and read the index version\n instead.\n\nSuch a mental model is useless to me in answering how commands should\nbehave in any interesting cases, and actually led to several\ninconsistencies and bugs[1]. However, you didn't leave it there; you\ntook it a step further...\n\n[1] https://lore.kernel.org/git/CABPp-BGJ_Nvi5TmgriD9Bh6eNXE2EDq2f8e8QKXAeYG3BxZafA@mail.gmail.com/\n\n> I understand if that seems like an odd way of thinking about\n> it. It might\n> help to think about how it might appear in a virtual file system where clean\n> files outside the sparse spec are presented from the index. Or it may be better\n> to not try to think about it in this weird way at all :)\n\nNow you've provided a real usecase (virtual file system really\npretending that all files are present), which gives us some context\nwith which we can try to answer those questions I asked above. I\nthink some of the answers might be different for this usecase than the\n\"behavior A\" and \"behavior B\" in my current sparse-checkout.txt\ndocument, so maybe we want to add this usecase to the mix.\n\nAnyway, thanks for the interesting bug report, and the context of\nanother usecase to consider.\n"},{"id":"464243","messageId":"85def494-46c5-3785-8c78-31a733ab72e0@github.com","threadId":"58554","inReplyTo":"CAESOdVByucFm=yJn2yL1mwKGqey7tHXH4A-JM-yP125Ok+_Q+g@mail.gmail.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Victoria Dye","fromEmail":"vdye@github.com","sentAt":"2022-10-05T16:11:06Z","receivedAt":"2022-10-05T16:11:13Z","isPatch":false,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"Martin von Zweigbergk wrote:>>>> Theoretically, 'restore' (like 'checkout') should be limited to pathspecs\n>>>> inside the sparse-checkout patterns (per the documentation of\n>>>> '--ignore-skip-worktree-bits'), but 'Documentation' does not match them.\n>>>> Then, there's a difference between 'restore' and 'checkout' that doesn't\n>>>> seem intentional; both remove the 'SKIP_WORKTREE' flag from the file, but\n>>>> only 'checkout' creates the file on-disk (therefore avoiding the \"deleted\"\n>>>> status).\n>>>\n>>> Restoring only into the index (as I think `git restore --staged` is supposed\n>>> to do) is weird.\n>>\n>> 'git restore --staged' is intended to restore to both the worktree and index\n>> (per 183fb44fd2 (restore: add --worktree and --staged, 2019-04-25)). The bug\n>> you've identified is that it's not restoring to the worktree.\n> \n> Ah, `--worktree` is on by default even if I pass `--staged`, I see. Hmm, the\n> help text actually says \"Specifying --staged will only restore the index.\"...\n\nYou (and Elijah [1]) are correct. '--staged' overrides the \"checkout to\nworktree\" default behavior of 'git restore' to only restore to the index. If\nyou want to checkout to the worktree _and_ the index, 'git restore --staged\n--worktree' is what you'd use.\n\nSorry for the incorrect information! \n\n[1] https://lore.kernel.org/git/CABPp-BGeC3hXw-v3voniY5ZU2f6W8NXfXVvq0C03eGGhvSefgg@mail.gmail.com/\n"},{"id":"464258","messageId":"CAESOdVDt7SU=OJhF0mgyZ=B3sncB49aML8oOzKTKAnmGO5BaVQ@mail.gmail.com","threadId":"58554","inReplyTo":"CABPp-BH5_=Tq9DM6iAfG3+DuzEE7dR-H8rhP34x-A5FQhLO+bg@mail.gmail.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@google.com","sentAt":"2022-10-05T20:00:00Z","receivedAt":"2022-10-05T20:00:19Z","isPatch":false,"sender":{"key":"martinvonz@google.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Oct 5, 2022 at 12:51 AM Elijah Newren wrote:\n>\n> On Tue, Oct 4, 2022 at 9:53 PM Martin von Zweigbergk\n> wrote:\n> >\n> > On Tue, Oct 4, 2022 at 1:35 PM Victoria Dye wrote:\n> > >\n> > > Martin von Zweigbergk wrote:\n> > > >> On Tue, Oct 4, 2022 at 9:34 AM Victoria Dye > wrote:\n> > > >>\n> [...]\n> > > >> Thanks for reporting this! There are a few confusing things going on with\n> > > >> 'restore' here.\n> > > >>\n> > > >> First is that the out-of-cone was even restored in the first place.\n> > > >>\n> > > >\n> > > > I was actually happy that the out-of-cone paths were restored. I ran that\n> > > > command as an experiment while reading Elijah's doc because I was curious\n> > > > what would happen. The reason I think it should restore out-of-cone paths is\n> > > > so you can do `git restore --staged --source && git commit -m\n> > > > \"restore to old commit\"` without caring about the sparse spec.\n> > >\n> > > Conversely, that's behavior a user *wouldn't* want if they want to keep\n> > > their sparse cone intact (not to mention the performance impact of checking\n> > > out the entire worktree). I think it does more harm to those users than it\n> > > would benefit the ones that want to checkout out-of-cone files.\n> > >\n> > > The use-case you're describing should be served by the\n> > > '--ignore-skip-worktree-bits' option (not the most intuitive name,\n> > > unfortunately). Luckily, there's an increasing desire to improve the naming\n> > > of sparse-related options, so the UX situation should improve in the future.\n> >\n> > I realized after sending my previous email that I might have a\n> > different view of what\n> > sparse checkout is about. To me, it seems like it should be just a performance\n> > optimization. That's why I feel like commands should behave the same way with\n> > or without a sparse spec (unless that proposed `--restrict` flag is passed). I\n> > understand if that's just not feasible. Sorry about the noise in that case :)\n>\n> The problem I see with that definition is that I'm not even sure what\n> that means. Behaving the same way with or without a sparse\n> specification at the extreme means that switching branches should\n> populate all the files in that branch...meaning a dense checkout.\n> That kind of defeats the point. So, I'm sure you don't mean that they\n> behave the same in all cases...but where do you draw the line?\n\nI agree with you and Stolee that there are two different cases: some\npeople use sparse checkouts to restrict what they see (behavior A), and\nsome people use it just as a performance optimization (behavior B). So I\nsuspect we roughly agree about what should happen if you pass\n`--restrict` (or if that becomes the default so you don't actually need to\npass it). My arguments were about the `--no-restrict` case. Sorry, I\nshould have made that clear.\n\nI also agree that having a way to make commands restrict to certain paths\nby default is useful, and I agree that tying that set of paths to the current\nworktree's sparse spec makes sense.\n\nI'll answer the questions below for the `--no-restrict` case\n(behavior B).\n\n>\n> Is it just switch/checkout & `reset --hard` that avoid reading and\n> writing outside the sparse specification?\n>\n> Should diff & status ignore files outside the sparse specification\n> even if users wrote to such files? A \"performance optimization\" might\n> suggest we should, but would users get confused?\n\nI think they should be included (again, in the `--no-restrict` case).\n\n>\n> What about merge/rebase/cherry-pick/revert? Should those write\n> additional files to the working tree or avoid it? What about if there\n> are conflicts outside the sparse specification?\n\nI think they should avoid it, but since the user will need to resolve\nthat conflict anyway, I can see it makes sense to write them to disk\nif there are conflicts.\n\n>\n> And if extra files get written outside the sparse specification, are\n> there ways for this to get \"cleaned up\" where after resolving\n> conflicts or changes we can again remove the file from the working\n> tree?\n\nI've never really used `git sparse-checkout` (until I read your doc),\nbut isn't that what `git sparse-checkout reapply` is for?\n\n>\n> What about `git grep PATTERN`? That's documented to search the\n> tracked files in the working tree. But should that include the files\n> that would have been there were it not for the \"performance\n> optimization\" of not checking them out? (Similarly, what about `git\n> grep --cached PATTERN` or `git grep PATTERN REVISION`?) I mean, if\n> these commands should behave the same regardless of sparse\n> specification, then you should search those other files, right? But\n> isn't that a nasty performance penalty if the user has a\n> sparse/partial clone since Git will have to do many network operations\n> to get additional blobs in order to search them? Is that really\n> wanted?\n\nI think it's consistent to search them with `--no-restrict` (but not\nwith `--restrict`, of course).\n\n>\n> What about `git rm '*.png'` to remove all the tracked png files from\n> my working tree. Should that also remove all the files that would\n> have been there were it not for the \"performance optimization\"? Will\n> that result in very negative surprises for those with a \"I want to\n> concentrate on just this subset of files\" mental model?\n\nSame here.\n\n>\n> What about `git worktree add`? Should the sparse specification be the\n> same for all worktrees? Be per-worktree? Should it default to dense?\n> To just top-level sparse? To the same sparsity as the worktree you\n> were in when you created a new one?\n\nThat's an interesting case. If someone does `git worktree add` and expects\nall files to be available in the working copy, they might be surprised, yes.\nI think that's a much smaller risk than\n`git restore --source HEAD^ --staged && git commit -m 'undo changes'` being\npartial, however.\n"},{"id":"464272","messageId":"CABPp-BE12gaeWEWnqc589N+kJwqq796K5KJOHDiGduvOmQ36Gw@mail.gmail.com","threadId":"58554","inReplyTo":"CAESOdVDt7SU=OJhF0mgyZ=B3sncB49aML8oOzKTKAnmGO5BaVQ@mail.gmail.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-10-06T04:20:33Z","receivedAt":"2022-10-06T04:20:55Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Oct 5, 2022 at 1:00 PM Martin von Zweigbergk\n wrote:\n>\n> On Wed, Oct 5, 2022 at 12:51 AM Elijah Newren wrote:\n> >\n[...]\n> I agree with you and Stolee that there are two different cases: some\n> people use sparse checkouts to restrict what they see (behavior A), and\n> some people use it just as a performance optimization (behavior B). So I\n> suspect we roughly agree about what should happen if you pass\n> `--restrict` (or if that becomes the default so you don't actually need to\n> pass it). My arguments were about the `--no-restrict` case. Sorry, I\n> should have made that clear.\n>\n> I also agree that having a way to make commands restrict to certain paths\n> by default is useful, and I agree that tying that set of paths to the current\n> worktree's sparse spec makes sense.\n>\n> I'll answer the questions below for the `--no-restrict` case\n> (behavior B).\n\nI don't think your usecase matches behavior B. I think we should\nlabel your VFS usecase as behavior C and define it separately. More\non that below...\n\n[...]\n> > What about merge/rebase/cherry-pick/revert? Should those write\n> > additional files to the working tree or avoid it? What about if there\n> > are conflicts outside the sparse specification?\n>\n> I think they should avoid it, but since the user will need to resolve\n> that conflict anyway, I can see it makes sense to write them to disk\n> if there are conflicts.\n>\n> >\n> > And if extra files get written outside the sparse specification, are\n> > there ways for this to get \"cleaned up\" where after resolving\n> > conflicts or changes we can again remove the file from the working\n> > tree?\n>\n> I've never really used `git sparse-checkout` (until I read your doc),\n> but isn't that what `git sparse-checkout reapply` is for?\n\nWhile that command is available for users that want to manually clean\nthings up proactively, my suspicion is that it is used very rarely --\nespecially now that we have the present-despite-skipped class of\nissues fixed. I suspect nearly all cleaning up is actually done as an\nimplicit side-effect of calls to unpack_trees(), which would affect\ncommands such `switch`, the switch-like portion of `checkout`, `reset\n--hard`, `merge`, `rebase`, and many others.\n\nAll of these commands have two types of implicit clean-up they do as\npart of their operation (which could be thought of as a\npost-processing step): (1) marking *unmodified* files outside the\nsparsity patterns as SKIP_WORKTREE in the index and removing them from\nthe working tree, and (2) taking files which match the sparsity\npatterns which were previously SKIP_WORKTREE and flip them to\n!SKIP_WORKTREE and restore them to the working tree. I've got a few\nexamples of what this clean up looks like over at:\nhttps://lore.kernel.org/git/CABPp-BHGrxLPu_S3y2zG-U6uo0rM5TYYEREZa2A=e=d9VZb2PA@mail.gmail.com/\n\nI have no idea how this cleanup affects the VFS usecase; it's very\nfocused towards \"sparse checkout means many files should NOT be\npresent in the working tree\" which may be at odds with how the VFS\nstuff is intended to behave. But it's also been part of\nsparse-checkout behavior the longest; for well over a decade now.\n\n> > What about `git grep PATTERN`? That's documented to search the\n> > tracked files in the working tree. But should that include the files\n> > that would have been there were it not for the \"performance\n> > optimization\" of not checking them out? (Similarly, what about `git\n> > grep --cached PATTERN` or `git grep PATTERN REVISION`?) I mean, if\n> > these commands should behave the same regardless of sparse\n> > specification, then you should search those other files, right? But\n> > isn't that a nasty performance penalty if the user has a\n> > sparse/partial clone since Git will have to do many network operations\n> > to get additional blobs in order to search them? Is that really\n> > wanted?\n>\n> I think it's consistent to search them with `--no-restrict` (but not\n> with `--restrict`, of course).\n>\n> > What about `git rm '*.png'` to remove all the tracked png files from\n> > my working tree. Should that also remove all the files that would\n> > have been there were it not for the \"performance optimization\"? Will\n> > that result in very negative surprises for those with a \"I want to\n> > concentrate on just this subset of files\" mental model?\n>\n> Same here.\n>\n> >\n> > What about `git worktree add`? Should the sparse specification be the\n> > same for all worktrees? Be per-worktree? Should it default to dense?\n> > To just top-level sparse? To the same sparsity as the worktree you\n> > were in when you created a new one?\n>\n> That's an interesting case. If someone does `git worktree add` and expects\n> all files to be available in the working copy, they might be surprised, yes.\n> I think that's a much smaller risk than\n> `git restore --source HEAD^ --staged && git commit -m 'undo changes'` being\n> partial, however.\n\nAfter you described the VFS usecase, I was guessing you'd answer how\nyou did for most of these commands. Most of your answers do not match\nthe answers I'd expect for behavior B, which seems to me to support my\nsuspicion that you've got a third usecase.\n\nIn particular, I think the difference between Behavior B and your\nusecase hinges on the expectation for the working tree:\n Behavior B: Files outside the sparse specification are NOT present\nin the working tree.\n Behavior C (your usecase): Files outside the sparse specification\nARE \"present\" in the working tree, but Git doesn't have to put them\nthere (they'll be lazily put into place by something else, and the VFS\nwill ensure that users don't ever notice them actually missing, so far\nall intents and purposes, the files are present).\n\nIn particular, that difference is perhaps most notable with `git grep`\n(without --cached or REVISION flags); such a command is supposed to\nsearch the worktree. For Behavior B, files outside the sparse\nspecification are NOT present in the working tree, and hence those\nfiles should NOT be searched. For your usecase, as you highlight\nabove, you view all files as present in the working tree (even if Git\nisn't the thing writing those files to the working tree and even if\nthey aren't technically present until you query whether they are\nthere), so all those files SHOULD be searched.\n\nThis difference about the \"presence\" of files has other knock-on\neffects too. Under Behavior B, users get used to working on just a\nsubset of files. Thus `git rm '*.jpg'` or `git restore --source HEAD^\n-- '*.md'` should NOT overwrite files outside the sparse specification\n(but an error should be shown if the pathspec doesn't match anything,\nand that error should point out how users can affect other files\noutside the sparse specification). Under your usecase, users are\nalways working on the full set of files and all of them can be viewed\nin their working copy (as enforced by the filesystem intercepting any\nattempts to view or edit files and suddenly magically materializing\nthem when users look) -- so users in your usecase are not expecting to\nbe working on a subset of files, and thus those commands would operate\ntree-wide.\n\nSimilarly, under Behavior B, `git add outside/of/cone/path` should\nthrow an error. If it doesn't, some future command will silently\nremove the file from the working copy, which may confuse the user;\nthey are getting themselves into an erroneous state. Users are\npointed to an override flag they can use if they want to accept the\nconsequences. Under your usecase, since ALL files are always\n\"present\" (but not materialized until an attempt to access is made),\nthat same command would be expected to run without an override and\nwith no error or warning.\n\nRelated to the above, under Behavior B, `git status` should probably\nreport the existence of any untracked files that do not match the\nsparsity patterns as an erroneous condition (because why wait until\ngit-add to throw an error; let the user know early). Under your\nusecase, we wouldn't.\n\n\nYou might think I'm describing Behavior A above, but only because\nBehavior A and Behavior B overlap on worktree-related operations. The\nprimary difference between Behavior A and Behavior B is with behavior\nof history-related operations. Behavior B says \"the working tree is\nsparse, but history is dense; if I do a query on older revisions of\nhistory (grep/diff/log/etc.) then give me results across all paths\",\nwhereas Behavior A says \"I only care about this subset of files, both\nfor the working tree and for history. Unless I override,\ngrep/diff/etc. on history should restrict all output to files within\nthe sparse specification.\n\nOne potential way to (over)simplify this would be:\n Behavior A: `--scope=sparse` for both worktree and history operations\n Behavior B: `--scope=sparse` for worktree operations,\n`--scope=all` for history operations\n Behavior C: `--scope=all` for both worktree and history operations\n"},{"id":"464320","messageId":"xmqqv8owkaww.fsf@gitster.g","threadId":"58554","inReplyTo":"96c4f52e-bc66-f4ee-f4f6-d22da579858e@github.com","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-06T19:30:07Z","receivedAt":"2022-10-06T19:30:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Victoria Dye writes:\n\n>> Restoring only into the index (as I think `git restore --staged` is supposed\n>> to do) is weird. \n>\n> 'git restore --staged' is intended to restore to both the worktree and index\n> (per 183fb44fd2 (restore: add --worktree and --staged, 2019-04-25)). The bug\n> you've identified is that it's not restoring to the worktree.\n\nI think you misread 183fb44fd2, which says --staged is to restore\nthe contents in the index and --worktree is to restore the contents\nin the working tree, and both of them can be used at the same time\nto affect both destinations. As gitcli(7) says, --staged here is a\nsynonym to --cached here and should not touch the working tree.\n\nHaving needless synonym may be a source of confusion, so we may want\nto straighten out the UI a bit around here, but that is a separate\ntopic.\n\nThanks.\n"},{"id":"464321","messageId":"xmqqr0zkkaiw.fsf@gitster.g","threadId":"58554","inReplyTo":"xmqqv8owkaww.fsf@gitster.g","subject":"Re: Bug report: `git restore --source --staged` deals poorly with sparse-checkout","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-06T19:38:31Z","receivedAt":"2022-10-06T19:38:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I think you misread 183fb44fd2, ...\n\nAh, now I see this was already resolved yesterday while I was\noffline. Sorry for the noise.\n"}]}