Jeff King <peff@xxxxxxxx> writes:
> I think the patch below is the right fix.
> ...
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> builtin/clone.c | 5 ++++-
> t/t5701-clone-local.sh | 13 +++++++++++++
> 2 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 60d9a64..55785d0 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -412,8 +412,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> path = get_repo_path(repo_name, &is_bundle);
> if (path)
> repo = xstrdup(make_nonrelative_path(repo_name));
> - else if (!strchr(repo_name, ':'))
> + else if (!strchr(repo_name, ':')) {
> + if (!file_exists(repo_name))
> + die("repository '%s' does not exist", repo_name);
> repo = xstrdup(make_absolute_path(repo_name));
> + }
> else
> repo = repo_name;
> is_local = path && !is_bundle;
Thanks, but I am confused.
The stuff goes through make_absolute_path() so we must be certain that
this has to be a local filesystem entity _if_ it is a repository.
But when will we see a file at repo_name in this new codepath? In what
situation would get_repo_path(repo_name, &is_bundle) return NULL but the
added file_exists(repo_name) would yield true to bypess your die()?
If repo_name is a directory, or a regular file, get_repo_path() would give
us a relative path to it, no?
IOW, wouldn't the fix be more like this? Your new test script seems to
pass with this.
diff --git a/builtin/clone.c b/builtin/clone.c
index 60d9a64..2ee1fa9 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -413,7 +413,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (path)
repo = xstrdup(make_nonrelative_path(repo_name));
else if (!strchr(repo_name, ':'))
- repo = xstrdup(make_absolute_path(repo_name));
+ die("repository '%s' does not exist", repo_name);
else
repo = repo_name;
is_local = path && !is_bundle;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html