From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sun, 16 Dec 2018 13:18:55 GMT
Subject: Re: [PATCH] worktree: allow to (re)move worktrees with uninitialized submodules
Message-ID: <CAPig+cQLz+mx8pXMCsYW_0=1C8GzFvzBb=uatfwLK0Dpj944Jg@mail.gmail.com>
In-Reply-To: <20181216121239.10017-1-pclouds@gmail.com>

On Sun, Dec 16, 2018 at 7:12 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Uninitialized submodules have nothing valueable for us to be worried
> about. They are just SHA-1. Let "worktree remove" and "worktree move"
> continue in this case so that people can still use multiple worktrees
> on repos with optional submodules that are never populated, like
> sha1collisiondetection in git.git when checked out by doc-diff script.
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -724,20 +725,32 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
>  static void validate_no_submodules(const struct worktree *wt)
>  {
> +       if (is_directory(worktree_git_path(wt, "modules")))
> +               die(_("working trees containing submodules cannot be moved or removed"));
> +
>         if (read_index_from(&istate, worktree_git_path(wt, "index"),
>                             get_worktree_git_dir(wt)) > 0) {
>                 [...]
> +                       found_submodules = 1;
> +                       break;
>         }
>         if (found_submodules)
>                 die(_("working trees containing submodules cannot be moved or removed"));

Not worth a re-roll, but an alternate way to structure this to avoid
duplicating the die() message would be:

    if (is_directory(...))
        found_submodules = 1;
    else if (read_index_from(...)) {
        ...
        found_submodules = 1;
        break;
    }
    if (found_submodules)
        die(...);