threads / patch / 12495

patchgit-submodule: Don't die when command fails for one submodule

Subject: [PATCH] git-submodule: Don't die when command fails for one submodule

## tl;dr

7 messages between Mar 4, 2008 and Mar 4, 2008. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Ping Yin· Mar 4, 2008, 14:35 UTC · lore

When handling multiple modules, init/update/status command will exit when it fails for one submodule. This patch makes the command continue bypassing the failure.

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 git-submodule.sh |   51 +++++++++++++++++++++++++++++++--------------------
 1 files changed, 31 insertions(+), 20 deletions(-)
Show changes to git-submodule.sh +31 −20
diff --git a/git-submodule.sh b/git-submodule.sh
index 67d3224..2d1639c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -100,15 +100,18 @@ module_clone()
 	# succeed but the rmdir will fail. We might want to fix this.
 	if test -d "$path"
 	then
-		rmdir "$path" 2>/dev/null ||
-		die "Directory '$path' exist, but is neither empty nor a git repository"
+		! rmdir "$path" 2>/dev/null &&
+		say "Directory '$path' exist, but is neither empty nor a git repository" &&
+		return 1
 	fi
 
 	test -e "$path" &&
-	die "A file already exist at path '$path'"
+	say "A file already exist at path '$path'" &&
+	return 1
 
-	git-clone -n "$url" "$path" ||
-	die "Clone of '$url' into submodule path '$path' failed"
+	! git-clone -n "$url" "$path" &&
+	say "Clone of '$url' into submodule path '$path' failed" &&
+	return 1
 }
 
 #
@@ -224,13 +227,14 @@ cmd_init()
 	while read mode sha1 stage path
 	do
 		# Skip already registered paths
-		name=$(module_name "$path") || exit
+		name=$(module_name "$path") || continue
 		url=$(git config submodule."$name".url)
 		test -z "$url" || continue
 
 		url=$(GIT_CONFIG=.gitmodules git config submodule."$name".url)
 		test -z "$url" &&
-		die "No url found for submodule path '$path' in .gitmodules"
+		say "No url found for submodule path '$path' in .gitmodules" &&
+		continue
 
 		# Possibly a url relative to parent
 		case "$url" in
@@ -239,10 +243,13 @@ cmd_init()
 			;;
 		esac
 
-		git config submodule."$name".url "$url" ||
-		die "Failed to register url for submodule path '$path'"
+		if git config submodule."$name".url "$url"
+		then
+			say "Submodule '$name' ($url) registered for path '$path'"
+		else
+			say "Failed to register url for submodule path '$path'"
+		fi
 
-		say "Submodule '$name' ($url) registered for path '$path'"
 	done
 }
 
@@ -277,7 +284,7 @@ cmd_update()
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do
-		name=$(module_name "$path") || exit
+		name=$(module_name "$path") || continue
 		url=$(git config submodule."$name".url)
 		if test -z "$url"
 		then
@@ -290,21 +297,25 @@ cmd_update()
 
 		if ! test -d "$path"/.git
 		then
-			module_clone "$path" "$url" || exit
+			module_clone "$path" "$url" || continue
 			subsha1=
 		else
-			subsha1=$(unset GIT_DIR; cd "$path" &&
-				git rev-parse --verify HEAD) ||
-			die "Unable to find current revision in submodule path '$path'"
+			! subsha1=$(unset GIT_DIR; cd "$path" &&
+				git rev-parse --verify HEAD 2>/dev/null) &&
+			say "Unable to find current revision in submodule path '$path'" &&
+			continue
 		fi
 
 		if test "$subsha1" != "$sha1"
 		then
-			(unset GIT_DIR; cd "$path" && git-fetch &&
-				git-checkout -q "$sha1") ||
-			die "Unable to checkout '$sha1' in submodule path '$path'"
+			if (unset GIT_DIR; cd "$path" && git-fetch &&
+				git-checkout -q "$sha1")
+			then
+				say "Submodule path '$path': checked out '$sha1'"
+			else
+				say "Unable to checkout '$sha1' in submodule path '$path'"
+			fi
 
-			say "Submodule path '$path': checked out '$sha1'"
 		fi
 	done
 }
@@ -360,7 +371,7 @@ cmd_status()
 	git ls-files --stage -- "$@" | grep -e '^160000 ' |
 	while read mode sha1 stage path
 	do
-		name=$(module_name "$path") || exit
+		name=$(module_name "$path") || continue
 		url=$(git config submodule."$name".url)
 		if test -z "$url" || ! test -d "$path"/.git
 		then
