threads / discuss / 12308

[PATCHv2] clone: support cloning full bundles

Subject: [PATCHv2] clone: support cloning full bundles

## tl;dr

12 messages between Feb 25, 2008 and Feb 29, 2008.

replies: 11people: 5as markdown or json

Santi Béjar· Feb 25, 2008, 10:12 UTC · lore
Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
Hi *,
  while waiting for the C port of git-clone I've updated my patch to
support bundles in "git clone".
Santi
 git-clone.sh           |   17 +++++++++++++----
 t/t5701-clone-local.sh |   11 +++++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)
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" ] &&
@@ -364,11 +367,17 @@ yes)
 		fi
 		;;
 	*)
-		case "$upload_pack" in
-		'') git-fetch-pack --all -k $quiet $depth $no_progress "$repo";;
-		*) git-fetch-pack --all -k $quiet "$upload_pack" $depth $no_progress "$repo" ;;
-		esac >"$GIT_DIR/CLONE_HEAD" ||
+		if [ -f "$repo" ] ; then
+			git bundle unbundle "$repo" > "$GIT_DIR/CLONE_HEAD" ||
+			die "unbundle from '$repo' failed."
+		else
+			case "$upload_pack" in
+			'') git-fetch-pack --all -k $quiet $depth $no_progress "$repo";;
+			*) git-fetch-pack --all -k \
+				$quiet "$upload_pack" $depth $no_progress "$repo" ;;
+			esac >"$GIT_DIR/CLONE_HEAD" ||
 			die "fetch-pack from '$repo' failed."
+		fi
 		;;
 	esac
 	;;
diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 59a165a..8a8e65c 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -9,6 +9,7 @@ test_expect_success 'preparing origin repository' '
 	: >file && git add . && git commit -m1 &&
 	git clone --bare . a.git &&
 	git clone --bare . x &&
+	git bundle create b1.bundle --all HEAD &&
 	test "$(GIT_CONFIG=a.git/config git config --bool core.bare)" = true &&
 	test "$(GIT_CONFIG=x/config git config --bool core.bare)" = true
 '
@@ -71,4 +72,14 @@ test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
 	git fetch &&
 	test ! -e .git/refs/remotes/origin/HEAD'
 
+test_expect_success 'bundle clone with suffix' '
+	cd "$D" &&
+	git clone b1.bundle
+'
+
+test_expect_success 'bundle clone' '
+	cd "$D" &&
+	git clone b1.bundle bdl1
+'
+
 test_done

-- 
1.5.4.3.293.gac81
Junio C Hamano· Feb 25, 2008, 19:30 UTC · re: Santi Béjar · lore

Re: [PATCHv2] clone: support cloning full bundles

Santi Béjar <sbejar@gmail.com> writes:
Show 7 quoted lines
> Signed-off-by: Santi Béjar <sbejar@gmail.com>
> ---
>
> Hi *,
>
>   while waiting for the C port of git-clone I've updated my patch to
> support bundles in "git clone".

I do not recall the v1 round of this patch, but it does not look too bad.

I was hoping, however, if we can replace "git fetch-pack" there with "git-fetch" so that you do not have to have the if conditional based on "$repo"'s type. Isn't "git fetch" capable of fetching from a bundle already?

Santi Béjar· Feb 25, 2008, 20:29 UTC · re: Junio C Hamano · lore

Re: [PATCHv2] clone: support cloning full bundles

On Mon, Feb 25, 2008 at 8:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> Santi Béjar <sbejar@gmail.com> writes:
>
>  > Signed-off-by: Santi Béjar <sbejar@gmail.com>
>  > ---
>  >
>  > Hi *,
>  >
>  >   while waiting for the C port of git-clone I've updated my patch to
>  > support bundles in "git clone".
>
>  I do not recall the v1 round of this patch, but it does not look
>  too bad.
Well, it was during the freeze, you probably had more important things to do :-)
http://article.gmane.org/gmane.comp.version-control.git/68035
Show 6 quoted lines
>
>  I was hoping, however, if we can replace "git fetch-pack" there
>  with "git-fetch" so that you do not have to have the if
>  conditional based on "$repo"'s type.  Isn't "git fetch" capable
>  of fetching from a bundle already?
>

