git clone NonExistentLocation

> Hi.
> 
> If I do:
> 
>  $ uname -a
> Linux as100897 2.6.26-2-686 #1 SMP Thu Nov 25 01:53:57 UTC 2010 i686 GNU/Linux
>  $ git version
> git version 1.7.4.1
>  $ ls -l NonExistentLocation
> ls: cannot access NonExistentLocation: No such file or directory
>  $ git clone NonExistentLocation
> Cloning into NonExistentLocation...
> warning: You appear to have cloned an empty repository.
>  $
> 
> I get a new (empty) git repository in 'NonExistentLocation':
> 
>  $ tree -a NonExistentLocation
> NonExistentLocation
> `-- .git
>     |-- HEAD
>     |-- branches
>     |-- config
>     |-- description
>     |-- hooks
>     |   |-- applypatch-msg.sample
>     |   |-- commit-msg.sample
>     |   |-- post-commit.sample
>     |   |-- post-receive.sample
>     |   |-- post-update.sample
>     |   |-- pre-applypatch.sample
>     |   |-- pre-commit.sample
>     |   |-- pre-rebase.sample
>     |   |-- prepare-commit-msg.sample
>     |   `-- update.sample
>     |-- info
>     |   `-- exclude
>     |-- objects
>     |   |-- info
>     |   `-- pack
>     `-- refs
>         |-- heads
>         `-- tags
> 
> 10 directories, 14 files
> 
> Is this the intended behaviour ?
> 
> Thanks,
>   Stefan

> 
> It is useful, and it even gives you a warning that it still might not be
> what you intended. Would be funny if it were accidental. Indeed, a git
> "log -S" on that warning reveals that it was introduced intentionally in
> 
> 86ac751 (Allow cloning an empty repository, 2009-01-23)

> On 2/17/2011 1:39 PM, Michael J Gruber wrote:
>>
>> It is useful, and it even gives you a warning that it still might not be
>> what you intended. Would be funny if it were accidental. Indeed, a git
>> "log -S" on that warning reveals that it was introduced intentionally in
>>
>> 86ac751 (Allow cloning an empty repository, 2009-01-23)
> 
> OK. But that's about 'cloning an empty repository'.
> 'NonExistentLocation' is not empty initially - it simply does
> not exist.
> 
> Contrast that to 'git clone http://url.does.not.exist'. You don't
> get an empty repository in 'url.does.not.exist' after running that.

> Sverre, with your 86ac751, the following two are equivalent (modulo a
> warning) on a nonexisting dir:
>
> git clone dir
> git init dir
>
> Is that intentional?

> Heya,
> 
> [Thanks for summarizing.]
> 
> On Thu, Feb 17, 2011 at 12:59, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Sverre, with your 86ac751, the following two are equivalent (modulo a
>> warning) on a nonexisting dir:
>>
>> git clone dir
>> git init dir
>>
>> Is that intentional?
> 
> No, that was not intentional. The former should still be an error if
> 'dir' is an empy directory.
> 

> Digging a little further: since a nonexisting directory is neither a dir
> nor a file, clone thinks it is not local (is_local=is_bundle=0). None of
> the transport_* commands error out since the relevant one is guarded by
> 86ac751...
> 
> Reverting that or forcing is_local=1 both help, but how to detect "local
> nonexisting" reliably?
> 
> In fact, I don't have a problem with the current state if we document it :)

> 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;

> 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()?

> 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;