-- 
1.5.4.3.347.g5314c
Johannes Schindelin· Mar 4, 2008, 15:01 UTC · re: Ping Yin · lore

Re: [PATCH] git-submodule: Don't die when command fails for one submodule

Hi,
On Tue, 4 Mar 2008, Ping Yin wrote:
> When handling multiple modules, init/update/status command will exit 
> when it fails for one submodule. This patch makes the command continue 
> bypassing the failure.

I think that you should not do this. The failure should not be ignored. Maybe continuing, but not forgetting about the error. So:

> -		name=$(module_name "$path") || exit
> +		name=$(module_name "$path") || continue

I do not like this. It should remember that there was something foul, and eventually exit with that status.

Ciao, Dscho

Ping Yin· Mar 4, 2008, 16:00 UTC · re: Johannes Schindelin · lore

Re: [PATCH] git-submodule: Don't die when command fails for one submodule

On Tue, Mar 4, 2008 at 11:01 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 19 quoted lines
> Hi,
>
>
>  On Tue, 4 Mar 2008, Ping Yin wrote:
>
>  > When handling multiple modules, init/update/status command will exit
>  > when it fails for one submodule. This patch makes the command continue
>  > bypassing the failure.
>
>  I think that you should not do this.  The failure should not be ignored.
>  Maybe continuing, but not forgetting about the error.  So:
>
>
>  > -             name=$(module_name "$path") || exit
>  > +             name=$(module_name "$path") || continue
>
>  I do not like this.  It should remember that there was something foul, and
>  eventually exit with that status.
>
The error output is not lost and is in module_name
>  Ciao,
>  Dscho
>
>
-- 
Ping Yin
Johannes Schindelin· Mar 4, 2008, 16:03 UTC · re: Ping Yin · lore

Re: [PATCH] git-submodule: Don't die when command fails for one submodule

Hi,
On Wed, 5 Mar 2008, Ping Yin wrote:
> The error output is not lost and is in module_name

That's what I am saying: the error output is not enough. The exit status needs to indicate that there was an error, too. Everything else is unusable by scripts.

Thank you very much, Dscho

Ping Yin· Mar 4, 2008, 16:06 UTC · re: Johannes Schindelin · lore

Re: [PATCH] git-submodule: Don't die when command fails for one submodule

On Wed, Mar 5, 2008 at 12:03 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 10 quoted lines
> Hi,
>
>
>  On Wed, 5 Mar 2008, Ping Yin wrote:
>
>  > The error output is not lost and is in module_name
>
>  That's what I am saying: the error output is not enough.  The exit status
>  needs to indicate that there was an error, too.  Everything else is
>  unusable by scripts.

I think this can be done in a following patch which capture any error and give the right exit status.

>
-- 
Ping Yin
Johannes Schindelin· Mar 4, 2008, 16:13 UTC · re: Ping Yin · lore

Re: [PATCH] git-submodule: Don't die when command fails for one submodule

Hi,
On Wed, 5 Mar 2008, Ping Yin wrote:
Show 13 quoted lines
> On Wed, Mar 5, 2008 at 12:03 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> >  On Wed, 5 Mar 2008, Ping Yin wrote:
> >
> >  > The error output is not lost and is in module_name
> >
> >  That's what I am saying: the error output is not enough.  The exit status
> >  needs to indicate that there was an error, too.  Everything else is
> >  unusable by scripts.
> 
> I think this can be done in a following patch which capture any error
> and give the right exit status.

That's horrible! Are you seriously suggesting breaking a perfectly fine paradigm in one patch, just to fix it in the next one?

*Shudders*, Dscho

Junio C Hamano· Mar 4, 2008, 20:32 UTC · re: Ping Yin · lore

Re: [PATCH] git-submodule: Don't die when command fails for one submodule

"Ping Yin" <pkufranky@gmail.com> writes:
Show 15 quoted lines
> On Wed, Mar 5, 2008 at 12:03 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> Hi,
>>
>>
>>  On Wed, 5 Mar 2008, Ping Yin wrote:
>>
>>  > The error output is not lost and is in module_name
>>
>>  That's what I am saying: the error output is not enough.  The exit status
>>  needs to indicate that there was an error, too.  Everything else is
>>  unusable by scripts.
>
> I think this can be done in a following patch which capture any error
> and give the right exit status.
Please.

"You are absolutely right. I'll fix it in my next round to do frotz and xyzzy as you suggested. I want to make my next round better with fixes and improvements based on other comments I will receive during this round, so it will be at least in a few days. Thanks."

That's how I want to see things work around here.

← back to recent threads