Subject: Re: [PATCH v2 08/15] scalar: implement the `clone` subcommand

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 06 Sep 2021 03:12:11 +0200

On Fri, Sep 03 2021, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <[email-masked]>
>
> This implements Scalar's opinionated `clone` command: it tries to use a
> partial clone and sets up a sparse checkout by default. In contrast to
> `git clone`, `scalar clone` sets up the worktree in the `src/`
> subdirectory, to encourage a separation between the source files and the
> build output (which helps Git tremendously because it avoids untracked
> files that have to be specifically ignored when refreshing the index).

Perhaps it's simpler to just say about that /src/ injection:

    `scalar clone` adds an implicit "/src" subdirectory to whatever
    directory the user provides, with the added stricture on top of
    doing that with "git clone" that the "src" cannot exist already.

...

> +	if (is_directory(enlistment))
> +		die(_("directory '%s' exists already"), enlistment);
> +
> +	dir = xstrfmt("%s/src", enlistment);

Which also seems to suggest a bug here. I.e. if I "git clone <repo>
/tmp/xyz/abc" and Ctrl+C it we'll remove "abc", but leave "xyz"
behind. Since we're creating that "xyz" (or "src") implicitly here an
abort/ctrl+C followed by a retry is going to run into this error, isn't
it?

I.e. it seems what's missing in this state machine is checking if the
directory was there already, and if it isn't add it to the existing
atexit() removals.

Which may be tricky seeing as this is shelling out to "init" then
"fetch" etc, i.e. who removes it? But maybe not.