"git fetch" is capable to fetch bundles, but to use "git fetch" instead of "git fetch-pack" you need to reorganize other things (not the same output, to download all the refs you need to write a config with refs/heads/*:..., "git fetch" does not have a --no-progress), so I opted for a minimal path (and not break the most used path :-).

I sent another patch to convert every transport to "git fetch" in:
http://article.gmane.org/gmane.comp.version-control.git/65669

but it had some problems (ups now I see I didn't answer to Johannes Schindelin's reply, sorry, or I cannot find it). At the end as git-clone was going to be ported to C...

Santi
Junio C Hamano· Feb 25, 2008, 21:00 UTC · re: Santi Béjar · lore

Re: [PATCHv2] clone: support cloning full bundles

"Santi Béjar" <sbejar@gmail.com> writes:
Show 5 quoted lines
> ...
> you need to reorganize other things (not the same output, to download
> all the refs you need to write a config with refs/heads/*:..., "git
> fetch" does not have a --no-progress), so I opted for
> a minimal path (and not break the most used path :-).

That sounds like a sensible explanation. As the external interface won't change even if we later do so, let's take this version for 1.5.5.

Thanks.
Junio C Hamano· Feb 27, 2008, 22:44 UTC · re: Santi Béjar · lore

Re: [PATCHv2] clone: support cloning full bundles

Santi Béjar <sbejar@gmail.com> writes:
Show 20 quoted lines
> 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.

Santi Béjar· Feb 27, 2008, 23:04 UTC · re: Junio C Hamano · lore

Re: [PATCHv2] clone: support cloning full bundles

On Wed, Feb 27, 2008 at 11:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 43 quoted lines
> 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, 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.

Santi
Santi Béjar· Feb 27, 2008, 23:16 UTC · re: Santi Béjar · lore

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
Daniel Barkalow· Feb 27, 2008, 23:48 UTC · re: Santi Béjar · lore

Re: [PATCHv2] clone: support cloning full bundles

On Thu, 28 Feb 2008, Santi Béjar wrote:
Show 71 quoted lines
> 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.

The transport.c determination of a bundle is if it's a local path that names a file, so [ -f "$repo" ] is the corresponding test.

	-Daniel
*This .sig left intentionally blank*
Junio C Hamano· Feb 28, 2008, 00:19 UTC · re: Santi Béjar · lore

Re: [PATCHv2] clone: support cloning full bundles

"Santi Béjar" <sbejar@gmail.com> writes:
Show 5 quoted lines
> My preference are in order:
>
> 1) define an official extension + the && [ -f "$repo" ]
> 2) the && [ -f "$repo"]
> 3) just remove this line

Sure. (1) sounds sane and if you make the extension ".git", then I do not think you need anything more ;-). Jokes aside, calling it .bundle or .bndl would be sane.

Opinions?
Johannes Schindelin· Feb 28, 2008, 00:26 UTC · re: Junio C Hamano · lore

Re: [PATCHv2] clone: support cloning full bundles

Hi,
On Wed, 27 Feb 2008, Junio C Hamano wrote:
Show 13 quoted lines
> "Santi Béjar" <sbejar@gmail.com> writes:
> 
> > My preference are in order:
> >
> > 1) define an official extension + the && [ -f "$repo" ]
> > 2) the && [ -f "$repo"]
> > 3) just remove this line
> 
> Sure.  (1) sounds sane and if you make the extension ".git",
> then I do not think you need anything more ;-).  Jokes aside,
> calling it .bundle or .bndl would be sane.
> 
> Opinions?
I like .bundle, obviously (I used it in every single example I gave).

Ciao, Dscho

P.S.: only recently, I found out how useful bundles really are. Thanks, Mark, for persisting!

Santi Béjar· Feb 28, 2008, 00:32 UTC · re: Johannes Schindelin · lore

Re: [PATCHv2] clone: support cloning full bundles

On Thu, Feb 28, 2008 at 1:26 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

>  On Wed, 27 Feb 2008, Junio C Hamano wrote:
>
[...]
Show 5 quoted lines
>  > calling it .bundle or .bndl would be sane.
>  >
>  > Opinions?
>
>  I like .bundle, obviously (I used it in every single example I gave).
+1
Santi
Mark Levedahl· Feb 29, 2008, 04:27 UTC · re: Johannes Schindelin · lore

Re: [PATCHv2] clone: support cloning full bundles

Johannes Schindelin wrote:
Show 8 quoted lines
> I like .bundle, obviously (I used it in every single example I gave).
>
> Ciao,
> Dscho
>
> P.S.: only recently, I found out how useful bundles really are.  Thanks, 
> Mark, for persisting!
>

You're very welcome, and thank you for the implementation - bundles are a central part of my workflow.

Mark

← back to recent threads