[PATCH] submodule update: continue when an update fails

[PATCH] submodule update: continue when an update fails

[Fredrik Gustafsson]

Hi,
when running git submodule update the scripts stop after failing to
fetch an submodule. This is errornous because this implies that if a
submodule 'a' isn't availiable then submodule 'x' shouldn't be loaded.
This implies a relationship that isn't definied within git.

There's three different ways to solve this (because the script is runned
as a childprocess of the git-submodule.sh script):

1. use a global variable (via the export command) to share data between
 processes.
2. Print outdata from the childprocess to stdout and read it via a pipe
 in the parent process.
3. Do all error handling in the child process

I've choosen alternative 3 as it seams to have the smallest impact.


From 7ba07dcfdd99c14522946e923ec63bd0bcd60021 Mon Sep 17 00:00:00 2001
From: Fredrik Gustafsson <iveqy@iveqy.com>
Date: Mon, 18 Apr 2011 23:08:59 +0200
Subject: [PATCH] submodule update: continue when an update fails

git submodule update dies when a remote reference isn't a tree. Instead
of dying, print an error message and continue update the next submodule
(and die when all modules are done if there was any errors).

Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
---
 git-submodule.sh |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index b010a67..06ff7f2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -441,7 +441,8 @@ cmd_update()
 	fi
 
 	cloned_modules=
-	module_list "$@" |
+	err=
+	module_list "$@" | {
 	while read mode sha1 stage path
 	do
 		if test "$stage" = U
@@ -521,17 +522,32 @@ cmd_update()
 				;;
 			esac
 
-			(clear_local_git_env; cd "$path" && $command "$sha1") ||
-			die "Unable to $action '$sha1' in submodule path '$path'"
-			say "Submodule path '$path': $msg '$sha1'"
+			if (clear_local_git_env; cd "$path" && $command "$sha1")
+			then
+				   say "Submodule path '$path': $msg '$sha1'"
+			else
+				   say "Unable to $action '$sha1' in submodule path '$path'"
+				   err="Failed to $action one or more submodule(s)"
+				   continue
+			fi
 		fi
 
 		if test -n "$recursive"
 		then
-			(clear_local_git_env; cd "$path" && eval cmd_update "$orig_flags") ||
-			die "Failed to recurse into submodule path '$path'"
+			if !(clear_local_git_env; cd "$path" && eval cmd_update "$orig_flags")
+			then
+				say "Failed to recurse into submodule path '$path'"
+				continue
+			fi
 		fi
+
+	if test -n "$err"
+	then
+		die $err
+	fi
+
+	}
 }
 
 set_name_rev () {
-- 
1.7.5.rc0.133.gcc4dd.dirty

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

Re: [PATCH] submodule update: continue when an update fails

[Junio C Hamano]

Fredrik Gustafsson <iveqy@iveqy.com> writes:

> when running git submodule update the scripts stop after failing to
> fetch an submodule. This is errornous because this implies that if a
> submodule 'a' isn't availiable then submodule 'x' shouldn't be loaded.
> This implies a relationship that isn't definied within git.

I do not quite understand this. The relationship between 'a' and 'x' are
clearly defined within your superproject in that they are both submodules
of it.

It is indeed asymmetric in that if we fail 'a' we do not even attempt 'x',
while if we succeed a' and then fail 'x', we do not roll-back 'a', i.e. we
do not try to be atomic.

One could argue that if we do not bother to be atomic, it would be better
to try doing the remainder than stopping at the first failure.  Is that
what you wanted to say?

I suspect that it depends on the nature of the "failure" and the state the
failure leaves the particular submodule in.  For example, if the failure
leaves the failed submodule intact (i.e. idempotent and/or no-op), and if
it fails quickly enough, then it makes perfect sense to keep going and
finish all that can succeed.  On the other hand, if it leaves a huge mess
the user needs to go in and manually fix (think: conflict resolution),
some users may prefer to do so one at a time, instead of having to deal
with many independent huge messes that will be left by this change.

> There's three different ways to solve this (because the script is runned
> as a childprocess of the git-submodule.sh script):
>
> 1. use a global variable (via the export command) to share data between
>  processes.

It is unclear what you mean by "share data"?  Data on what?

> 2. Print outdata from the childprocess to stdout and read it via a pipe
>  in the parent process.

The same.  The reason why you need the communication is not explained
well. What are you trying to pass from the child to the parent? Why does
the parent need to know it?

> 3. Do all error handling in the child process

Ahh, so the "share data" was about "the outcome of the processing for one
submodule, if it succeeded, or if it failed and if so how".

> I've choosen alternative 3 as it seams to have the smallest impact.

All of the above, if expressed clearly (in other words, when rewritten so
that I do not have to say "I don't understand this"), should be in the
commit log message.

> From 7ba07dcfdd99c14522946e923ec63bd0bcd60021 Mon Sep 17 00:00:00 2001
> From: Fredrik Gustafsson <iveqy@iveqy.com>
> Date: Mon, 18 Apr 2011 23:08:59 +0200
> Subject: [PATCH] submodule update: continue when an update fails
>
> git submodule update dies when a remote reference isn't a tree. Instead

"When a remote reference isn't a tree"?  I am not sure what you mean.
What remote reference?

> of dying, print an error message and continue update the next submodule
> (and die when all modules are done if there was any errors).

More importantly, for the purpose of your change, it does not matter how a
command run for one submodule fails.  Perhaps restate the above like this?

    "git submodule update" stops at the first submodule it cannot update.
    Instead, report the error, continue to update other submodules, and
    exit with failure status at the end.

> Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
> ---
>  git-submodule.sh |   30 +++++++++++++++++++++++-------
>  1 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index b010a67..06ff7f2 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -441,7 +441,8 @@ cmd_update()
>  	fi
>  
>  	cloned_modules=
> -	module_list "$@" |
> +	err=
> +	module_list "$@" | {
>  	while read mode sha1 stage path
>  	do

Shouldn't this be

	module_list "$@" | {
        err=
        while read ...
	do

as the $err is only relevant to the child process that is on the
downstream of this pipe?