Re: [PATCH 2/2] git clone: don't clone into non-empty directory

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Jul 1, 2020 at 5:46 AM Ben Wijen <ben@wijen.net> wrote:
> When using git clone with --separate-git-dir realgitdir and
> realgitdir already exists, it's content is destroyed.
>
> Make sure we don't clone into an existing non-empty directory.
>
> Signed-off-by: Ben Wijen <ben@wijen.net>
> ---
> diff --git a/builtin/clone.c b/builtin/clone.c
> @@ -1021,6 +1021,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>                 die(_("destination path '%s' already exists and is not "
>                         "an empty directory."), dir);
>
> +       if (real_git_dir) {
> +               real_dest_exists = path_exists(real_git_dir);
> +               if (real_dest_exists && !is_empty_dir(real_git_dir))

"git init --separate-git-dir=" checks only whether the path exists,
and errors out if it does; it doesn't care whether the directory is
empty or not. I'm wondering, therefore, if this check should be
tightened to more closely align the behavior of the two commands.
(Using the tighter semantic now doesn't prohibit loosening it in the
future, whereas it's harder to tighten behavior which started out
loose.)

> +                       die(_("destination path '%s' already exists and is not "
> +                               "an empty directory."), real_git_dir);

This message might be a bit clearer if "destination" is changed to
"repository". (And drop the bit about not being empty -- if we go that
route.)