# [PATCHv2] clone: support cloning full bundles 12 messages from 2008-02-25 to 2008-02-29. Participants: Santi Béjar, Junio C Hamano, Daniel Barkalow, Johannes Schindelin, Mark Levedahl. Thread: https://gitlist.dev/t/12308 ## Santi Béjar, 2008-02-25 10:12 Subject: [PATCHv2] clone: support cloning full bundles Message-ID: <1203934349-12125-1-git-send-email-sbejar@gmail.com> URL: https://gitlist.dev/e/1203934349-12125-1-git-send-email-sbejar%40gmail.com ``` Signed-off-by: Santi Béjar --- 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, 2008-02-25 19:30 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <7vlk58j0k1.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vlk58j0k1.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1203934349-12125-1-git-send-email-sbejar@gmail.com> ``` Santi Béjar writes: > Signed-off-by: Santi Béjar > --- > > 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, 2008-02-25 20:29 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <8aa486160802251229x778c7427nd51093bc23ec6c8c@mail.gmail.com> URL: https://gitlist.dev/e/8aa486160802251229x778c7427nd51093bc23ec6c8c%40mail.gmail.com In-Reply-To: <7vlk58j0k1.fsf@gitster.siamese.dyndns.org> ``` On Mon, Feb 25, 2008 at 8:30 PM, Junio C Hamano wrote: > Santi Béjar writes: > > > Signed-off-by: Santi Béjar > > --- > > > > 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 > > 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, 2008-02-25 21:00 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <7vk5kshhu7.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vk5kshhu7.fsf%40gitster.siamese.dyndns.org In-Reply-To: <8aa486160802251229x778c7427nd51093bc23ec6c8c@mail.gmail.com> ``` "Santi Béjar" writes: > ... > 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, 2008-02-27 22:44 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <7v63waqasu.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v63waqasu.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1203934349-12125-1-git-send-email-sbejar@gmail.com> ``` Santi Béjar writes: > Signed-off-by: Santi Béjar > 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, 2008-02-27 23:04 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <8aa486160802271504r7744422ag290b1f4d1725210f@mail.gmail.com> URL: https://gitlist.dev/e/8aa486160802271504r7744422ag290b1f4d1725210f%40mail.gmail.com In-Reply-To: <7v63waqasu.fsf@gitster.siamese.dyndns.org> ``` On Wed, Feb 27, 2008 at 11:44 PM, Junio C Hamano wrote: > Santi Béjar writes: > > > Signed-off-by: Santi Béjar > > > 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, 2008-02-27 23:16 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <8aa486160802271516h7c2c636dna3a9f067fc51da2@mail.gmail.com> URL: https://gitlist.dev/e/8aa486160802271516h7c2c636dna3a9f067fc51da2%40mail.gmail.com In-Reply-To: <8aa486160802271504r7744422ag290b1f4d1725210f@mail.gmail.com> ``` On Thu, Feb 28, 2008 at 12:04 AM, Santi Béjar wrote: > > On Wed, Feb 27, 2008 at 11:44 PM, Junio C Hamano wrote: > > Santi Béjar writes: > > > > > Signed-off-by: Santi Béjar > > > > > 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. Santi ``` ## Daniel Barkalow, 2008-02-27 23:48 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0802271846180.19665%40iabervon.org In-Reply-To: <8aa486160802271516h7c2c636dna3a9f067fc51da2@mail.gmail.com> ``` On Thu, 28 Feb 2008, Santi Béjar wrote: > On Thu, Feb 28, 2008 at 12:04 AM, Santi Béjar wrote: > > > > On Wed, Feb 27, 2008 at 11:44 PM, Junio C Hamano wrote: > > > Santi Béjar writes: > > > > > > > Signed-off-by: Santi Béjar > > > > > > > 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, 2008-02-28 00:19 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <7vpruindag.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vpruindag.fsf%40gitster.siamese.dyndns.org In-Reply-To: <8aa486160802271516h7c2c636dna3a9f067fc51da2@mail.gmail.com> ``` "Santi Béjar" 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? ``` ## Johannes Schindelin, 2008-02-28 00:26 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0802280024460.22527%40racer.site In-Reply-To: <7vpruindag.fsf@gitster.siamese.dyndns.org> ``` Hi, On Wed, 27 Feb 2008, Junio C Hamano wrote: > "Santi Béjar" 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, 2008-02-28 00:32 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <8aa486160802271632k5c7f14e0q132bd12f53838ed1@mail.gmail.com> URL: https://gitlist.dev/e/8aa486160802271632k5c7f14e0q132bd12f53838ed1%40mail.gmail.com In-Reply-To: ``` On Thu, Feb 28, 2008 at 1:26 AM, Johannes Schindelin wrote: > On Wed, 27 Feb 2008, Junio C Hamano wrote: > [...] > > 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, 2008-02-29 04:27 Subject: Re: [PATCHv2] clone: support cloning full bundles Message-ID: <47C789C7.3090201@verizon.net> URL: https://gitlist.dev/e/47C789C7.3090201%40verizon.net In-Reply-To: ``` Johannes Schindelin wrote: > 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 ```