Re: [PATCHv2] clone: support cloning full bundles

> On Thu, Feb 28, 2008 at 12:04 AM, Santi Béjar <sbejar@gmail.com> wrote:
> >
> > On Wed, Feb 27, 2008 at 11:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >  > Santi Béjar <sbejar@gmail.com> writes:
> >  >
> >  >  > Signed-off-by: Santi Béjar <sbejar@gmail.com>
> >  >
> >  > > diff --git a/git-clone.sh b/git-clone.sh
> >  >  > index 0d686c3..2efb947 100755
> >  >  > --- a/git-clone.sh
> >  >  > +++ b/git-clone.sh
> >  >  > @@ -210,11 +210,14 @@ if base=$(get_repo_base "$repo"); then
> >  >  >       then
> >  >  >               local=yes
> >  >  >       fi
> >  >  > +elif [ -f "$repo" ] ; then
> >  >  > +     case "$repo" in /*);; *) repo="$PWD/$repo" ; esac
> >  >  >  fi
> >  >  >
> >  >  >  dir="$2"
> >  >  >  # Try using "humanish" part of source repo if user didn't specify one
> >  >  >  [ -z "$dir" ] && dir=$(echo "$repo" | sed -e 's|/$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
> >  >  > +[ -f "$dir" ] && dir=$(expr "$repo" : '.*/\([^/]*\)\.[^/]*')
> >  >  >  [ -e "$dir" ] && die "destination directory '$dir' already exists."
> >  >  >  [ yes = "$bare" ] && unset GIT_WORK_TREE
> >  >  >  [ -n "$GIT_WORK_TREE" ] && [ -e "$GIT_WORK_TREE" ] &&
> >  >
> >  >  What is this [ -f "$dir" ] line doing?  The purpose of these
> >  >  lines is:
> >  >
> >  >   - The user (might have) said "clone to $2";
> >  >
> >  >   - If the user didn't, then set dir to humanish part;
> >  >
> >  >   - If that exists, we barf.
> >  >
> >  >  I do not see any valid reason for an additional logic in this
> >  >  sequence when adding a new clone _source_ type.
> >  >
> >  >  The check to see if "$dir" _exists_ is even worse.  If the user
> >  >  said "clone _to_ this", then we would not have invented $dir
> >  >  based on the $repo (which is what the user said "clone _from_")
> >  >  but used whatever name the user has given us.  The existing
> >  >  check to barf "Hey, that explicit location you told me to clone
> >  >  to is WRONG!!!" should not be broken.
> >  >
> >
> >  Oops, I did not thought about that possibility. The check was to be
> >  able to clone a bundle that was in the local directory. So:
> >
> >  $ git clone git.bundle
> >
> >  clones git.bundle to the directory git. Maybe we can add an && in the
> >  previous line
> 
> and check for [ -f "$repo" ] insted of [ -f "$dir" ]
> 
> >  , or we can say simply that to clone a bundle in the same
> >  directory you have to specify explicitly where to clone.
> >
> >  In another patch we could add an "official" extension to the bundles
> >  (the same way as with the repositories "*.git") as project.bdl or
> >  project.bundle and just remove this extension.
> 
> My preference are in order:
> 
> 1) define an official extension + the && [ -f "$repo" ]
> 2) the && [ -f "$repo"]
> 3) just remove this line
> 
> but not that strong.