Re: git clone NonExistentLocation
- From
Jeff King <peff@peff.net>
- Date
- Feb 18, 2011, 06:11 UTC
- Message-ID
- <20110218061133.GA25847@sigill.intra.peff.net>
- In-Reply-To
- <7vd3mp6ftp.fsf@alter.siamese.dyndns.org>
On Thu, Feb 17, 2011 at 10:02:10PM -0800, Junio C Hamano wrote:
Show 36 quoted lines
> Jeff King <peff@peff.net> writes:
>
> > I think the patch below is the right fix.
> > ...
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> > 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()?Hmm, good point. I didn't look carefully enough into get_repo_path. It is not really about finding a repo, but rather about finding a directory or filename. Plus, my file_exists() isn't quite right, anyway. We really want to make sure $repo_name.git, $repo_name.bundle, etc don't exist. But get_repo_path has already done that for us, so we should just trust its output.
Show 13 quoted lines
> 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;Yeah, I think that is a better fix.
-Peff