[PATCH] Fix 'git checkout <submodule>' to update the index

[PATCH] Fix 'git checkout <submodule>' to update the index

[Johannes Schindelin]

While 'git checkout <submodule>' should not update the submodule's
working directory, it should update the index.  This is in line with
how submodules are handled in the rest of Git.

While at it, test 'git reset [<commit>] <submodule>', too.

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

	I have no idea who thought that it would be a good idea to leave 
	the submodules alone, but I think it is highly counterintuitive 
	that

		git checkout <submodule>

	does not touch the index.

	This patch fixes it.

	I'm really swamped with work these days, but happily, this fix was 
	part of it: one of my users had a merge conflict in a submodule 
	and wanted to use the same procedure as for files.

	In the meantime, "git reset <submodule>" did the job, but this 
	rather limits the user's options:

		git checkout --theirs <submodule>

	is a much desired function here.  (Note: I did not test that this 
	works, actually, but then, I do not see why it should not, after
	this patch...)

 builtin-checkout.c            |    3 --
 t/t2013-checkout-submodule.sh |   42 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)
 create mode 100755 t/t2013-checkout-submodule.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 33d1fec..2e4fe1a 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -53,9 +53,6 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
 	int len;
 	struct cache_entry *ce;
 
-	if (S_ISGITLINK(mode))
-		return 0;
-
 	if (S_ISDIR(mode))
 		return READ_TREE_RECURSIVE;
 
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
new file mode 100755
index 0000000..fda3f0a
--- /dev/null
+++ b/t/t2013-checkout-submodule.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='checkout can handle submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	mkdir submodule &&
+	(cd submodule &&
+	 git init &&
+	 test_commit first) &&
+	git add submodule &&
+	test_tick &&
+	git commit -m superproject &&
+	(cd submodule &&
+	 test_commit second) &&
+	git add submodule &&
+	test_tick &&
+	git commit -m updated.superproject
+'
+
+test_expect_success '"reset <submodule>" updates the index' '
+	git update-index --refresh &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD &&
+	test_must_fail git reset HEAD^ submodule &&
+	test_must_fail git diff-files --quiet &&
+	git reset submodule &&
+	git diff-files --quiet
+'
+
+test_expect_success '"checkout <submodule>" updates the index only' '
+	git update-index --refresh &&
+	git diff-files --quiet &&
+	git diff-index --quiet --cached HEAD &&
+	git checkout HEAD^ submodule &&
+	test_must_fail git diff-files --quiet &&
+	git checkout HEAD submodule &&
+	git diff-files --quiet
+'
+
+test_done
-- 
1.6.2.1.493.g67cf3

Re: [PATCH] Fix 'git checkout <submodule>' to update the index

[Junio C Hamano]

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

> While 'git checkout <submodule>' should not update the submodule's
> working directory, it should update the index.  This is in line with
> how submodules are handled in the rest of Git.
>
> While at it, test 'git reset [<commit>] <submodule>', too.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	I have no idea who thought that it would be a good idea to leave 
> 	the submodules alone, but I think it is highly counterintuitive 
> 	that
>
> 		git checkout <submodule>
>
> 	does not touch the index.

I do not see anything that treats submodule specially in the scripted
version and this check for S_ISGITLINK() has been there since the very
beginning of C rewrite 782c2d6 (Build in checkout, 2008-02-07), so it is a
regression fix that can go to maint (all the way down to 1.5.5 if you
want).

Thanks.

Re: [PATCH] Fix 'git checkout <submodule>' to update the index

[Johannes Schindelin]

Hi,

On Thu, 2 Apr 2009, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > While 'git checkout <submodule>' should not update the submodule's
> > working directory, it should update the index.  This is in line with
> > how submodules are handled in the rest of Git.
> >
> > While at it, test 'git reset [<commit>] <submodule>', too.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >
> > 	I have no idea who thought that it would be a good idea to leave 
> > 	the submodules alone, but I think it is highly counterintuitive 
> > 	that
> >
> > 		git checkout <submodule>
> >
> > 	does not touch the index.
> 
> I do not see anything that treats submodule specially in the scripted 
> version and this check for S_ISGITLINK() has been there since the very 
> beginning of C rewrite 782c2d6 (Build in checkout, 2008-02-07), so it is 
> a regression fix that can go to maint (all the way down to 1.5.5 if you 
> want).

I would appreciate that.

Thanks,
Dscho