git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] worktree add: be tolerant of corrupt worktrees

From
SHShaheed Haque <shaheedhaque@gmail.com>
Date
Jun 1, 2019, 19:29 UTC
Message-ID
<CAHAc2jdXDcOT61xSDcmSMrOxDDhEn+WeuP2OqKQ4=2wR58OCww@mail.gmail.com>
In-Reply-To
<CACsJy8BBtSo8ikwG4sMRYbM5=L4Ck-Cgioea6XGKt7-nVq_Ogg@mail.gmail.com>
On Sat, 18 May 2019 at 12:50, Duy Nguyen <pclouds@gmail.com> wrote:
Show 36 quoted lines
>
> On Fri, May 17, 2019 at 2:46 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> >
> > On Mon, May 13, 2019 at 6:50 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> > > find_worktree() can die() unexpectedly because it uses real_path()
> > > instead of the gentler version. When it's used in 'git worktree add' [1]
> > > and there's a bad worktree, this die() could prevent people from adding
> > > new worktrees.
> >
> > This is good to know because, to fix [1], I think we'll want to add a
> > new function[2] akin to find_worktree(), but without magic suffix
> > matching (that is, just literal absolute path comparison).
>
> Yeah. find_worktree() was made to handle command line options from
> worktree's move/remove, it's probably a bit too magical for this case.
>
> I still want to store relative path in "gitdir" files at some point,
> which would complicate the last "absolute path comparison" part a bit.
> But it should be manageable.
>
> > [1]: https://public-inbox.org/git/0308570E-AAA3-43B8-A592-F4DA9760DBED@synopsys.com/
> > [2]: https://public-inbox.org/git/CAPig+cQh8hxeoVjLHDKhAcZVQPpPT5v0AUY8gsL9=qfJ7z-L2A@mail.gmail.com/
> >
> > > The "bad" condition to trigger this is when a parent of the worktree's
> > > location is deleted. Then real_path() will complain.
> > >
> > > Use the other version so that bad worktrees won't affect 'worktree
> > > add'. The bad ones will eventually be pruned, we just have to tolerate
> > > them for a bit.
> >
> > The patch itself makes sense, though, as Shaheed noted in his
> > response, pruning seems to get short-circuited somehow under this
> > situation; perhaps that needs its own fix, but certainly shouldn't
> > hold up this fix.
>
> I might have missed that detail. Thanks for pointing out. Will get another look.

On the off-chance this is not obvious to you experts, 'worktree remove' also hits this issue, as you can see from this little example I just caught:

$ git worktree remove --force /tmp/tmp56t59s2k/git_worktree.pa44kgs0 fatal: Invalid path '/tmp/tmpy4f98pwj': No such file or directory $ mkdir /tmp/tmpy4f98pwj $ git worktree remove --force /tmp/tmp56t59s2k/git_worktree.pa44kgs0 fatal: Invalid path '/tmp/tmp_q3p2mon': No such file or directory $ mkdir /tmp/tmp_q3p2mon ... several more suppressed ... $ git worktree remove --force /tmp/tmp56t59s2k/git_worktree.pa44kgs0 fatal: '/tmp/tmp56t59s2k/git_worktree.pa44kgs0' is not a working tree

Thanks, Shaheed
>
> --
> Duy
Previous: Duy Nguyen
Message 10 of 10 in “"add worktree" fails with "fatal: Invalid path" error”
  1. Shaheed HaqueMay 12, 2019
  2. Duy NguyenMay 13, 2019
  3. Shaheed HaqueMay 13, 2019
  4. Duy NguyenMay 14, 2019
  5. Shaheed HaqueMay 14, 2019
  6. worktree add: be tolerant of corrupt worktreesNguyễn Thái Ngọc Duy, May 13, 2019
  7. Shaheed HaqueMay 13, 2019
  8. Eric SunshineMay 17, 2019
  9. Duy NguyenMay 18, 2019
  10. Shaheed HaqueJun 1, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.