[PATCH] submodules: ensure clean environment when operating in a submodule

[PATCH] submodules: ensure clean environment when operating in a submodule

[Giuseppe Bilotta]

git-submodule takes care of clearing GIT_DIR whenever it operates
on a submodule index or configuration, but forgot to unset GIT_WORK_TREE
before operating on the submodule worktree, which would lead to failures
when GIT_WORK_TREE was set.

This only happened in very unusual contexts such as operating on the
main worktree from outside of it, but since "git-gui: set GIT_DIR and
GIT_WORK_TREE after setup" (a9fa11fe5bd5978bb) such failures could also
be provoked by invoking an external tool such as "git submodule update"
from the Git GUI in a standard setup.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-submodule.sh |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

The bug (git submodules not working in Git GUI since my patch) was spotted by
Heiko Voigt working on/with msysgit, and he kindly provided a recipe to
replicate it:
http://article.gmane.org/gmane.comp.version-control.msysgit/8755

I'm pretty confident fixing this on the submodules side is the more correct
approach, since otherwise even a simple
$ GIT_WORK_TREE=. git submodule update
on the command-line can fail.

I also believe this is material for git maint.

diff --git a/git-submodule.sh b/git-submodule.sh
index 5869c00..69afc84 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -223,6 +223,7 @@ cmd_add()
 		module_clone "$path" "$realrepo" "$reference" || exit
 		(
 			unset GIT_DIR
+			unset GIT_WORK_TREE
 			cd "$path" &&
 			# ash fails to wordsplit ${branch:+-b "$branch"...}
 			case "$branch" in
@@ -279,6 +280,7 @@ cmd_foreach()
 			(
 				prefix="$prefix$path/"
 				unset GIT_DIR
+				unset GIT_WORK_TREE
 				cd "$path" &&
 				eval "$@" &&
 				if test -n "$recursive"
@@ -477,7 +479,7 @@ cmd_update()
 				;;
 			esac
 
-			(unset GIT_DIR; cd "$path" && $command "$sha1") ||
+			(unset GIT_DIR; unset GIT_WORK_TREE; cd "$path" && $command "$sha1") ||
 			die "Unable to $action '$sha1' in submodule path '$path'"
 			say "Submodule path '$path': $msg '$sha1'"
 		fi
@@ -771,6 +773,7 @@ cmd_status()
 			(
 				prefix="$displaypath/"
 				unset GIT_DIR
+				unset GIT_WORK_TREE
 				cd "$path" &&
 				cmd_status $orig_args
 			) ||
-- 
1.7.0.199.g49ef3.dirty

Re: [PATCH] submodules: ensure clean environment when operating in a submodule

[Junio C Hamano]

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> I'm pretty confident fixing this on the submodules side is the more correct
> approach, since otherwise even a simple
> $ GIT_WORK_TREE=. git submodule update
> on the command-line can fail.

True; while I didn't bother to check what the codepaths after these
unsetting do, I suspect you should also think about what effect it has to
have other GIT_* environment variables seep through to them (GIT_INDEX_FILE,
GIT_CONFIG and GIT_OBJECT_DIRECTORY come to mind).  You would probably
want to have a single shell helper function to unset even if you end up
deciding that it is sufficient to clear GIT_DIR and GIT_WORK_TREE and
nothing else.

Re: [PATCH] submodules: ensure clean environment when operating in a submodule

[Giuseppe Bilotta]

On Mon, Feb 22, 2010 at 11:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
>> I'm pretty confident fixing this on the submodules side is the more correct
>> approach, since otherwise even a simple
>> $ GIT_WORK_TREE=. git submodule update
>> on the command-line can fail.
>
> True; while I didn't bother to check what the codepaths after these
> unsetting do, I suspect you should also think about what effect it has to
> have other GIT_* environment variables seep through to them (GIT_INDEX_FILE,
> GIT_CONFIG and GIT_OBJECT_DIRECTORY come to mind).  You would probably
> want to have a single shell helper function to unset even if you end up
> deciding that it is sufficient to clear GIT_DIR and GIT_WORK_TREE and
> nothing else.

Good point. All GIT_* env variables should be resent when descending
into a submodule. Is there a way to loop over them, or do I have to do
something horrible like env | grep ^GIT_ | cut -f1 -d=  to get the
list?


-- 
Giuseppe "Oblomov" Bilotta

Re: [PATCH] submodules: ensure clean environment when operating in a submodule

[Junio C Hamano]

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> Good point. All GIT_* env variables should be resent when descending
> into a submodule. Is there a way to loop over them, or do I have to do
> something horrible like env | grep ^GIT_ | cut -f1 -d=  to get the
> list?

I would suggest to enumerate only what you care about explicitly, without
consulting the user's environment.  The user may have variables you do not
know about and not related to git at all.

IOW, don't try to be too clever for your own good ;-)

[PATCH 1/2] shell setup: clear_local_git_env() function

[Giuseppe Bilotta]

Introduce an auxiliary function to clear all repo-local environment
variables. This should be invoked by any shell script that switches
repository during execution, to ensure that the environment is clean
and that things such as the git dir and worktree are set up correctly.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-sh-setup.sh |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a09566..d382879 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -172,6 +172,15 @@ get_author_ident_from_commit () {
 	LANG=C LC_ALL=C sed -ne "$pick_author_script"
 }
 
+# Clear repo-local GIT_* environment variables. Useful when switching to
+# another repository (e.g. when entering a submodule)
+clear_local_git_env() {
+	unset	GIT_DIR GIT_WORKTREE GIT_OBJECT_DIRECTORY \
+		GIT_INDEX_FILE GIT_GRAFT_FILE GIT_CONFIG \
+		GIT_NO_REPLACE_OBJECTS
+
+}
+
 # Make sure we are in a valid repository of a vintage we understand,
 # if we require to be in a git repository.
 if test -z "$NONGIT_OK"
-- 
1.7.0.200.g5ba36.dirty

Re: [PATCH 1/2] shell setup: clear_local_git_env() function

[Johannes Sixt]

Giuseppe Bilotta schrieb:
> +# Clear repo-local GIT_* environment variables. Useful when switching to
> +# another repository (e.g. when entering a submodule)
> +clear_local_git_env() {
> +	unset	GIT_DIR GIT_WORKTREE GIT_OBJECT_DIRECTORY \
> +		GIT_INDEX_FILE GIT_GRAFT_FILE GIT_CONFIG \
> +		GIT_NO_REPLACE_OBJECTS

IMO, this list should be in sync with the one you find in
connect.c:git_connect() around line 611. They have the same purpose.

(And, BTW, a vertical list would be more readable than a mixed
horizontal+vertical list, IMVHO.)

-- Hannes

Re: [PATCH 1/2] shell setup: clear_local_git_env() function

[Giuseppe Bilotta]

On Tue, Feb 23, 2010 at 7:49 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Giuseppe Bilotta schrieb:
>> +# Clear repo-local GIT_* environment variables. Useful when switching to
>> +# another repository (e.g. when entering a submodule)
>> +clear_local_git_env() {
>> +     unset   GIT_DIR GIT_WORKTREE GIT_OBJECT_DIRECTORY \
>> +             GIT_INDEX_FILE GIT_GRAFT_FILE GIT_CONFIG \
>> +             GIT_NO_REPLACE_OBJECTS
>
> IMO, this list should be in sync with the one you find in
> connect.c:git_connect() around line 611. They have the same purpose.

Ah, interesting, I was looking for such a list but only found the more
generic one in cache.h
By comparing them it would seem they serve the same purpose, indeed. I
notice that the connect.c is missing GIT_CONFIG (which _must_ be unset
for us). I also notice that the connect.c one unsets the alternate DB;
I had doubts about it when preparing the list in this case.

I will resend a new patch to replace this one, syncing the two lists.

> (And, BTW, a vertical list would be more readable than a mixed
> horizontal+vertical list, IMVHO.)

I tend to conserve vertical space, but I have no particular objection to that.

-- 
Giuseppe "Oblomov" Bilotta

[PATCHv2 0/2] clear environment for submodules

[Giuseppe Bilotta]

Second (or maybe actually third) iteration of the series to ensure that
submodules work correctly when repo-local environment is set. The only
thing changed from the previous iteration is an extra variable in
clear_local_git_env() to bring it up in sync with the list in git_connect(),
and also the addition of CONFIG_ENVIRONMENT in git_connect() itself.

I had considered making a static list of these variables somewhere, but
making it accessible to both the built-ins and shell scripts (something à
la git rev-parse --local-git-env maybe?) is a little over my head at the
moment (not much free time to dedicate to git, sadly). Also I'm not sure
the extra git call needed to get the list would be worth it.

Giuseppe Bilotta (2):
  shell setup: clear_local_git_env() function
  submodules: ensure clean environment when operating in a submodule

 connect.c        |    2 ++
 git-sh-setup.sh  |   15 +++++++++++++++
 git-submodule.sh |   20 ++++++++++----------
 3 files changed, 27 insertions(+), 10 deletions(-)

[PATCHv2 1/2] shell setup: clear_local_git_env() function

[Giuseppe Bilotta]

Introduce an auxiliary function to clear all repo-local environment
variables. This should be invoked by any shell script that switches
repository during execution, to ensure that the environment is clean
and that things such as the git dir and worktree are set up correctly.

The list matches the one in git_connect(), so bring them in sync by adding
the missing CONFIG_ENVIRONMENT. Also add a note about the fact that they
should be kept that way.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 connect.c       |    2 ++
 git-sh-setup.sh |   15 +++++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/connect.c b/connect.c
index 9f39038..12dd0b5 100644
--- a/connect.c
+++ b/connect.c
@@ -583,8 +583,10 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	}
 	else {
 		/* remove these from the environment */
+		/* see also clear_local_git_env() in git-sh-setup.sh */
 		const char *env[] = {
 			ALTERNATE_DB_ENVIRONMENT,
+			CONFIG_ENVIRONMENT,
 			DB_ENVIRONMENT,
 			GIT_DIR_ENVIRONMENT,
 			GIT_WORK_TREE_ENVIRONMENT,
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a09566..f1be832 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -172,6 +172,21 @@ get_author_ident_from_commit () {
 	LANG=C LC_ALL=C sed -ne "$pick_author_script"
 }
 
+# Clear repo-local GIT_* environment variables. Useful when switching to
+# another repository (e.g. when entering a submodule). See also the env
+# list in git_connect()
+clear_local_git_env() {
+	unset	GIT_ALTERNATE_OBJECT_DIRECTORIES \
+		GIT_CONFIG \
+		GIT_DIR \
+		GIT_GRAFT_FILE \
+		GIT_INDEX_FILE \
+		GIT_NO_REPLACE_OBJECTS \
+		GIT_OBJECT_DIRECTORY \
+		GIT_WORKTREE
+
+}
+
 # Make sure we are in a valid repository of a vintage we understand,
 # if we require to be in a git repository.
 if test -z "$NONGIT_OK"
-- 
1.7.0.200.g5ba36.dirty

Re: [PATCHv2 1/2] shell setup: clear_local_git_env() function

[Jens Lehmann]

Am 23.02.2010 09:30, schrieb Giuseppe Bilotta:
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 7a09566..f1be832 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -172,6 +172,21 @@ get_author_ident_from_commit () {
>  	LANG=C LC_ALL=C sed -ne "$pick_author_script"
>  }
>  
> +# Clear repo-local GIT_* environment variables. Useful when switching to
> +# another repository (e.g. when entering a submodule). See also the env
> +# list in git_connect()
> +clear_local_git_env() {
> +	unset	GIT_ALTERNATE_OBJECT_DIRECTORIES \
> +		GIT_CONFIG \
> +		GIT_DIR \
> +		GIT_GRAFT_FILE \
> +		GIT_INDEX_FILE \
> +		GIT_NO_REPLACE_OBJECTS \
> +		GIT_OBJECT_DIRECTORY \
> +		GIT_WORKTREE

Shouldn't that last one be GIT_WORK_TREE?


> +
> +}
> +
>  # Make sure we are in a valid repository of a vintage we understand,
>  # if we require to be in a git repository.
>  if test -z "$NONGIT_OK"

[PATCHv2 2/2] submodules: ensure clean environment when operating in a submodule

[Giuseppe Bilotta]

git-submodule used to take care of clearing GIT_DIR whenever it operated
on a submodule index or configuration, but forgot to unset GIT_WORK_TREE
or other repo-local variables. This which would lead to failures e.g.
when GIT_WORK_TREE was set.

This only happened in very unusual contexts such as operating on the
main worktree from outside of it, but since "git-gui: set GIT_DIR and
GIT_WORK_TREE after setup" (a9fa11fe5bd5978bb) such failures could also
be provoked by invoking an external tool such as "git submodule update"
from the Git Gui in a standard setup.

Solve by using the newly introduced clear_local_git_env() shell function
to ensure that all repo-local environment variables are unset.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-submodule.sh |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 5869c00..1ea4143 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -222,7 +222,7 @@ cmd_add()
 
 		module_clone "$path" "$realrepo" "$reference" || exit
 		(
-			unset GIT_DIR
+			clear_local_git_env
 			cd "$path" &&
 			# ash fails to wordsplit ${branch:+-b "$branch"...}
 			case "$branch" in
@@ -278,7 +278,7 @@ cmd_foreach()
 			name=$(module_name "$path")
 			(
 				prefix="$prefix$path/"
-				unset GIT_DIR
+				clear_local_git_env
 				cd "$path" &&
 				eval "$@" &&
 				if test -n "$recursive"
@@ -434,7 +434,7 @@ cmd_update()
 			module_clone "$path" "$url" "$reference"|| exit
 			subsha1=
 		else
-			subsha1=$(unset GIT_DIR; cd "$path" &&
+			subsha1=$(clear_local_git_env; cd "$path" &&
 				git rev-parse --verify HEAD) ||
 			die "Unable to find current revision in submodule path '$path'"
 		fi
@@ -454,7 +454,7 @@ cmd_update()
 
 			if test -z "$nofetch"
 			then
-				(unset GIT_DIR; cd "$path" &&
+				(clear_local_git_env; cd "$path" &&
 					git-fetch) ||
 				die "Unable to fetch in submodule path '$path'"
 			fi
@@ -477,14 +477,14 @@ cmd_update()
 				;;
 			esac
 
-			(unset GIT_DIR; cd "$path" && $command "$sha1") ||
+			(clear_local_git_env; cd "$path" && $command "$sha1") ||
 			die "Unable to $action '$sha1' in submodule path '$path'"
 			say "Submodule path '$path': $msg '$sha1'"
 		fi
 
 		if test -n "$recursive"
 		then
-			(unset GIT_DIR; cd "$path" && cmd_update $orig_args) ||
+			(clear_local_git_env; cd "$path" && cmd_update $orig_args) ||
 			die "Failed to recurse into submodule path '$path'"
 		fi
 	done
@@ -492,7 +492,7 @@ cmd_update()
 
 set_name_rev () {
 	revname=$( (
-		unset GIT_DIR
+		clear_local_git_env
 		cd "$1" && {
 			git describe "$2" 2>/dev/null ||
 			git describe --tags "$2" 2>/dev/null ||
@@ -760,7 +760,7 @@ cmd_status()
 		else
 			if test -z "$cached"
 			then
-				sha1=$(unset GIT_DIR; cd "$path" && git rev-parse --verify HEAD)
+				sha1=$(clear_local_git_env; cd "$path" && git rev-parse --verify HEAD)
 				set_name_rev "$path" "$sha1"
 			fi
 			say "+$sha1 $displaypath$revname"
@@ -770,7 +770,7 @@ cmd_status()
 		then
 			(
 				prefix="$displaypath/"
-				unset GIT_DIR
+				clear_local_git_env
 				cd "$path" &&
 				cmd_status $orig_args
 			) ||
@@ -821,7 +821,7 @@ cmd_sync()
 		if test -e "$path"/.git
 		then
 		(
-			unset GIT_DIR
+			clear_local_git_env
 			cd "$path"
 			remote=$(get_default_remote)
 			say "Synchronizing submodule url for '$name'"
-- 
1.7.0.200.g5ba36.dirty

[PATCH 2/2] submodules: ensure clean environment when operating in a submodule

[Giuseppe Bilotta]

git-submodule used to take care of clearing GIT_DIR whenever it operated
on a submodule index or configuration, but forgot to unset GIT_WORK_TREE
or other repo-local variables. This which would lead to failures e.g.
when GIT_WORK_TREE was set.

This only happened in very unusual contexts such as operating on the
main worktree from outside of it, but since "git-gui: set GIT_DIR and
GIT_WORK_TREE after setup" (a9fa11fe5bd5978bb) such failures could also
be provoked by invoking an external tool such as "git submodule update"
from the Git Gui in a standard setup.

Solve by using the newly introduced clear_local_git_env() shell function
to ensure that all repo-local environment variables are unset.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-submodule.sh |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 5869c00..1ea4143 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -222,7 +222,7 @@ cmd_add()
 
 		module_clone "$path" "$realrepo" "$reference" || exit
 		(
-			unset GIT_DIR
+			clear_local_git_env
 			cd "$path" &&
 			# ash fails to wordsplit ${branch:+-b "$branch"...}
 			case "$branch" in
@@ -278,7 +278,7 @@ cmd_foreach()
 			name=$(module_name "$path")
 			(
 				prefix="$prefix$path/"
-				unset GIT_DIR
+				clear_local_git_env
 				cd "$path" &&
 				eval "$@" &&
 				if test -n "$recursive"
@@ -434,7 +434,7 @@ cmd_update()
 			module_clone "$path" "$url" "$reference"|| exit
 			subsha1=
 		else
-			subsha1=$(unset GIT_DIR; cd "$path" &&
+			subsha1=$(clear_local_git_env; cd "$path" &&
 				git rev-parse --verify HEAD) ||
 			die "Unable to find current revision in submodule path '$path'"
 		fi
@@ -454,7 +454,7 @@ cmd_update()
 
 			if test -z "$nofetch"
 			then
-				(unset GIT_DIR; cd "$path" &&
+				(clear_local_git_env; cd "$path" &&
 					git-fetch) ||
 				die "Unable to fetch in submodule path '$path'"
 			fi
@@ -477,14 +477,14 @@ cmd_update()
 				;;
 			esac
 
-			(unset GIT_DIR; cd "$path" && $command "$sha1") ||
+			(clear_local_git_env; cd "$path" && $command "$sha1") ||
 			die "Unable to $action '$sha1' in submodule path '$path'"
 			say "Submodule path '$path': $msg '$sha1'"
 		fi
 
 		if test -n "$recursive"
 		then
-			(unset GIT_DIR; cd "$path" && cmd_update $orig_args) ||
+			(clear_local_git_env; cd "$path" && cmd_update $orig_args) ||
 			die "Failed to recurse into submodule path '$path'"
 		fi
 	done
@@ -492,7 +492,7 @@ cmd_update()
 
 set_name_rev () {
 	revname=$( (
-		unset GIT_DIR
+		clear_local_git_env
 		cd "$1" && {
 			git describe "$2" 2>/dev/null ||
 			git describe --tags "$2" 2>/dev/null ||
@@ -760,7 +760,7 @@ cmd_status()
 		else
 			if test -z "$cached"
 			then
-				sha1=$(unset GIT_DIR; cd "$path" && git rev-parse --verify HEAD)
+				sha1=$(clear_local_git_env; cd "$path" && git rev-parse --verify HEAD)
 				set_name_rev "$path" "$sha1"
 			fi
 			say "+$sha1 $displaypath$revname"
@@ -770,7 +770,7 @@ cmd_status()
 		then
 			(
 				prefix="$displaypath/"
-				unset GIT_DIR
+				clear_local_git_env
 				cd "$path" &&
 				cmd_status $orig_args
 			) ||
@@ -821,7 +821,7 @@ cmd_sync()
 		if test -e "$path"/.git
 		then
 		(
-			unset GIT_DIR
+			clear_local_git_env
 			cd "$path"
 			remote=$(get_default_remote)
 			say "Synchronizing submodule url for '$name'"
-- 
1.7.0.200.g5ba36.dirty

Re: [msysGit] [PATCH] submodules: ensure clean environment when operating in a submodule

[Johannes Schindelin]

Hi,

On Mon, 22 Feb 2010, Giuseppe Bilotta wrote:

> git-submodule takes care of clearing GIT_DIR whenever it operates
> on a submodule index or configuration, but forgot to unset GIT_WORK_TREE
> before operating on the submodule worktree, which would lead to failures
> when GIT_WORK_TREE was set.
> 
> This only happened in very unusual contexts such as operating on the
> main worktree from outside of it, but since "git-gui: set GIT_DIR and
> GIT_WORK_TREE after setup" (a9fa11fe5bd5978bb) such failures could also
> be provoked by invoking an external tool such as "git submodule update"
> from the Git GUI in a standard setup.
> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---

Heiko, would you say that this patch is an elegant solution to the problem 
you reported? If so, would you please pull it to 4msysgit.git's devel?

Thanks,
Dscho

Re: Re: [msysGit] [PATCH] submodules: ensure clean environment when operating in a submodule

[Heiko Voigt]

On Tue, Feb 23, 2010 at 01:04:00AM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 22 Feb 2010, Giuseppe Bilotta wrote:
> 
> > git-submodule takes care of clearing GIT_DIR whenever it operates
> > on a submodule index or configuration, but forgot to unset GIT_WORK_TREE
> > before operating on the submodule worktree, which would lead to failures
> > when GIT_WORK_TREE was set.
> > 
> > This only happened in very unusual contexts such as operating on the
> > main worktree from outside of it, but since "git-gui: set GIT_DIR and
> > GIT_WORK_TREE after setup" (a9fa11fe5bd5978bb) such failures could also
> > be provoked by invoking an external tool such as "git submodule update"
> > from the Git GUI in a standard setup.
> > 
> > Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> > ---
> 
> Heiko, would you say that this patch is an elegant solution to the problem 
> you reported? If so, would you please pull it to 4msysgit.git's devel?

I would like to leave the patch cooking a little more. For example see
the objection of Jens[1]. Just to be sure that we do not pull in other
issues with this. Sorry I do not have the time for extensive testing
currently. So my suggestion would be to leave msysgit in the current
state as I did not discover any downsides with just the revert with my
current users tests.

As far as I understand the patch fixes issues when submodules itself
contain submodules and it seems that not many users are doing this
currently. Once everybody is happy (and tried) the patches I do not have
any objections to pushing it into devel.

What do you think?

cheers Heiko

[1] http://groups.google.com/group/msysgit/msg/481e252ba4f49ede

Re: Re: [msysGit] [PATCH] submodules: ensure clean environment when operating in a submodule

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Heiko Voigt wrote:

> On Tue, Feb 23, 2010 at 01:04:00AM +0100, Johannes Schindelin wrote:
> 
> > On Mon, 22 Feb 2010, Giuseppe Bilotta wrote:
> > 
> > > git-submodule takes care of clearing GIT_DIR whenever it operates on 
> > > a submodule index or configuration, but forgot to unset 
> > > GIT_WORK_TREE before operating on the submodule worktree, which 
> > > would lead to failures when GIT_WORK_TREE was set.
> > > 
> > > This only happened in very unusual contexts such as operating on the 
> > > main worktree from outside of it, but since "git-gui: set GIT_DIR 
> > > and GIT_WORK_TREE after setup" (a9fa11fe5bd5978bb) such failures 
> > > could also be provoked by invoking an external tool such as "git 
> > > submodule update" from the Git GUI in a standard setup.
> > 
> > Heiko, would you say that this patch is an elegant solution to the 
> > problem you reported? If so, would you please pull it to 
> > 4msysgit.git's devel?
> 
> I would like to leave the patch cooking a little more.

Yes, that matches my feeling.

Thanks,
Dscho