Re: [PATCHv2] clone: support cloning full bundles
On Thu, Feb 28, 2008 at 12:04 AM, Santi Béjar <sbejar@gmail.com> wrote:
Show 53 quoted lines
>
> 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" ]
Show 6 quoted lines
> , 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.
Santi