git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Shaheed Haque <shaheedhaque@gmail.com>
To: Nguyễn Thái Ngọc Duy  <pclouds@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH] worktree add: be tolerant of corrupt worktrees
Date: Mon, 13 May 2019 13:42:46 +0100
Message-ID: <CAHAc2jeFva3MLpuXEiBbwa7U5HuZiaqawkc3udsyPCaFR4FAnA@mail.gmail.com> (raw)
In-Reply-To: <20190513104944.20367-1-pclouds@gmail.com>

Hi Nguyễn,

Thanks for the quick response. While I leave the code to the experts,
I can confirm that restoring the missing directory (but no content in
it) does allow "worktree add" to function again.

One point may be worth clarifying...

On Mon, 13 May 2019 at 11:50, 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.
>
> 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.

...as I mentioned, from my experiments, trying a "worktree prune" did
NOT resolve the issue for me. But since I don't know the logic that
prune uses, there may have been some other reason for this.

Thanks again, Shaheed

> [1] added in cb56f55c16 (worktree: disallow adding same path multiple
>     times, 2018-08-28), or since v2.20.0. Though the real bug in
>     find_worktree() is much older.
>
> Reported-by: Shaheed Haque <shaheedhaque@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  t/t2025-worktree-add.sh | 12 ++++++++++++
>  worktree.c              |  7 +++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> index 286bba35d8..d83a9f0fdc 100755
> --- a/t/t2025-worktree-add.sh
> +++ b/t/t2025-worktree-add.sh
> @@ -570,4 +570,16 @@ test_expect_success '"add" an existing locked but missing worktree' '
>         git worktree add --force --force --detach gnoo
>  '
>
> +test_expect_success '"add" should not fail because of another bad worktree' '
> +       git init add-fail &&
> +       (
> +               cd add-fail &&
> +               test_commit first &&
> +               mkdir sub &&
> +               git worktree add sub/to-be-deleted &&
> +               rm -rf sub &&
> +               git worktree add second
> +       )
> +'
> +
>  test_done
> diff --git a/worktree.c b/worktree.c
> index d6a0ee7f73..c79b3e42bb 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -222,9 +222,12 @@ struct worktree *find_worktree(struct worktree **list,
>                 free(to_free);
>                 return NULL;
>         }
> -       for (; *list; list++)
> -               if (!fspathcmp(path, real_path((*list)->path)))
> +       for (; *list; list++) {
> +               const char *wt_path = real_path_if_valid((*list)->path);
> +
> +               if (wt_path && !fspathcmp(path, wt_path))
>                         break;
> +       }
>         free(path);
>         free(to_free);
>         return *list;
> --
> 2.21.0.1141.gd54ac2cb17
>

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-12 10:12 "add worktree" fails with "fatal: Invalid path" error Shaheed Haque
2019-05-13  9:20 ` Duy Nguyen
2019-05-13 12:55   ` Shaheed Haque
2019-05-14 12:33     ` Duy Nguyen
2019-05-14 14:48       ` Shaheed Haque
2019-05-13 10:49 ` [PATCH] worktree add: be tolerant of corrupt worktrees Nguyễn Thái Ngọc Duy
2019-05-13 12:42   ` Shaheed Haque [this message]
2019-05-17  7:46   ` Eric Sunshine
2019-05-18 11:49     ` Duy Nguyen
2019-06-01 19:29       ` Shaheed Haque

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHAc2jeFva3MLpuXEiBbwa7U5HuZiaqawkc3udsyPCaFR4FAnA@mail.gmail.com \
    --to=shaheedhaque@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox