git clone broken in next

git clone broken in next

[Kevin Ballard]

Something's horribly wrong in next. I just installed the latest from  
next and then ran

   git clone git://github.com/kballard/github-gem.git

The resulting repo had absolutely nothing in the workdir. `git status`  
claims every single file is deleted but not updated. `git checkout -f  
HEAD` fixed the workdir.

This is 100% repeatable with different repositories.

$ git --version
git version 1.5.5.1.373.gce4aa

(this is the tip of next plus a single documentation patch).

Oddly, all of the tests with "clone" in their name seem to be passing  
just fine.

Ok, after running a git-bisect, I narrowed it down to  
8434c2f1afedb936e0ea8c07ce25733013c2f743 (Build in clone).

My system is Mac OS X 10.5.2.

-- 
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com

Re: git clone broken in next

[Jeff King]

On Wed, May 14, 2008 at 11:54:39PM -0400, Kevin Ballard wrote:

> Something's horribly wrong in next. I just installed the latest from next 
> and then ran
>
>   git clone git://github.com/kballard/github-gem.git
>
> The resulting repo had absolutely nothing in the workdir. `git status`  
> claims every single file is deleted but not updated. `git checkout -f  
> HEAD` fixed the workdir.

Hmm, builtin-clone does seem to be completely broken. It looks like
unpack_trees is not doing its job for some reason, but I haven't looked
further. But it surprised me that we aren't testing anything as simple
as "clone works". The test below currently fails (though I am getting
other test failures which are presumably related).

---
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index dc9d63d..593d1a3 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -23,4 +23,11 @@ test_expect_success 'clone with excess parameters' '
 
 '
 
+test_expect_success 'clone checks out files' '
+
+	git clone src dst &&
+	test -f dst/file
+
+'
+
 test_done

Re: git clone broken in next

[Jeff King]

On Thu, May 15, 2008 at 12:44:02AM -0400, Jeff King wrote:

> Hmm, builtin-clone does seem to be completely broken. It looks like
> unpack_trees is not doing its job for some reason, but I haven't looked
> further. But it surprised me that we aren't testing anything as simple
> as "clone works". The test below currently fails (though I am getting
> other test failures which are presumably related).
> 
> ---
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index dc9d63d..593d1a3 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -23,4 +23,11 @@ test_expect_success 'clone with excess parameters' '
>  
>  '
>  
> +test_expect_success 'clone checks out files' '
> +
> +	git clone src dst &&
> +	test -f dst/file
> +
> +'
> +
>  test_done

Just to be sure, I bisected this test, and sure enough, the problem
commit is 8434c2f1 (build in clone). Daniel?

-Peff

[PATCH] builtin-clone: fix initial checkout

[Johannes Schindelin]

Somewhere in the process of finishing up builtin-clone, the update of
the working tree was lost.  This was due to not using the option "merge"
for unpack_trees().

Breakage noticed by Kevin Ballard.

Test by Jeff King.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	> On Thu, May 15, 2008 at 12:44:02AM -0400, Jeff King wrote:
	> 
	> > Hmm, builtin-clone does seem to be completely broken. It looks 
	> > like unpack_trees is not doing its job for some reason, but I
	> > haven't looked further. But it surprised me that we aren't 
	> > testing anything as simple as "clone works". The test below
	> > currently fails (though I am getting other test failures which 
	> > are presumably related).

	I did not notice the breakage earlier, because I ran with a 
	"twoway_merge" version of builtin-clone in my personal tree.

 builtin-clone.c  |    3 +++
 t/t5601-clone.sh |    7 +++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index a7c075d..8713128 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -525,7 +525,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 		memset(&opts, 0, sizeof opts);
 		opts.update = 1;
+		opts.merge = 1;
+		opts.fn = oneway_merge;
 		opts.verbose_update = !option_quiet;
+		opts.src_index = &the_index;
 		opts.dst_index = &the_index;
 
 		tree = parse_tree_indirect(remote_head->old_sha1);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index dc9d63d..593d1a3 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -23,4 +23,11 @@ test_expect_success 'clone with excess parameters' '
 
 '
 
+test_expect_success 'clone checks out files' '
+
+	git clone src dst &&
+	test -f dst/file
+
+'
+
 test_done
-- 
1.5.5.1.424.g3256b

Re: [PATCH] builtin-clone: fix initial checkout

[Jeff King]

On Thu, May 15, 2008 at 10:48:25AM +0100, Johannes Schindelin wrote:

> Somewhere in the process of finishing up builtin-clone, the update of
> the working tree was lost.  This was due to not using the option "merge"
> for unpack_trees().

Looking at how "git-checkout" does it made me think that might be
related, but for some reason I didn't try the obvious.

Thanks.

Tested-by: Jeff King <peff@peff.net>

-Peff

Re: [PATCH] builtin-clone: fix initial checkout

[Daniel Barkalow]

On Thu, 15 May 2008, Johannes Schindelin wrote:

> Somewhere in the process of finishing up builtin-clone, the update of
> the working tree was lost.  This was due to not using the option "merge"
> for unpack_trees().

I was sure I'd added support for update without merge, but now I can't see 
what became of it. I was pretty sure I'd actually needed it for something, 
too, but I guess that got resolved some other way. I'll have to check 
tonight on my laptop for something of the sort in my reflogs.

I'm pretty sure that just including merge and the necessary other options 
here will work fine (since there's no index to merge into) and I don't 
have a simple fix off the top of my head for that flag combination, so:

Acked-by: Daniel Barkalow <barkalow@iabervon.org>

> Breakage noticed by Kevin Ballard.
> 
> Test by Jeff King.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> 	> On Thu, May 15, 2008 at 12:44:02AM -0400, Jeff King wrote:
> 	> 
> 	> > Hmm, builtin-clone does seem to be completely broken. It looks 
> 	> > like unpack_trees is not doing its job for some reason, but I
> 	> > haven't looked further. But it surprised me that we aren't 
> 	> > testing anything as simple as "clone works". The test below
> 	> > currently fails (though I am getting other test failures which 
> 	> > are presumably related).
> 
> 	I did not notice the breakage earlier, because I ran with a 
> 	"twoway_merge" version of builtin-clone in my personal tree.
> 
>  builtin-clone.c  |    3 +++
>  t/t5601-clone.sh |    7 +++++++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/builtin-clone.c b/builtin-clone.c
> index a7c075d..8713128 100644
> --- a/builtin-clone.c
> +++ b/builtin-clone.c
> @@ -525,7 +525,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  
>  		memset(&opts, 0, sizeof opts);
>  		opts.update = 1;
> +		opts.merge = 1;
> +		opts.fn = oneway_merge;
>  		opts.verbose_update = !option_quiet;
> +		opts.src_index = &the_index;
>  		opts.dst_index = &the_index;
>  
>  		tree = parse_tree_indirect(remote_head->old_sha1);
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index dc9d63d..593d1a3 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -23,4 +23,11 @@ test_expect_success 'clone with excess parameters' '
>  
>  '
>  
> +test_expect_success 'clone checks out files' '
> +
> +	git clone src dst &&
> +	test -f dst/file
> +
> +'
> +
>  test_done
> -- 
> 1.5.5.1.424.g3256b
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] builtin-clone: fix initial checkout

[Kevin Ballard]

On May 15, 2008, at 5:48 AM, Johannes Schindelin wrote:

> Somewhere in the process of finishing up builtin-clone, the update of
> the working tree was lost.  This was due to not using the option  
> "merge"
> for unpack_trees().
>
> Breakage noticed by Kevin Ballard.
>
> Test by Jeff King.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This patch fixes the issue for me. Thanks!

-Kevin Ballard

-- 
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com