{"thread":{"id":"52675","subject":"Sparse Checkout Trouble (2.5.0)","startedAt":"2020-01-22T00:42:05Z","lastAt":"2020-01-29T16:31:44Z","messageCount":11,"participants":["JunkYardMail1","Derrick Stolee","Elijah Newren","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"390217","messageId":"062301d5d0bc$c3e17760$4ba46620$@Frontier.com","threadId":"52675","inReplyTo":null,"subject":"Sparse Checkout Trouble (2.5.0)","fromName":"JunkYardMail1","fromEmail":"junkyardmail1@frontier.com","sentAt":"2020-01-22T00:42:02Z","receivedAt":"2020-01-22T00:42:05Z","isPatch":false,"sender":{"key":"junkyardmail1@frontier.com","avatar":null},"body":"I have a shallow repository clone and using sparse-checkout of just a\nhandful of directories.  When I upgraded from git version 2.24.1 to 2.25.0\nsome files not in the sparse-checkout were staged to be deleted.  The\ndirectory path of these files contain the Windows reserved name of \"prn\".\nEx: \"japanese/prn/. . .\"  Unable to un-stage these files and reset the\nchanges.\n\n.git/inof/Sparse-checkout:\n!/*/\n/*/directory-*/\n\nReverting back to the previous version (2.24.1) allowed to un-stage the\nfiles and reset the changes.\n\n\n"},{"id":"390218","messageId":"d4056731-c13b-c89d-dfd2-e6235e29fed6@gmail.com","threadId":"52675","inReplyTo":"062301d5d0bc$c3e17760$4ba46620$@Frontier.com","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-22T01:19:22Z","receivedAt":"2020-01-22T01:19:28Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/21/2020 7:42 PM, JunkYardMail1 wrote:\n> I have a shallow repository clone and using sparse-checkout of just a\n> handful of directories.  When I upgraded from git version 2.24.1 to 2.25.0\n> some files not in the sparse-checkout were staged to be deleted.  The\n> directory path of these files contain the Windows reserved name of \"prn\".\n> Ex: \"japanese/prn/. . .\"  Unable to un-stage these files and reset the\n> changes.\n\nAre you using Git for Windows? I'm not sure why the \"prn\" would be important\notherwise.\n\n> !/*/\n> /*/directory-*/\n\nI don't see anything complicated with these patterns, although they seem\na bit strange. Why would you not want anything in a directory except one\nnamed \"<something>/directory-<anotherthing>/\"? Is this actually the exact\npattern set?\n \n> Reverting back to the previous version (2.24.1) allowed to un-stage the\n> files and reset the changes.\n\nThis is definitely pointing to a regression, and the feature did get a\nbit of an overhaul. The goal was to not change how existing users\ninteracted with it, though, so this is a bit alarming.\n\nI did find a behavior change related to these paths in Git for Windows\n2.25.0 versus 2.24.1:\n\n  $ git clone https://github.com/derrickstolee/git-sparse-checkout-test\n  Cloning into 'git-sparse-checkout-test'...\n  remote: Enumerating objects: 6, done.\n  remote: Counting objects: 100% (6/6), done.\n  remote: Compressing objects: 100% (4/4), done.\n  remote: Total 6 (delta 0), reused 6 (delta 0), pack-reused 0\n  Receiving objects: 100% (6/6), done.\n  error: invalid path 'directory-1/prn/a'\n  fatal: unable to checkout working tree\n  warning: Clone succeeded, but checkout failed.\n  You can inspect what was checked out with 'git status'\n  and retry with 'git restore --source=HEAD :/'\n  \nThe checkout here is _expected_ to fail, due to the protected\nfilenames. What is troublesome is the following sparse-checkout\ncommands fail, and the read-tree command fails in a way that\n2.24.1 does not:\n \n  $ git sparse-checkout init\n  error: invalid path 'directory-1/prn/a'\n  error: invalid path 'directory-1/prn/a'\n  \n  $ git sparse-checkout set \"/*\" \"\\!/*/\"\n  error: invalid path 'directory-1/prn/a'\n  error: invalid path 'directory-1/prn/a'\n  \n  $ git read-tree -mu HEAD\n  error: invalid path 'directory-1/prn/a'\n\nThe double error messages are due to the \"try with an in-memory\npattern set, then roll back if there is a failure.\" The patterns\nwe are trying to create do not include the directory that is failing.\n\nI'll keep looking into this.\n\nThanks,\n-Stolee\n"},{"id":"390219","messageId":"062f01d5d0c8$9ec6c140$dc5443c0$@Frontier.com","threadId":"52675","inReplyTo":"d4056731-c13b-c89d-dfd2-e6235e29fed6@gmail.com","subject":"RE: Sparse Checkout Trouble (2.5.0)","fromName":"JunkYardMail1","fromEmail":"junkyardmail1@frontier.com","sentAt":"2020-01-22T02:06:54Z","receivedAt":"2020-01-22T02:06:58Z","isPatch":false,"sender":{"key":"junkyardmail1@frontier.com","avatar":null},"body":"Yes.  Using git for Windows with bash terminal and Git Extensions GUI.\n\nThe reason for the sparse checkout pattern is that the repo is very large, approximately a gigabyte, and I only need of few sub-directories that amount to only about 60 megabytes.\nThe first sparse checkout rule \"!/*/\" is expected to exclude everything.\nThe second sparse checkout rule \"/*/directory-*/\" is expected to include only sub-directories that contain \"directory-\" as the leading part of a directory element.\n\nLooks like you're able to reproduce the issue and are hot on the trail.  Thanks.\n\n\n-----Original Message-----\nFrom: Derrick Stolee [mailto:stolee@gmail.com] \nSent: Tuesday, January 21, 2020 5:19 PM\nTo: JunkYardMail1 <JunkYardMail1@Frontier.com>; git@vger.kernel.org\nSubject: Re: Sparse Checkout Trouble (2.5.0)\n\nOn 1/21/2020 7:42 PM, JunkYardMail1 wrote:\n> I have a shallow repository clone and using sparse-checkout of just a \n> handful of directories.  When I upgraded from git version 2.24.1 to \n> 2.25.0 some files not in the sparse-checkout were staged to be \n> deleted.  The directory path of these files contain the Windows reserved name of \"prn\".\n> Ex: \"japanese/prn/. . .\"  Unable to un-stage these files and reset the \n> changes.\n\nAre you using Git for Windows? I'm not sure why the \"prn\" would be important otherwise.\n\n> !/*/\n> /*/directory-*/\n\nI don't see anything complicated with these patterns, although they seem a bit strange. Why would you not want anything in a directory except one named \"<something>/directory-<anotherthing>/\"? Is this actually the exact pattern set?\n \n> Reverting back to the previous version (2.24.1) allowed to un-stage \n> the files and reset the changes.\n\nThis is definitely pointing to a regression, and the feature did get a bit of an overhaul. The goal was to not change how existing users interacted with it, though, so this is a bit alarming.\n\nI did find a behavior change related to these paths in Git for Windows\n2.25.0 versus 2.24.1:\n\n  $ git clone https://github.com/derrickstolee/git-sparse-checkout-test\n  Cloning into 'git-sparse-checkout-test'...\n  remote: Enumerating objects: 6, done.\n  remote: Counting objects: 100% (6/6), done.\n  remote: Compressing objects: 100% (4/4), done.\n  remote: Total 6 (delta 0), reused 6 (delta 0), pack-reused 0\n  Receiving objects: 100% (6/6), done.\n  error: invalid path 'directory-1/prn/a'\n  fatal: unable to checkout working tree\n  warning: Clone succeeded, but checkout failed.\n  You can inspect what was checked out with 'git status'\n  and retry with 'git restore --source=HEAD :/'\n  \nThe checkout here is _expected_ to fail, due to the protected filenames. What is troublesome is the following sparse-checkout commands fail, and the read-tree command fails in a way that\n2.24.1 does not:\n \n  $ git sparse-checkout init\n  error: invalid path 'directory-1/prn/a'\n  error: invalid path 'directory-1/prn/a'\n  \n  $ git sparse-checkout set \"/*\" \"\\!/*/\"\n  error: invalid path 'directory-1/prn/a'\n  error: invalid path 'directory-1/prn/a'\n  \n  $ git read-tree -mu HEAD\n  error: invalid path 'directory-1/prn/a'\n\nThe double error messages are due to the \"try with an in-memory pattern set, then roll back if there is a failure.\" The patterns we are trying to create do not include the directory that is failing.\n\nI'll keep looking into this.\n\nThanks,\n-Stolee\n\n"},{"id":"390365","messageId":"405d83bd-cd50-49c5-a068-6d3ce102f669@gmail.com","threadId":"52675","inReplyTo":"d4056731-c13b-c89d-dfd2-e6235e29fed6@gmail.com","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-24T15:40:57Z","receivedAt":"2020-01-24T15:41:02Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/21/2020 8:19 PM, Derrick Stolee wrote:\n> On 1/21/2020 7:42 PM, JunkYardMail1 wrote:\n>> Reverting back to the previous version (2.24.1) allowed to un-stage the\n>> files and reset the changes.\n> \n> This is definitely pointing to a regression, and the feature did get a\n> bit of an overhaul. The goal was to not change how existing users\n> interacted with it, though, so this is a bit alarming.\n> \n> I did find a behavior change related to these paths in Git for Windows\n> 2.25.0 versus 2.24.1:\n> \n>   $ git clone https://github.com/derrickstolee/git-sparse-checkout-test\n>   Cloning into 'git-sparse-checkout-test'...\n>   remote: Enumerating objects: 6, done.\n>   remote: Counting objects: 100% (6/6), done.\n>   remote: Compressing objects: 100% (4/4), done.\n>   remote: Total 6 (delta 0), reused 6 (delta 0), pack-reused 0\n>   Receiving objects: 100% (6/6), done.\n>   error: invalid path 'directory-1/prn/a'\n>   fatal: unable to checkout working tree\n>   warning: Clone succeeded, but checkout failed.\n>   You can inspect what was checked out with 'git status'\n>   and retry with 'git restore --source=HEAD :/'\n>   \n> The checkout here is _expected_ to fail, due to the protected\n> filenames. What is troublesome is the following sparse-checkout\n> commands fail, and the read-tree command fails in a way that\n> 2.24.1 does not:\n>  \n>   $ git sparse-checkout init\n>   error: invalid path 'directory-1/prn/a'\n>   error: invalid path 'directory-1/prn/a'\n>   \n>   $ git sparse-checkout set \"/*\" \"\\!/*/\"\n>   error: invalid path 'directory-1/prn/a'\n>   error: invalid path 'directory-1/prn/a'\n>   \n>   $ git read-tree -mu HEAD\n>   error: invalid path 'directory-1/prn/a'\n> \n> The double error messages are due to the \"try with an in-memory\n> pattern set, then roll back if there is a failure.\" The patterns\n> we are trying to create do not include the directory that is failing.\n> \n> I'll keep looking into this.\n\nI am still struggling to find out exactly what goes wrong here.\n\nI'm CC'ing Elijah because he also made changes to dir.c, and\nperhaps he has some idea of what's going on.\n\nUsing GIT_TRACE2_PERF=1, I can see that the error appears during\nthe traverse_trees() which is after the sparse-checkout patterns\nhave applied the skip-worktree bit. \n\nThanks,\n-Stolee\n"},{"id":"390469","messageId":"CABPp-BFB3pN1arWb9Acj7La1aX7j=axANnSWV8f7SmW_KuqD0g@mail.gmail.com","threadId":"52675","inReplyTo":"405d83bd-cd50-49c5-a068-6d3ce102f669@gmail.com","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-01-25T20:59:25Z","receivedAt":"2020-01-25T20:59:39Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jan 24, 2020 at 7:41 AM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> On 1/21/2020 8:19 PM, Derrick Stolee wrote:\n> > On 1/21/2020 7:42 PM, JunkYardMail1 wrote:\n> >> Reverting back to the previous version (2.24.1) allowed to un-stage the\n> >> files and reset the changes.\n> >\n> > This is definitely pointing to a regression, and the feature did get a\n> > bit of an overhaul. The goal was to not change how existing users\n> > interacted with it, though, so this is a bit alarming.\n> >\n> > I did find a behavior change related to these paths in Git for Windows\n> > 2.25.0 versus 2.24.1:\n> >\n> >   $ git clone https://github.com/derrickstolee/git-sparse-checkout-test\n> >   Cloning into 'git-sparse-checkout-test'...\n> >   remote: Enumerating objects: 6, done.\n> >   remote: Counting objects: 100% (6/6), done.\n> >   remote: Compressing objects: 100% (4/4), done.\n> >   remote: Total 6 (delta 0), reused 6 (delta 0), pack-reused 0\n> >   Receiving objects: 100% (6/6), done.\n> >   error: invalid path 'directory-1/prn/a'\n> >   fatal: unable to checkout working tree\n> >   warning: Clone succeeded, but checkout failed.\n> >   You can inspect what was checked out with 'git status'\n> >   and retry with 'git restore --source=HEAD :/'\n> >\n> > The checkout here is _expected_ to fail, due to the protected\n> > filenames. What is troublesome is the following sparse-checkout\n> > commands fail, and the read-tree command fails in a way that\n> > 2.24.1 does not:\n> >\n> >   $ git sparse-checkout init\n> >   error: invalid path 'directory-1/prn/a'\n> >   error: invalid path 'directory-1/prn/a'\n> >\n> >   $ git sparse-checkout set \"/*\" \"\\!/*/\"\n> >   error: invalid path 'directory-1/prn/a'\n> >   error: invalid path 'directory-1/prn/a'\n> >\n> >   $ git read-tree -mu HEAD\n> >   error: invalid path 'directory-1/prn/a'\n> >\n> > The double error messages are due to the \"try with an in-memory\n> > pattern set, then roll back if there is a failure.\" The patterns\n> > we are trying to create do not include the directory that is failing.\n> >\n> > I'll keep looking into this.\n>\n> I am still struggling to find out exactly what goes wrong here.\n>\n> I'm CC'ing Elijah because he also made changes to dir.c, and\n> perhaps he has some idea of what's going on.\n\nIf you think it might be related to the dir.c changes, I can take a\nlook.  I don't have any immediate ideas off the top of my head.\nHowever, since I'm really suffering with \"git read-tree -mu HEAD\"\nbeing the mechanism for updating sparsity (for reasons independent of\nthe issue being discussed here), I've been tempted to dig into that\nanyway to write a replacement without the nasty side-effects.  I'll\ntake a look early next week and see if I can spot anything.\n\n> Using GIT_TRACE2_PERF=1, I can see that the error appears during\n> the traverse_trees() which is after the sparse-checkout patterns\n> have applied the skip-worktree bit.\n>\n> Thanks,\n> -Stolee\n"},{"id":"390512","messageId":"de300697-baba-62a6-ea4c-c5bd472954c2@gmail.com","threadId":"52675","inReplyTo":"CABPp-BFB3pN1arWb9Acj7La1aX7j=axANnSWV8f7SmW_KuqD0g@mail.gmail.com","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-26T14:09:38Z","receivedAt":"2020-01-26T14:09:47Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/25/2020 3:59 PM, Elijah Newren wrote:\n> On Fri, Jan 24, 2020 at 7:41 AM Derrick Stolee <stolee@gmail.com> wrote:\n>> I'm CC'ing Elijah because he also made changes to dir.c, and\n>> perhaps he has some idea of what's going on.\n> \n> If you think it might be related to the dir.c changes, I can take a\n> look.  I don't have any immediate ideas off the top of my head.\n\nThe only thing I can think of is that these paths are already\nmarked as sparse, but something is requiring us to test if the\npath can be created with the filesystem. I'll try to debug\nmore into exactly where that is. It's telling that this happens\nboth in cone mode and without.\n\n> However, since I'm really suffering with \"git read-tree -mu HEAD\"\n> being the mechanism for updating sparsity (for reasons independent of\n> the issue being discussed here), I've been tempted to dig into that\n> anyway to write a replacement without the nasty side-effects.  I'll\n> take a look early next week and see if I can spot anything.\n\nIf by \"nasty side-effects\" you mean \"overwrites staged changes, even\nif in the sparse set before and after\" then I would welcome such a\nchange. Otherwise, it will fall to me, and this is far outside of\nmy expertise. Of course, it is something I should learn, but I can\nlearn that during code review, too. ;)\n\nThanks,\n-Stolee\n"},{"id":"390611","messageId":"CABPp-BHRAA71LmfYb61N+qAgwhtii7TSEVJksiAGnnO-4bk2dQ@mail.gmail.com","threadId":"52675","inReplyTo":"de300697-baba-62a6-ea4c-c5bd472954c2@gmail.com","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-01-28T05:21:41Z","receivedAt":"2020-01-28T05:21:56Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sun, Jan 26, 2020 at 6:09 AM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> On 1/25/2020 3:59 PM, Elijah Newren wrote:\n> > On Fri, Jan 24, 2020 at 7:41 AM Derrick Stolee <stolee@gmail.com> wrote:\n> >> I'm CC'ing Elijah because he also made changes to dir.c, and\n> >> perhaps he has some idea of what's going on.\n> >\n> > If you think it might be related to the dir.c changes, I can take a\n> > look.  I don't have any immediate ideas off the top of my head.\n>\n> The only thing I can think of is that these paths are already\n> marked as sparse, but something is requiring us to test if the\n> path can be created with the filesystem. I'll try to debug\n> more into exactly where that is. It's telling that this happens\n> both in cone mode and without.\n\nYeah, I'll take a look.  The exponentially slow 'status --ignored'\nreport is forcing me to look at dir.c again anyway, though it's also\ndelaying me from getting a chance to look at this particular report.\n\n> > However, since I'm really suffering with \"git read-tree -mu HEAD\"\n> > being the mechanism for updating sparsity (for reasons independent of\n> > the issue being discussed here), I've been tempted to dig into that\n> > anyway to write a replacement without the nasty side-effects.  I'll\n> > take a look early next week and see if I can spot anything.\n>\n> If by \"nasty side-effects\" you mean \"overwrites staged changes, even\n> if in the sparse set before and after\" then I would welcome such a\n> change. Otherwise, it will fall to me, and this is far outside of\n> my expertise. Of course, it is something I should learn, but I can\n> learn that during code review, too. ;)\n\nYep, that's exactly what I mean.  It'd be nice if Duy were still\naround to bug, but I've touched unpack-trees a few times so I might be\nable to find my way around.  I'll try to take a stab at it.\n\n> Thanks,\n> -Stolee\n"},{"id":"390665","messageId":"e4bb20e9-ad90-3e9b-7ed2-c7793cb8cb22@gmail.com","threadId":"52675","inReplyTo":"CABPp-BHRAA71LmfYb61N+qAgwhtii7TSEVJksiAGnnO-4bk2dQ@mail.gmail.com","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-28T20:47:51Z","receivedAt":"2020-01-28T20:47:56Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/28/2020 12:21 AM, Elijah Newren wrote:\n> On Sun, Jan 26, 2020 at 6:09 AM Derrick Stolee <stolee@gmail.com> wrote:\n>>\n>> On 1/25/2020 3:59 PM, Elijah Newren wrote:\n>>> On Fri, Jan 24, 2020 at 7:41 AM Derrick Stolee <stolee@gmail.com> wrote:\n>>>> I'm CC'ing Elijah because he also made changes to dir.c, and\n>>>> perhaps he has some idea of what's going on.\n>>>\n>>> If you think it might be related to the dir.c changes, I can take a\n>>> look.  I don't have any immediate ideas off the top of my head.\n>>\n>> The only thing I can think of is that these paths are already\n>> marked as sparse, but something is requiring us to test if the\n>> path can be created with the filesystem. I'll try to debug\n>> more into exactly where that is. It's telling that this happens\n>> both in cone mode and without.\n> \n> Yeah, I'll take a look.  The exponentially slow 'status --ignored'\n> report is forcing me to look at dir.c again anyway, though it's also\n> delaying me from getting a chance to look at this particular report.\n\nI made some progress, at least, in root-causing the issue.\nThe problem bisects down to 4dc42c6c1 (mingw: refuse paths\ncontaining reserved names, 2019-12-21) [1]. CC'ing Dscho.\n\nThat commit updates is_valid_win32_path() to fail on these\npaths. We were _already_ calling this method even for paths\noutside the sparse cone, but the method didn't fail for these\nexamples.\n\nThis means the fix is probably even more complicated: we need\nto not call this method when traversing paths that have the\nskip-worktree bit enabled. This may lead to some tiny\nperformance gains when hydrating a very small fraction of a\nvery large index.\n\nThanks,\n-Stolee\n\n[1] http://github.com/git/git/commit/4dc42c6c1867a52e22f1f04a1a247b5a7538b8af\n"},{"id":"390728","messageId":"nycvar.QRO.7.76.6.2001291626550.46@tvgsbejvaqbjf.bet","threadId":"52675","inReplyTo":"e4bb20e9-ad90-3e9b-7ed2-c7793cb8cb22@gmail.com","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-01-29T15:30:35Z","receivedAt":"2020-01-29T15:30:44Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 28 Jan 2020, Derrick Stolee wrote:\n\n> On 1/28/2020 12:21 AM, Elijah Newren wrote:\n> > On Sun, Jan 26, 2020 at 6:09 AM Derrick Stolee <stolee@gmail.com> wrote:\n> >>\n> >> On 1/25/2020 3:59 PM, Elijah Newren wrote:\n> >>> On Fri, Jan 24, 2020 at 7:41 AM Derrick Stolee <stolee@gmail.com> wrote:\n> >>>> I'm CC'ing Elijah because he also made changes to dir.c, and\n> >>>> perhaps he has some idea of what's going on.\n> >>>\n> >>> If you think it might be related to the dir.c changes, I can take a\n> >>> look.  I don't have any immediate ideas off the top of my head.\n> >>\n> >> The only thing I can think of is that these paths are already\n> >> marked as sparse, but something is requiring us to test if the\n> >> path can be created with the filesystem. I'll try to debug\n> >> more into exactly where that is. It's telling that this happens\n> >> both in cone mode and without.\n> >\n> > Yeah, I'll take a look.  The exponentially slow 'status --ignored'\n> > report is forcing me to look at dir.c again anyway, though it's also\n> > delaying me from getting a chance to look at this particular report.\n>\n> I made some progress, at least, in root-causing the issue.\n> The problem bisects down to 4dc42c6c1 (mingw: refuse paths\n> containing reserved names, 2019-12-21) [1]. CC'ing Dscho.\n>\n> That commit updates is_valid_win32_path() to fail on these\n> paths. We were _already_ calling this method even for paths\n> outside the sparse cone, but the method didn't fail for these\n> examples.\n>\n> This means the fix is probably even more complicated: we need\n> to not call this method when traversing paths that have the\n> skip-worktree bit enabled. This may lead to some tiny\n> performance gains when hydrating a very small fraction of a\n> very large index.\n\nHmm. I am actually not sure that this would be a fix. It is all too easy\nfor a skip-worktree entry to become checked out (think e.g. a merge\nconflict in a cherry-pick, during a three-way merge of a file that is not\nin the cone but still needs to be handled).\n\nMy preferred solution for this issue would be for the files/directories to\nbe renamed using `git -c core.protectntfs=false mv <reserved-name>\n<non-reserved-name>`.\n\nI think if we try to play extra games with the skip-worktree bit, we risk\nopening a vulnerability again.\n\nCiao,\nDscho\n\n>\n> Thanks,\n> -Stolee\n>\n> [1] http://github.com/git/git/commit/4dc42c6c1867a52e22f1f04a1a247b5a7538b8af\n>\n"},{"id":"390729","messageId":"60b34188-d05d-6753-7116-a34793592227@gmail.com","threadId":"52675","inReplyTo":"nycvar.QRO.7.76.6.2001291626550.46@tvgsbejvaqbjf.bet","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-29T15:40:51Z","receivedAt":"2020-01-29T15:40:54Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/29/2020 10:30 AM, Johannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 28 Jan 2020, Derrick Stolee wrote:\n>> I made some progress, at least, in root-causing the issue.\n>> The problem bisects down to 4dc42c6c1 (mingw: refuse paths\n>> containing reserved names, 2019-12-21) [1]. CC'ing Dscho.\n>>\n>> That commit updates is_valid_win32_path() to fail on these\n>> paths. We were _already_ calling this method even for paths\n>> outside the sparse cone, but the method didn't fail for these\n>> examples.\n>>\n>> This means the fix is probably even more complicated: we need\n>> to not call this method when traversing paths that have the\n>> skip-worktree bit enabled. This may lead to some tiny\n>> performance gains when hydrating a very small fraction of a\n>> very large index.\n> \n> Hmm. I am actually not sure that this would be a fix. It is all too easy\n> for a skip-worktree entry to become checked out (think e.g. a merge\n> conflict in a cherry-pick, during a three-way merge of a file that is not\n> in the cone but still needs to be handled).\n\nThis is a very good point, and something worth investigating.\n\n> My preferred solution for this issue would be for the files/directories to\n> be renamed using `git -c core.protectntfs=false mv <reserved-name>\n> <non-reserved-name>`.\n\nOne thing that I realized after root-causing the issue is that now the\nLinux kernel repository cannot be checked out _at all_ on Windows due to\nthe existence of an aux.c file. Git complains that the path is invalid\nand does not write a single file to the working directory.\n\nAt least we _could_ allow someone to create most of the working directory\n(as we did before) by allowing invalid paths outside the sparse cone.\n \n> I think if we try to play extra games with the skip-worktree bit, we risk\n> opening a vulnerability again.\n\nI agree that we need to be _very_ careful with this.\n\nThanks,\n-Stolee\n"},{"id":"390730","messageId":"a180c7a5-2406-0384-9355-be9db05a16fd@gmail.com","threadId":"52675","inReplyTo":"60b34188-d05d-6753-7116-a34793592227@gmail.com","subject":"Re: Sparse Checkout Trouble (2.5.0)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-29T16:31:40Z","receivedAt":"2020-01-29T16:31:44Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/29/2020 10:40 AM, Derrick Stolee wrote:\n> On 1/29/2020 10:30 AM, Johannes Schindelin wrote:\n>> Hi,\n>>\n>> On Tue, 28 Jan 2020, Derrick Stolee wrote:\n>>> I made some progress, at least, in root-causing the issue.\n>>> The problem bisects down to 4dc42c6c1 (mingw: refuse paths\n>>> containing reserved names, 2019-12-21) [1]. CC'ing Dscho.\n>>>\n>>> That commit updates is_valid_win32_path() to fail on these\n>>> paths. We were _already_ calling this method even for paths\n>>> outside the sparse cone, but the method didn't fail for these\n>>> examples.\n>>>\n>>> This means the fix is probably even more complicated: we need\n>>> to not call this method when traversing paths that have the\n>>> skip-worktree bit enabled. This may lead to some tiny\n>>> performance gains when hydrating a very small fraction of a\n>>> very large index.\n\nAfter digging into this more, at the place where we call\nverify_path() and create this warning, all cache entries have\nthe same ce_flags  (0x0209_0000):\n\n* 0x0200_0000: CE_NEW_SKIP_WORKTREE\n* 0x0008_0000: CE_ADDED\n* 0x0001_0000: CE_UPDATE\n\nSo, at this point there is no way to differentiate between\npaths that will be written to disk or paths that will not.\nThat decision appears to be made at another time, and connecting\nthese decisions at a distance is ripe for unintended behavior.\n\n>> My preferred solution for this issue would be for the files/directories to\n>> be renamed using `git -c core.protectntfs=false mv <reserved-name>\n>> <non-reserved-name>`.\n> \n> One thing that I realized after root-causing the issue is that now the\n> Linux kernel repository cannot be checked out _at all_ on Windows due to\n> the existence of an aux.c file. Git complains that the path is invalid\n> and does not write a single file to the working directory.\n>\n> At least we _could_ allow someone to create most of the working directory\n> (as we did before) by allowing invalid paths outside the sparse cone.\n\nOf course, as you say, setting core.protectntfs=false allows the Linux\nkernel to be checked out as before, but without the safety valve.\n  \n>> I think if we try to play extra games with the skip-worktree bit, we risk\n>> opening a vulnerability again.\n> \n> I agree that we need to be _very_ careful with this.\n\nAfter my investigation, and your workaround, I'm content to settle\nthis situation \"as designed\" and to leave it at that.\n\nThanks,\n-Stolee\n"}]}