[PATCH 1/3] run-command: optionally clear git environment

[PATCH 1/3] run-command: optionally clear git environment

[skimo]

From: Sven Verdoolaege <skimo@kotnet.org>

Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
---
 run-command.c |    6 ++++++
 run-command.h |    1 +
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/run-command.c b/run-command.c
index 7e779d3..5c47f45 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,6 +2,10 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 
+static const char* git_env_list[] = { ALTERNATE_DB_ENVIRONMENT, DB_ENVIRONMENT,
+			CONFIG_ENVIRONMENT, GIT_DIR_ENVIRONMENT,
+			GRAFT_ENVIRONMENT, INDEX_ENVIRONMENT, NULL };
+
 static inline void close_pair(int fd[2])
 {
 	close(fd[0]);
@@ -153,6 +157,8 @@ static void prepare_run_command_v_opt(struct child_process *cmd,
 	cmd->no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;
 	cmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
 	cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
+	if (opt & RUN_COMMAND_CLEAR_GIT_ENV)
+		cmd->env = git_env_list;
 }
 
 int run_command_v_opt(const char **argv, int opt)
diff --git a/run-command.h b/run-command.h
index 7958eb1..a5374cc 100644
--- a/run-command.h
+++ b/run-command.h
@@ -33,6 +33,7 @@ int run_command(struct child_process *);
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
 #define RUN_COMMAND_STDOUT_TO_STDERR 4
+#define RUN_COMMAND_CLEAR_GIT_ENV	(1 << 3)
 int run_command_v_opt(const char **argv, int opt);
 int run_command_v_opt_cd(const char **argv, int opt, const char *dir);
 
-- 
1.5.2.838.gbeec

[PATCH 2/3] entry.c: checkout available submodules

[skimo]

From: Sven Verdoolaege <skimo@kotnet.org>

That is, checkout any submodule that has a valid HEAD in it.

Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
---
 Makefile     |    5 +++--
 entry.c      |   30 ++++++++++++++++++++++++++++--
 submodules.c |    8 ++++++++
 submodules.h |    6 ++++++
 4 files changed, 45 insertions(+), 4 deletions(-)
 create mode 100644 submodules.c
 create mode 100644 submodules.h

diff --git a/Makefile b/Makefile
index c79a6da..6d24048 100644
--- a/Makefile
+++ b/Makefile
@@ -297,7 +297,7 @@ LIB_H = \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
 	utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
-	mailmap.h remote.h
+	mailmap.h remote.h submodules.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -319,7 +319,8 @@ LIB_OBJS = \
 	write_or_die.o trace.o list-objects.o grep.o match-trees.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
 	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
-	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o
+	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
+	submodules.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/entry.c b/entry.c
index ae64764..97f95c6 100644
--- a/entry.c
+++ b/entry.c
@@ -1,5 +1,7 @@
 #include "cache.h"
 #include "blob.h"
+#include "run-command.h"
+#include "submodules.h"
 
 static void create_directories(const char *path, const struct checkout *state)
 {
@@ -75,6 +77,31 @@ static void *read_blob_entry(struct cache_entry *ce, const char *path, unsigned
 	return NULL;
 }
 
+static int checkout_submodule(struct cache_entry *ce, const char *path, const struct checkout *state)
+{
+	const char *args[10];
+	int argc;
+	int err;
+
+	if (!is_checkedout_submodule(ce->name))
+		return 0;
+
+	argc = 0;
+	args[argc++] = "checkout";
+	if (state->force)
+	    args[argc++] = "-f";
+	args[argc++] = sha1_to_hex(ce->sha1);
+	args[argc] = NULL;
+
+	err = run_command_v_opt_cd(args, RUN_GIT_CMD|RUN_COMMAND_CLEAR_GIT_ENV,
+				   path);
+
+	if (err)
+		return error("failed to run git-checkout in submodule '%s'", path);
+
+	return 0;
+}
+
 static int write_entry(struct cache_entry *ce, char *path, const struct checkout *state, int to_tempfile)
 {
 	int fd;
@@ -193,9 +220,8 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
 		 */
 		unlink(path);
 		if (S_ISDIR(st.st_mode)) {
-			/* If it is a gitlink, leave it alone! */
 			if (S_ISGITLINK(ntohl(ce->ce_mode)))
-				return 0;
+				return checkout_submodule(ce, path, state);
 			if (!state->force)
 				return error("%s is a directory", path);
 			remove_subtree(path);
diff --git a/submodules.c b/submodules.c
new file mode 100644
index 0000000..5baf90a
--- /dev/null
+++ b/submodules.c
@@ -0,0 +1,8 @@
+#include "cache.h"
+#include "refs.h"
+
+int is_checkedout_submodule(const char *path)
+{
+	unsigned char sha1[20];
+	return resolve_gitlink_ref(path, "HEAD", sha1) == 0;
+}
diff --git a/submodules.h b/submodules.h
new file mode 100644
index 0000000..099c4c3
--- /dev/null
+++ b/submodules.h
@@ -0,0 +1,6 @@
+#ifndef SUBMODULES_H
+#define SUBMODULES_H
+
+int is_checkedout_submodule(const char *path);
+
+#endif
-- 
1.5.2.838.gbeec

Re: [PATCH 2/3] entry.c: checkout available submodules

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]

hoi :)

On Fri, May 25, 2007 at 11:07:12PM +0200, skimo@liacs.nl wrote:
>  create mode 100644 submodules.c
>  create mode 100644 submodules.h

I think the list tends to prefer subproject over submodule.

> @@ -193,9 +220,8 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
>  		 */
>  		unlink(path);
>  		if (S_ISDIR(st.st_mode)) {
> -			/* If it is a gitlink, leave it alone! */
>  			if (S_ISGITLINK(ntohl(ce->ce_mode)))
> -				return 0;
> +				return checkout_submodule(ce, path, state);
>  			if (!state->force)
>  				return error("%s is a directory", path);
>  			remove_subtree(path);

I think the call to checkout_submodule should be moved to write_entry,
to keep it in line with the other mode types.

Aside from that I really like it :-)

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] entry.c: checkout available submodules

[Sven Verdoolaege]

On Fri, May 25, 2007 at 11:31:03PM +0200, Martin Waitz wrote:
> I think the list tends to prefer subproject over submodule.

Does it?  It seems that everyone writing code is use submodule
instead of subproject.  Either way, I don't really care.

> > @@ -193,9 +220,8 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
> >  		 */
> >  		unlink(path);
> >  		if (S_ISDIR(st.st_mode)) {
> > -			/* If it is a gitlink, leave it alone! */
> >  			if (S_ISGITLINK(ntohl(ce->ce_mode)))
> > -				return 0;
> > +				return checkout_submodule(ce, path, state);
> >  			if (!state->force)
> >  				return error("%s is a directory", path);
> >  			remove_subtree(path);
> 
> I think the call to checkout_submodule should be moved to write_entry,
> to keep it in line with the other mode types.

Well, like your patch, this only deals with cases where the submodule
is already available.  In write_entry you could potentially clone
submodules based on some criteria, but I'm not doing this just yet
since some people apparently prefer to get these things in pieces.

Also, it seems that some people would like this to be a step
that is separated from git-checkout (see Lars' patch).

skimo

Re: [PATCH 2/3] entry.c: checkout available submodules

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 1985 bytes --]

hoi :)

On Fri, May 25, 2007 at 11:42:05PM +0200, Sven Verdoolaege wrote:
> On Fri, May 25, 2007 at 11:31:03PM +0200, Martin Waitz wrote:
> > I think the list tends to prefer subproject over submodule.
> 
> Does it?  It seems that everyone writing code is use submodule
> instead of subproject.  Either way, I don't really care.

I got that impression from my small poll.

> > > @@ -193,9 +220,8 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
> > >  		 */
> > >  		unlink(path);
> > >  		if (S_ISDIR(st.st_mode)) {
> > > -			/* If it is a gitlink, leave it alone! */
> > >  			if (S_ISGITLINK(ntohl(ce->ce_mode)))
> > > -				return 0;
> > > +				return checkout_submodule(ce, path, state);
> > >  			if (!state->force)
> > >  				return error("%s is a directory", path);
> > >  			remove_subtree(path);
> > 
> > I think the call to checkout_submodule should be moved to write_entry,
> > to keep it in line with the other mode types.
> 
> Well, like your patch, this only deals with cases where the submodule
> is already available.  In write_entry you could potentially clone
> submodules based on some criteria, but I'm not doing this just yet
> since some people apparently prefer to get these things in pieces.

yes, first we need checkout and then can add more building blocks on
top.

Up to now the quoted code block above only handles cleaning the
tree from conflicting / old entries and write_entry creates the
real content.

For subprojects we first have to remove any non-subproject content
in that location and then later call write_subproject or similiar
in write_entry to update the subproject (or create some empty dummy
directory).

But we can also leave those details for later when we are clear about
the complete semantics.  At the moment it is important to reach a
common base everybody agrees on and which is enough to experiment with
all the high level tools.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] entry.c: checkout available submodules

[Junio C Hamano]

Martin Waitz <tali@admingilde.org> writes:

> But we can also leave those details for later when we are clear about
> the complete semantics.  At the moment it is important to reach a
> common base everybody agrees on and which is enough to experiment with
> all the high level tools.

Overall I am reasonably happy with the direction these "smaller"
patches take us, although I suspect the semantics implemented by
this series _might_ need to be scrapped when we start talking
about switching between branches that has and does not have a
subproject at that path, and other corner cases we do not forsee
right now.

I think we are Ok, as long as we make it is clear that we
currently do not support switching from a commit that has a
submodule at one path to another commit that does not (in which
case with a naive implementation we would end up having to nuke
the submodule, and we need to have a way to save it, which we
discussed yesterday, with .git/subproject/$name.git/ being the
stashed away mirror to either quick-clone from, or symlink to).
And more importantly, we would need to make it crystal clear
that the superproject support by the Porcelain layer is still
experimental and is subject to change in potentially backward
incompatible way.  We haven't had enough experience to decide
the best semantics from day one, and experience cannot be gained
without playing with something small like this series anyway.

We may come up with a much superior design after gaining the
experience, and if that is incompatible with the layout this
series assumes, so be it.  We'd have a big feature release that
changes the semantics and that will incur some transition pain,
but overall we would be better off with the final result.

A clear separation of the superproject and the projects it uses
as its submodules helps us here.  At the worst case, migrating
to the updated layout in the future would involve moving .git
directories around in the checked out tree and perhaps making
symlinks and/or setting up various .git/config files by hand to
imitate what the final toolset would have done for the user,
which should be manageable.

[PATCH 3/3] test for simple submodule checkout support

[skimo]

From: Martin Waitz <tali@admingilde.org>

Signed-off-by: Martin Waitz <tali@admingilde.org>
Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
---
 t/t3041-subprojects-checkout.sh |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
 create mode 100755 t/t3041-subprojects-checkout.sh

diff --git a/t/t3041-subprojects-checkout.sh b/t/t3041-subprojects-checkout.sh
new file mode 100755
index 0000000..4b3cea9
--- /dev/null
+++ b/t/t3041-subprojects-checkout.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+test_description='submodule checkout'
+. ./test-lib.sh
+
+test_expect_success 'submodule creation' \
+    '(mkdir A && cd A &&
+      git init &&
+      echo 1 > a &&
+      git add a &&
+      git commit -m "create submodule" || exit $? )'
+
+test_expect_success 'Super module creation' \
+    'git add A &&
+     git commit -m "supermodule creation" &&
+     git branch one'
+
+test_expect_success 'submodule change' \
+    '(cd A &&
+      echo 2 > a &&
+      git add a &&
+      git commit -m "create submodule" || exit $? )'
+
+test_expect_success 'supermodule change' \
+    'git add A &&
+     git commit -m "supermodule creation"'
+
+test_expect_success 'supermodule switching branch' \
+    'git checkout one &&
+     echo 1 > expected &&
+     git diff expected A/a'
+
+test_expect_success 'supermodule reset' \
+    'git reset --hard master &&
+     echo 2 > expected &&
+     git diff expected A/a'
+
+
+test_done
-- 
1.5.2.838.gbeec