{"thread":{"id":"51076","subject":"\"add worktree\" fails with \"fatal: Invalid path\" error","startedAt":"2019-05-12T10:13:03Z","lastAt":"2019-06-01T19:29:49Z","messageCount":10,"participants":["Shaheed Haque","Duy Nguyen","Nguyễn Thái Ngọc Duy","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"375352","messageId":"CAHAc2je-Yz4oej-sqvp+G+2Wv+eBABeJWUMm4scRwF2z_diUXw@mail.gmail.com","threadId":"51076","inReplyTo":null,"subject":"\"add worktree\" fails with \"fatal: Invalid path\" error","fromName":"Shaheed Haque","fromEmail":"shaheedhaque@gmail.com","sentAt":"2019-05-12T10:12:50Z","receivedAt":"2019-05-12T10:13:03Z","isPatch":false,"sender":{"key":"shaheedhaque@gmail.com","avatar":null},"body":"Hi,\n\nI'm running git v.2.20.1 on Ubuntu from a program which follows the pattern:\n\n============\n1. create a temporary directory /tmp/tmpabc\n2. in a loop:\n    2a. create a second level of temporary directory /tmp/tmpabc/tmpworktree123\n    2b. use \"git worktree add\" on the second level directory\n    2c. do something\n3. cleanup\n    3b. \"git branch -D\" on each basename(second level directory)\n    3a. \"git worktree prune\"\n============\n\nThe loop size is of the order of 8-20. In step 2b, I often get errors\nlike this (from a Bash reproducer):\n\n============\n$ git worktree add /tmp/tmpgtxug4y9/git_worktree.gBGqnfnU\nPreparing worktree (new branch 'git_worktree.gBGqnfnU')\nfatal: Invalid path '/tmp/tmp1q9ysvyl': No such file or directory\n============\n\nI can see that the problematic path exists in the \"gitdir\" file of\nwhat must be an earlier worktree from an older run (the branch is\ngone, but the tree is still there). The path appear to relate to the\nolder run's first level directory:\n\n============\n$ grep -r /tmp/tmp1q9ysvyl ../.git/worktrees/\n../.git/worktrees/git_worktree.frcwtjt_/gitdir:/tmp/tmp1q9ysvyl/git_worktree.frcwtjt_/.git\n$ git worktree list\n...\n/tmp/tmp1q9ysvyl/git_worktree.frcwtjt_  edde3f25 (detached HEAD)\n...\n$ git branch | grep frcwtjt_\n<no matches>\n============\n\nNOTE: I've not yet had to try deleting the worktree, since \"add\nworktree\" does appear to work some of the time, so I am able to limp\nalong.\n\nI have these questions:\n\n1. There is no branch or first level directory, but \"git prune\" has\nnot deleted the worktree, is this expected?\n2. Is there something wrong with the sequence of steps I am following?\n\nThanks, Shaheed\n\nP.S. I have an strace of a failing worktree add if needed.\n"},{"id":"375372","messageId":"CACsJy8C++Ds4kfs_Wc8UiVQgni-ypbyJ+0bFg1m5brt+s0Tfig@mail.gmail.com","threadId":"51076","inReplyTo":"CAHAc2je-Yz4oej-sqvp+G+2Wv+eBABeJWUMm4scRwF2z_diUXw@mail.gmail.com","subject":"Re: \"add worktree\" fails with \"fatal: Invalid path\" error","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-13T09:20:42Z","receivedAt":"2019-05-13T09:21:10Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, May 12, 2019 at 5:14 PM Shaheed Haque <shaheedhaque@gmail.com> wrote:\n>\n> Hi,\n>\n> I'm running git v.2.20.1 on Ubuntu from a program which follows the pattern:\n>\n> ============\n> 1. create a temporary directory /tmp/tmpabc\n\nWhen is this directory deleted? After step 3a?\n\n> 2. in a loop:\n>     2a. create a second level of temporary directory /tmp/tmpabc/tmpworktree123\n>     2b. use \"git worktree add\" on the second level directory\n>     2c. do something\n> 3. cleanup\n>     3b. \"git branch -D\" on each basename(second level directory)\n>     3a. \"git worktree prune\"\n> ============\n>\n> The loop size is of the order of 8-20. In step 2b, I often get errors\n> like this (from a Bash reproducer):\n>\n> ============\n> $ git worktree add /tmp/tmpgtxug4y9/git_worktree.gBGqnfnU\n> Preparing worktree (new branch 'git_worktree.gBGqnfnU')\n> fatal: Invalid path '/tmp/tmp1q9ysvyl': No such file or directory\n> ============\n>\n> I can see that the problematic path exists in the \"gitdir\" file of\n> what must be an earlier worktree from an older run (the branch is\n> gone, but the tree is still there). The path appear to relate to the\n> older run's first level directory:\n>\n> ============\n> $ grep -r /tmp/tmp1q9ysvyl ../.git/worktrees/\n> ../.git/worktrees/git_worktree.frcwtjt_/gitdir:/tmp/tmp1q9ysvyl/git_worktree.frcwtjt_/.git\n> $ git worktree list\n> ...\n> /tmp/tmp1q9ysvyl/git_worktree.frcwtjt_  edde3f25 (detached HEAD)\n> ...\n> $ git branch | grep frcwtjt_\n> <no matches>\n> ============\n\nYeah I think I know where that \"Invalid path\" comes from and it should\nnot be there (at least it should not be a fatal error). I'll need to\nreproduce this first. But I'm certain you've given me enough\ninformation to do so.\n\n> NOTE: I've not yet had to try deleting the worktree, since \"add\n> worktree\" does appear to work some of the time, so I am able to limp\n> along.\n\nIt's probably best to stay clean and delete things after you're done.\nAt least you should be able to avoid this problem this way until it's\nfixed.\n\n>\n> I have these questions:\n>\n> 1. There is no branch or first level directory, but \"git prune\" has\n> not deleted the worktree, is this expected?\n\nI assume you meant \"git worktree prune\", not \"git prune\". See\ngc.worktreePruneExpire. Dead worktree info stays for a while until\nit's deleted, so that you can recover stuff if you need to.\n\n> 2. Is there something wrong with the sequence of steps I am following?\n\nNope. I mean, you could try \"git worktree remove\" to be on the safe\nside. But it should work even without that. To me this looks very much\nlike a bug.\n\n> Thanks, Shaheed\n>\n> P.S. I have an strace of a failing worktree add if needed.\n-- \nDuy\n"},{"id":"375375","messageId":"20190513104944.20367-1-pclouds@gmail.com","threadId":"51076","inReplyTo":"CAHAc2je-Yz4oej-sqvp+G+2Wv+eBABeJWUMm4scRwF2z_diUXw@mail.gmail.com","subject":"[PATCH] worktree add: be tolerant of corrupt worktrees","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-13T10:49:44Z","receivedAt":"2019-05-13T10:50:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"find_worktree() can die() unexpectedly because it uses real_path()\ninstead of the gentler version. When it's used in 'git worktree add' [1]\nand there's a bad worktree, this die() could prevent people from adding\nnew worktrees.\n\nThe \"bad\" condition to trigger this is when a parent of the worktree's\nlocation is deleted. Then real_path() will complain.\n\nUse the other version so that bad worktrees won't affect 'worktree\nadd'. The bad ones will eventually be pruned, we just have to tolerate\nthem for a bit.\n\n[1] added in cb56f55c16 (worktree: disallow adding same path multiple\n    times, 2018-08-28), or since v2.20.0. Though the real bug in\n    find_worktree() is much older.\n\nReported-by: Shaheed Haque <shaheedhaque@gmail.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n t/t2025-worktree-add.sh | 12 ++++++++++++\n worktree.c              |  7 +++++--\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\nindex 286bba35d8..d83a9f0fdc 100755\n--- a/t/t2025-worktree-add.sh\n+++ b/t/t2025-worktree-add.sh\n@@ -570,4 +570,16 @@ test_expect_success '\"add\" an existing locked but missing worktree' '\n \tgit worktree add --force --force --detach gnoo\n '\n \n+test_expect_success '\"add\" should not fail because of another bad worktree' '\n+\tgit init add-fail &&\n+\t(\n+\t\tcd add-fail &&\n+\t\ttest_commit first &&\n+\t\tmkdir sub &&\n+\t\tgit worktree add sub/to-be-deleted &&\n+\t\trm -rf sub &&\n+\t\tgit worktree add second\n+\t)\n+'\n+\n test_done\ndiff --git a/worktree.c b/worktree.c\nindex d6a0ee7f73..c79b3e42bb 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -222,9 +222,12 @@ struct worktree *find_worktree(struct worktree **list,\n \t\tfree(to_free);\n \t\treturn NULL;\n \t}\n-\tfor (; *list; list++)\n-\t\tif (!fspathcmp(path, real_path((*list)->path)))\n+\tfor (; *list; list++) {\n+\t\tconst char *wt_path = real_path_if_valid((*list)->path);\n+\n+\t\tif (wt_path && !fspathcmp(path, wt_path))\n \t\t\tbreak;\n+\t}\n \tfree(path);\n \tfree(to_free);\n \treturn *list;\n-- \n2.21.0.1141.gd54ac2cb17\n\n"},{"id":"375381","messageId":"CAHAc2jeFva3MLpuXEiBbwa7U5HuZiaqawkc3udsyPCaFR4FAnA@mail.gmail.com","threadId":"51076","inReplyTo":"20190513104944.20367-1-pclouds@gmail.com","subject":"Re: [PATCH] worktree add: be tolerant of corrupt worktrees","fromName":"Shaheed Haque","fromEmail":"shaheedhaque@gmail.com","sentAt":"2019-05-13T12:42:46Z","receivedAt":"2019-05-13T12:42:58Z","isPatch":true,"sender":{"key":"shaheedhaque@gmail.com","avatar":null},"body":"Hi Nguyễn,\n\nThanks for the quick response. While I leave the code to the experts,\nI can confirm that restoring the missing directory (but no content in\nit) does allow \"worktree add\" to function again.\n\nOne point may be worth clarifying...\n\nOn Mon, 13 May 2019 at 11:50, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n>\n> find_worktree() can die() unexpectedly because it uses real_path()\n> instead of the gentler version. When it's used in 'git worktree add' [1]\n> and there's a bad worktree, this die() could prevent people from adding\n> new worktrees.\n>\n> The \"bad\" condition to trigger this is when a parent of the worktree's\n> location is deleted. Then real_path() will complain.\n>\n> Use the other version so that bad worktrees won't affect 'worktree\n> add'. The bad ones will eventually be pruned, we just have to tolerate\n> them for a bit.\n\n...as I mentioned, from my experiments, trying a \"worktree prune\" did\nNOT resolve the issue for me. But since I don't know the logic that\nprune uses, there may have been some other reason for this.\n\nThanks again, Shaheed\n\n> [1] added in cb56f55c16 (worktree: disallow adding same path multiple\n>     times, 2018-08-28), or since v2.20.0. Though the real bug in\n>     find_worktree() is much older.\n>\n> Reported-by: Shaheed Haque <shaheedhaque@gmail.com>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  t/t2025-worktree-add.sh | 12 ++++++++++++\n>  worktree.c              |  7 +++++--\n>  2 files changed, 17 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh\n> index 286bba35d8..d83a9f0fdc 100755\n> --- a/t/t2025-worktree-add.sh\n> +++ b/t/t2025-worktree-add.sh\n> @@ -570,4 +570,16 @@ test_expect_success '\"add\" an existing locked but missing worktree' '\n>         git worktree add --force --force --detach gnoo\n>  '\n>\n> +test_expect_success '\"add\" should not fail because of another bad worktree' '\n> +       git init add-fail &&\n> +       (\n> +               cd add-fail &&\n> +               test_commit first &&\n> +               mkdir sub &&\n> +               git worktree add sub/to-be-deleted &&\n> +               rm -rf sub &&\n> +               git worktree add second\n> +       )\n> +'\n> +\n>  test_done\n> diff --git a/worktree.c b/worktree.c\n> index d6a0ee7f73..c79b3e42bb 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -222,9 +222,12 @@ struct worktree *find_worktree(struct worktree **list,\n>                 free(to_free);\n>                 return NULL;\n>         }\n> -       for (; *list; list++)\n> -               if (!fspathcmp(path, real_path((*list)->path)))\n> +       for (; *list; list++) {\n> +               const char *wt_path = real_path_if_valid((*list)->path);\n> +\n> +               if (wt_path && !fspathcmp(path, wt_path))\n>                         break;\n> +       }\n>         free(path);\n>         free(to_free);\n>         return *list;\n> --\n> 2.21.0.1141.gd54ac2cb17\n>\n"},{"id":"375383","messageId":"CAHAc2jf2Ojve=NaEshXx9qk8rtD4NHxqLEpqZq8c9t0yE4m_Qw@mail.gmail.com","threadId":"51076","inReplyTo":"CACsJy8C++Ds4kfs_Wc8UiVQgni-ypbyJ+0bFg1m5brt+s0Tfig@mail.gmail.com","subject":"Re: \"add worktree\" fails with \"fatal: Invalid path\" error","fromName":"Shaheed Haque","fromEmail":"shaheedhaque@gmail.com","sentAt":"2019-05-13T12:55:23Z","receivedAt":"2019-05-13T12:55:36Z","isPatch":false,"sender":{"key":"shaheedhaque@gmail.com","avatar":null},"body":"Hi Duy,\n\nOn Mon, 13 May 2019 at 10:21, Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> On Sun, May 12, 2019 at 5:14 PM Shaheed Haque <shaheedhaque@gmail.com> wrote:\n> >\n> > Hi,\n> >\n> > I'm running git v.2.20.1 on Ubuntu from a program which follows the pattern:\n> >\n> > ============\n> > 1. create a temporary directory /tmp/tmpabc\n>\n> When is this directory deleted? After step 3a?\n\nYes. I note that I screwed up the numbering in my note - the order is\n[\"git branch -D\", \"git worktree prune\", delete directory].\n\n> > 2. in a loop:\n> >     2a. create a second level of temporary directory /tmp/tmpabc/tmpworktree123\n> >     2b. use \"git worktree add\" on the second level directory\n> >     2c. do something\n> > 3. cleanup\n> >     3b. \"git branch -D\" on each basename(second level directory)\n> >     3a. \"git worktree prune\"\n> > ============\n> >\n> > The loop size is of the order of 8-20. In step 2b, I often get errors\n> > like this (from a Bash reproducer):\n> >\n> > ============\n> > $ git worktree add /tmp/tmpgtxug4y9/git_worktree.gBGqnfnU\n> > Preparing worktree (new branch 'git_worktree.gBGqnfnU')\n> > fatal: Invalid path '/tmp/tmp1q9ysvyl': No such file or directory\n> > ============\n> >\n> > I can see that the problematic path exists in the \"gitdir\" file of\n> > what must be an earlier worktree from an older run (the branch is\n> > gone, but the tree is still there). The path appear to relate to the\n> > older run's first level directory:\n> >\n> > ============\n> > $ grep -r /tmp/tmp1q9ysvyl ../.git/worktrees/\n> > ../.git/worktrees/git_worktree.frcwtjt_/gitdir:/tmp/tmp1q9ysvyl/git_worktree.frcwtjt_/.git\n> > $ git worktree list\n> > ...\n> > /tmp/tmp1q9ysvyl/git_worktree.frcwtjt_  edde3f25 (detached HEAD)\n> > ...\n> > $ git branch | grep frcwtjt_\n> > <no matches>\n> > ============\n>\n> Yeah I think I know where that \"Invalid path\" comes from and it should\n> not be there (at least it should not be a fatal error). I'll need to\n> reproduce this first. But I'm certain you've given me enough\n> information to do so.\n>\n> > NOTE: I've not yet had to try deleting the worktree, since \"add\n> > worktree\" does appear to work some of the time, so I am able to limp\n> > along.\n>\n> It's probably best to stay clean and delete things after you're done.\n> At least you should be able to avoid this problem this way until it's\n> fixed.\n\nAck.\n\n> > I have these questions:\n> >\n> > 1. There is no branch or first level directory, but \"git prune\" has\n> > not deleted the worktree, is this expected?\n>\n> I assume you meant \"git worktree prune\", not \"git prune\". See\n> gc.worktreePruneExpire. Dead worktree info stays for a while until\n> it's deleted, so that you can recover stuff if you need to.\n\nYes, sorry for typo.\n\n> > 2. Is there something wrong with the sequence of steps I am following?\n>\n> Nope. I mean, you could try \"git worktree remove\" to be on the safe\n> side. But it should work even without that. To me this looks very much\n> like a bug.\n\nThe original code used the more obvious \"git worktree remove\" rather\nthan \"git worktree prune\" but I switched partly because remove seemed\nslow (I cannot now quantify what caused me to think that), and partly\nbecause I was having other issues which, I now realise, you probably\naddressed in your recent \"stat versus mkdir race\" change.\n\nBTW: I *love* worktrees. I have used/wrapped/developed multiple source\ncontrol systems over the years, and IMHO, this is one of the killer\nfeatures of git.\n\nThanks, Shaheed\n\n> > Thanks, Shaheed\n> >\n> > P.S. I have an strace of a failing worktree add if needed.\n> --\n> Duy\n"},{"id":"375526","messageId":"CACsJy8DnmQxO+r3ybg2zpCSMZJaTwc_C8V3QMCDjvga09sBigw@mail.gmail.com","threadId":"51076","inReplyTo":"CAHAc2jf2Ojve=NaEshXx9qk8rtD4NHxqLEpqZq8c9t0yE4m_Qw@mail.gmail.com","subject":"Re: \"add worktree\" fails with \"fatal: Invalid path\" error","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-14T12:33:39Z","receivedAt":"2019-05-14T12:34:07Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, May 13, 2019 at 7:55 PM Shaheed Haque <shaheedhaque@gmail.com> wrote:\n> The original code used the more obvious \"git worktree remove\" rather\n> than \"git worktree prune\" but I switched partly because remove seemed\n> slow (I cannot now quantify what caused me to think that), and partly\n> because I was having other issues which, I now realise, you probably\n> addressed in your recent \"stat versus mkdir race\" change.\n\nIt should be as slow as \"git status; rm -r\". The first command _could_\nbe slow. But if you find it significantly slower than that, I will be\nglad to receive another bug report.\n-- \nDuy\n"},{"id":"375541","messageId":"CAHAc2jfZ7QvZ_PTZo5q8orSStZL2y8GJ=ZPe2-+hcydjEZ0=Ew@mail.gmail.com","threadId":"51076","inReplyTo":"CACsJy8DnmQxO+r3ybg2zpCSMZJaTwc_C8V3QMCDjvga09sBigw@mail.gmail.com","subject":"Re: \"add worktree\" fails with \"fatal: Invalid path\" error","fromName":"Shaheed Haque","fromEmail":"shaheedhaque@gmail.com","sentAt":"2019-05-14T14:48:08Z","receivedAt":"2019-05-14T14:48:41Z","isPatch":false,"sender":{"key":"shaheedhaque@gmail.com","avatar":null},"body":"On Tue, 14 May 2019 at 13:34, Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> On Mon, May 13, 2019 at 7:55 PM Shaheed Haque <shaheedhaque@gmail.com> wrote:\n> > The original code used the more obvious \"git worktree remove\" rather\n> > than \"git worktree prune\" but I switched partly because remove seemed\n> > slow (I cannot now quantify what caused me to think that), and partly\n> > because I was having other issues which, I now realise, you probably\n> > addressed in your recent \"stat versus mkdir race\" change.\n>\n> It should be as slow as \"git status; rm -r\". The first command _could_\n> be slow. But if you find it significantly slower than that, I will be\n> glad to receive another bug report.\n\nAfter I wrote, I went back and checked, and I have no idea why I\nthought it slow. It seems just fine (by the time I have spawned out of\nPython and all), and so I have switched the code back to\n(synchronous/predictable) \"git worktree remove\".\n\nThanks, Shaheed\n\n> --\n> Duy\n"},{"id":"375793","messageId":"CAPig+cRNDmHD7JrvJL8yvo0r_3HSNdVuF79uYt7fG4iaBpCeCQ@mail.gmail.com","threadId":"51076","inReplyTo":"20190513104944.20367-1-pclouds@gmail.com","subject":"Re: [PATCH] worktree add: be tolerant of corrupt worktrees","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-17T07:46:26Z","receivedAt":"2019-05-17T07:46:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, May 13, 2019 at 6:50 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> find_worktree() can die() unexpectedly because it uses real_path()\n> instead of the gentler version. When it's used in 'git worktree add' [1]\n> and there's a bad worktree, this die() could prevent people from adding\n> new worktrees.\n\nThis is good to know because, to fix [1], I think we'll want to add a\nnew function[2] akin to find_worktree(), but without magic suffix\nmatching (that is, just literal absolute path comparison).\n\n[1]: https://public-inbox.org/git/0308570E-AAA3-43B8-A592-F4DA9760DBED@synopsys.com/\n[2]: https://public-inbox.org/git/CAPig+cQh8hxeoVjLHDKhAcZVQPpPT5v0AUY8gsL9=qfJ7z-L2A@mail.gmail.com/\n\n> The \"bad\" condition to trigger this is when a parent of the worktree's\n> location is deleted. Then real_path() will complain.\n>\n> Use the other version so that bad worktrees won't affect 'worktree\n> add'. The bad ones will eventually be pruned, we just have to tolerate\n> them for a bit.\n\nThe patch itself makes sense, though, as Shaheed noted in his\nresponse, pruning seems to get short-circuited somehow under this\nsituation; perhaps that needs its own fix, but certainly shouldn't\nhold up this fix.\n\n> Reported-by: Shaheed Haque <shaheedhaque@gmail.com>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n"},{"id":"375849","messageId":"CACsJy8BBtSo8ikwG4sMRYbM5=L4Ck-Cgioea6XGKt7-nVq_Ogg@mail.gmail.com","threadId":"51076","inReplyTo":"CAPig+cRNDmHD7JrvJL8yvo0r_3HSNdVuF79uYt7fG4iaBpCeCQ@mail.gmail.com","subject":"Re: [PATCH] worktree add: be tolerant of corrupt worktrees","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-05-18T11:49:38Z","receivedAt":"2019-05-18T11:50:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, May 17, 2019 at 2:46 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Mon, May 13, 2019 at 6:50 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> > find_worktree() can die() unexpectedly because it uses real_path()\n> > instead of the gentler version. When it's used in 'git worktree add' [1]\n> > and there's a bad worktree, this die() could prevent people from adding\n> > new worktrees.\n>\n> This is good to know because, to fix [1], I think we'll want to add a\n> new function[2] akin to find_worktree(), but without magic suffix\n> matching (that is, just literal absolute path comparison).\n\nYeah. find_worktree() was made to handle command line options from\nworktree's move/remove, it's probably a bit too magical for this case.\n\nI still want to store relative path in \"gitdir\" files at some point,\nwhich would complicate the last \"absolute path comparison\" part a bit.\nBut it should be manageable.\n\n> [1]: https://public-inbox.org/git/0308570E-AAA3-43B8-A592-F4DA9760DBED@synopsys.com/\n> [2]: https://public-inbox.org/git/CAPig+cQh8hxeoVjLHDKhAcZVQPpPT5v0AUY8gsL9=qfJ7z-L2A@mail.gmail.com/\n>\n> > The \"bad\" condition to trigger this is when a parent of the worktree's\n> > location is deleted. Then real_path() will complain.\n> >\n> > Use the other version so that bad worktrees won't affect 'worktree\n> > add'. The bad ones will eventually be pruned, we just have to tolerate\n> > them for a bit.\n>\n> The patch itself makes sense, though, as Shaheed noted in his\n> response, pruning seems to get short-circuited somehow under this\n> situation; perhaps that needs its own fix, but certainly shouldn't\n> hold up this fix.\n\nI might have missed that detail. Thanks for pointing out. Will get another look.\n-- \nDuy\n"},{"id":"376533","messageId":"CAHAc2jdXDcOT61xSDcmSMrOxDDhEn+WeuP2OqKQ4=2wR58OCww@mail.gmail.com","threadId":"51076","inReplyTo":"CACsJy8BBtSo8ikwG4sMRYbM5=L4Ck-Cgioea6XGKt7-nVq_Ogg@mail.gmail.com","subject":"Re: [PATCH] worktree add: be tolerant of corrupt worktrees","fromName":"Shaheed Haque","fromEmail":"shaheedhaque@gmail.com","sentAt":"2019-06-01T19:29:36Z","receivedAt":"2019-06-01T19:29:49Z","isPatch":true,"sender":{"key":"shaheedhaque@gmail.com","avatar":null},"body":"On Sat, 18 May 2019 at 12:50, Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> On Fri, May 17, 2019 at 2:46 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> >\n> > On Mon, May 13, 2019 at 6:50 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> > > find_worktree() can die() unexpectedly because it uses real_path()\n> > > instead of the gentler version. When it's used in 'git worktree add' [1]\n> > > and there's a bad worktree, this die() could prevent people from adding\n> > > new worktrees.\n> >\n> > This is good to know because, to fix [1], I think we'll want to add a\n> > new function[2] akin to find_worktree(), but without magic suffix\n> > matching (that is, just literal absolute path comparison).\n>\n> Yeah. find_worktree() was made to handle command line options from\n> worktree's move/remove, it's probably a bit too magical for this case.\n>\n> I still want to store relative path in \"gitdir\" files at some point,\n> which would complicate the last \"absolute path comparison\" part a bit.\n> But it should be manageable.\n>\n> > [1]: https://public-inbox.org/git/0308570E-AAA3-43B8-A592-F4DA9760DBED@synopsys.com/\n> > [2]: https://public-inbox.org/git/CAPig+cQh8hxeoVjLHDKhAcZVQPpPT5v0AUY8gsL9=qfJ7z-L2A@mail.gmail.com/\n> >\n> > > The \"bad\" condition to trigger this is when a parent of the worktree's\n> > > location is deleted. Then real_path() will complain.\n> > >\n> > > Use the other version so that bad worktrees won't affect 'worktree\n> > > add'. The bad ones will eventually be pruned, we just have to tolerate\n> > > them for a bit.\n> >\n> > The patch itself makes sense, though, as Shaheed noted in his\n> > response, pruning seems to get short-circuited somehow under this\n> > situation; perhaps that needs its own fix, but certainly shouldn't\n> > hold up this fix.\n>\n> I might have missed that detail. Thanks for pointing out. Will get another look.\n\nOn the off-chance this is not obvious to you experts, 'worktree\nremove' also hits this issue, as you can see from this little example\nI just caught:\n\n$ git worktree remove --force /tmp/tmp56t59s2k/git_worktree.pa44kgs0\nfatal: Invalid path '/tmp/tmpy4f98pwj': No such file or directory\n$ mkdir /tmp/tmpy4f98pwj\n$ git worktree remove --force /tmp/tmp56t59s2k/git_worktree.pa44kgs0\nfatal: Invalid path '/tmp/tmp_q3p2mon': No such file or directory\n$ mkdir /tmp/tmp_q3p2mon\n...\nseveral more suppressed\n...\n$ git worktree remove --force /tmp/tmp56t59s2k/git_worktree.pa44kgs0\nfatal: '/tmp/tmp56t59s2k/git_worktree.pa44kgs0' is not a working tree\n\nThanks, Shaheed\n\n>\n> --\n> Duy\n"}]}