[PATCH 11/11] Support bundles in builtin-clone

[PATCH 11/11] Support bundles in builtin-clone

[Johannes Schindelin]

This forward-ports c6fef0bb(clone: support cloning full bundles) to the
builtin clone.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-clone.c        |   63 ++++++++++++++++++++++++++++-------------------
 t/t5701-clone-local.sh |    6 ++--
 2 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index 1b83062..e4047ed 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -64,42 +64,52 @@ static struct option builtin_clone_options[] = {
 	OPT_END()
 };
 
-static char *get_repo_path(const char *repo)
+static char *get_repo_path(const char *repo, int *is_bundle)
 {
-	const char *path;
-	struct stat buf;
-
-	path = mkpath("%s/.git", repo);
-	if (!stat(path, &buf) && S_ISDIR(buf.st_mode))
-		return xstrdup(make_absolute_path(path));
-
-	path = mkpath("%s.git", repo);
-	if (!stat(path, &buf) && S_ISDIR(buf.st_mode))
-		return xstrdup(make_absolute_path(path));
+	static char *suffix[] = { "/.git", ".git", "" };
+	static char *bundle_suffix[] = { ".bundle", "" };
+	struct stat st;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(suffix); i++) {
+		const char *path;
+		path = mkpath("%s%s", repo, suffix[i]);
+		if (!stat(path, &st) && S_ISDIR(st.st_mode)) {
+			*is_bundle = 0;
+			return xstrdup(make_absolute_path(path));
+		}
+	}
 
-	if (!stat(repo, &buf) && S_ISDIR(buf.st_mode))
-		return xstrdup(make_absolute_path(repo));
+	for (i = 0; i < ARRAY_SIZE(bundle_suffix); i++) {
+		const char *path;
+		path = mkpath("%s%s", repo, bundle_suffix[i]);
+		if (!stat(path, &st) && S_ISREG(st.st_mode)) {
+			*is_bundle = 1;
+			return xstrdup(make_absolute_path(path));
+		}
+	}
 
 	return NULL;
 }
 
-static char *guess_dir_name(const char *repo)
+static char *guess_dir_name(const char *repo, int is_bundle)
 {
 	const char *p, *start, *end, *limit;
 	int after_slash_or_colon;
 
 	/* Guess dir name from repository: strip trailing '/',
-	 * strip trailing '[:/]*git', strip leading '.*[/:]'. */
+	 * strip trailing '[:/]*.{git,bundle}', strip leading '.*[/:]'. */
 
 	after_slash_or_colon = 1;
 	limit = repo + strlen(repo);
 	start = repo;
 	end = limit;
 	for (p = repo; p < limit; p++) {
-		if (!prefixcmp(p, ".git")) {
+		const char *prefix = is_bundle ? ".bundle" : ".git";
+		if (!prefixcmp(p, prefix)) {
 			if (!after_slash_or_colon)
 				end = p;
-			p += 3;
+			p += strlen(prefix) - 1;
 		} else if (!prefixcmp(p, ".bundle")) {
 			if (!after_slash_or_colon)
 				end = p;
@@ -312,6 +322,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 {
 	int use_local_hardlinks = 1;
 	int use_separate_remote = 1;
+	int is_bundle = 0;
 	struct stat buf;
 	const char *repo_name, *repo, *work_tree, *git_dir;
 	char *path, *dir;
@@ -345,14 +356,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	repo_name = argv[0];
 
-	if (argc == 2) {
-		dir = xstrdup(argv[1]);
-	} else {
-		dir = guess_dir_name(repo_name);
-	}
-	fprintf(stderr, "dir is %s\n", dir);
-
-	path = get_repo_path(repo_name);
+	path = get_repo_path(repo_name, &is_bundle);
 	if (path != NULL)
 		repo = xstrdup(make_absolute_path(path));
 	else if (!strchr(repo_name, ':'))
@@ -361,6 +365,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		repo = repo_name;
 	fprintf(stderr, "repo is %s\n", repo);
 
+	if (argc == 2) {
+		dir = xstrdup(argv[1]);
+	} else {
+		dir = guess_dir_name(repo_name, is_bundle);
+	}
+	fprintf(stderr, "dir is %s\n", dir);
+
 	if (!stat(dir, &buf))
 		die("destination directory '%s' already exists.", dir);
 
@@ -426,7 +437,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	refspec.src = "refs/heads/";
 	refspec.dst = branch_top;
 
-	if (path != NULL)
+	if (path != NULL && !is_bundle)
 		refs = clone_local(path, git_dir);
 	else {
 		struct remote *remote = remote_get(argv[0]);
diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 7cfe716..8dfaaa4 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -83,14 +83,14 @@ test_expect_success 'bundle clone without .bundle suffix' '
 	git fetch
 '
 
-test_expect_failure 'bundle clone with .bundle suffix' '
+test_expect_success 'bundle clone with .bundle suffix' '
 	cd "$D" &&
 	git clone b1.bundle &&
 	cd b1 &&
 	git fetch
 '
 
-test_expect_failure 'bundle clone from b4' '
+test_expect_success 'bundle clone from b4' '
 	cd "$D" &&
 	git clone b4 bdl &&
 	cd bdl &&
@@ -108,7 +108,7 @@ test_expect_success 'bundle clone from b4.bundle that does not exist' '
 	fi
 '
 
-test_expect_failure 'bundle clone with nonexistent HEAD' '
+test_expect_success 'bundle clone with nonexistent HEAD' '
 	cd "$D" &&
 	git clone b2.bundle b2 &&
 	cd b2 &&
-- 
1.5.4.3.327.g614d7.dirty

Re: [PATCH 11/11] Support bundles in builtin-clone

[Johannes Schindelin]

Hi,

On Sat, 8 Mar 2008, Johannes Schindelin wrote:

> This forward-ports c6fef0bb(clone: support cloning full bundles) to the
> builtin clone.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

Wow... Is this a new send-email feature?  I did not send this email...

Ciao,
Dscho

Re: [PATCH 11/11] Support bundles in builtin-clone

[Daniel Barkalow]

On Sun, 9 Mar 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Sat, 8 Mar 2008, Johannes Schindelin wrote:
> 
> > This forward-ports c6fef0bb(clone: support cloning full bundles) to the
> > builtin clone.
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> 
> Wow... Is this a new send-email feature?  I did not send this email...

Nope, it's an old format-patch feature. format-patch generates the 
messages with the From: being the commit author, and my MTA doesn't 
complain about the fact that I'm sending email with some entirely 
different From:. It would probably be more clever to have format-patch use 
the committer or the current user as the From:, and put an additional 
From: in the message body with the author if it's not the email From:.

Of course, you did sort of send that email 
(http://permalink.gmane.org/gmane.comp.version-control.git/75743). I just 
bounced it to the list again with a different subject, some other headers, 
the note removed, and the patch rebased. Or something like that. I 
actually didn't expect it to come out looking like you'd sent the email 
until it showed up (or, rather, until it didn't show up for a little while 
because vger thought it was odd).

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 11/11] Support bundles in builtin-clone

[Johannes Schindelin]

Hi,

On Sun, 9 Mar 2008, Daniel Barkalow wrote:

> On Sun, 9 Mar 2008, Johannes Schindelin wrote:
> 
> > On Sat, 8 Mar 2008, Johannes Schindelin wrote:
> > 
> > > This forward-ports c6fef0bb(clone: support cloning full bundles) to 
> > > the builtin clone.
> > > 
> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > ---
> > 
> > Wow... Is this a new send-email feature?  I did not send this email...
> 
> Nope, it's an old format-patch feature. format-patch generates the 
> messages with the From: being the commit author, and my MTA doesn't 
> complain about the fact that I'm sending email with some entirely 
> different From:. It would probably be more clever to have format-patch 
> use the committer or the current user as the From:, and put an 
> additional From: in the message body with the author if it's not the 
> email From:.

I think that makes sense.

> Of course, you did sort of send that email 
> (http://permalink.gmane.org/gmane.comp.version-control.git/75743).

Sort of.

Of course, I am okay with it, but I consider the From: issue a real bug in 
send-email (or format-patch, if you want).

Ciao,
Dscho

Re: [PATCH 11/11] Support bundles in builtin-clone

[Daniel Barkalow]

On Sun, 9 Mar 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Sun, 9 Mar 2008, Daniel Barkalow wrote:
> 
> > On Sun, 9 Mar 2008, Johannes Schindelin wrote:
> > 
> > > On Sat, 8 Mar 2008, Johannes Schindelin wrote:
> > > 
> > > > This forward-ports c6fef0bb(clone: support cloning full bundles) to 
> > > > the builtin clone.
> > > > 
> > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > > > ---
> > > 
> > > Wow... Is this a new send-email feature?  I did not send this email...
> > 
> > Nope, it's an old format-patch feature. format-patch generates the 
> > messages with the From: being the commit author, and my MTA doesn't 
> > complain about the fact that I'm sending email with some entirely 
> > different From:. It would probably be more clever to have format-patch 
> > use the committer or the current user as the From:, and put an 
> > additional From: in the message body with the author if it's not the 
> > email From:.
> 
> I think that makes sense.
> 
> > Of course, you did sort of send that email 
> > (http://permalink.gmane.org/gmane.comp.version-control.git/75743).
> 
> Sort of.
> 
> Of course, I am okay with it, but I consider the From: issue a real bug in 
> send-email (or format-patch, if you want).

Agreed; (format-patch, I didn't use send-email). But I don't understand 
the flow control in pretty.c nearly well enough to fix it right now. It 
lists the author in the headers, like all of the log output formats, and 
doesn't then have the author available in the body, afaict.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 11/11] Support bundles in builtin-clone

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Of course, I am okay with it, but I consider the From: issue a real bug in 
> send-email (or format-patch, if you want).

Format-patch is about recording the author of the patch on From: line
(think of it a tool to reproduce an e-mail that you would have received
and applied to your tree).  It may probably make a lot of sense to teach
send-email not to forge From: (even though, RFC is Ok with it, as long as
it also correctly adds Sender: pointing at the real sender).

Re: [PATCH 11/11] Support bundles in builtin-clone

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Of course, I am okay with it, but I consider the From: issue a real bug in 
>> send-email (or format-patch, if you want).
>
> Format-patch is about recording the author of the patch on From: line
> (think of it a tool to reproduce an e-mail that you would have received
> and applied to your tree).  It may probably make a lot of sense to teach
> send-email not to forge From: (even though, RFC is Ok with it, as long as
> it also correctly adds Sender: pointing at the real sender).

Well, as I re-read send-email, it does put you on the From: and adds the
patch author From: at the beginning of the message.  So there is no bug
here.  Daniel did not use send-email, so of course even if there were a
bug in send-email it would not have triggered ;-)

So there is no bug to hunt here.

Re: [PATCH 11/11] Support bundles in builtin-clone

[Johannes Schindelin]

Hi,

On Sun, 9 Mar 2008, Junio C Hamano wrote:

> Well, as I re-read send-email, it does put you on the From: and adds the 
> patch author From: at the beginning of the message.  So there is no bug 
> here.  Daniel did not use send-email, so of course even if there were a 
> bug in send-email it would not have triggered ;-)
> 
> So there is no bug to hunt here.

Thanks for clarifying,
Dscho