[RFC/PATCH] submodule: add 'exec' option to submodule update

[RFC/PATCH] submodule: add 'exec' option to submodule update

[Chris Packham]

This allows the user some finer grained control over how the update is
done. The primary motivation for this was interoperability with stgit
however being able to intercept the submodule update process may prove
useful for integrating or extending other tools.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
--
Hi,

At $dayjob we have a number of users that are accustomed to using stgit.
Stgit doesn't play nicely with git rebase which would be the logical
setting for submodule.*.update for our usage. Instead we need to run
'stg rebase --merged' on those submodules that have been initialised
with stgit.

Our current solution is an in-house script which is a poor substitute
for git submodule update. I'd much rather replace our script with git
submodule update but we do have a requirement to keep stgit for the
foreseeable future.  Rather than narrowing in on stgit it seems logical
to allow an arbitrary update command to be executed.
---
 Documentation/git-submodule.txt |  8 +++++++-
 git-submodule.sh                | 22 +++++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index e576713..a0d8b89 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
 	      [-f|--force] [--rebase] [--reference <repository>]
-	      [--merge] [--recursive] [--] [<path>...]
+	      [--merge] [--recursive] [--exec <command>] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
 	      [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
@@ -172,6 +172,12 @@ If `--force` is specified, the submodule will be checked out (using
 `git checkout --force` if appropriate), even if the commit specified in the
 index of the containing repository already matches the commit checked out in
 the submodule.
++
+If `--exec` is specified the next argument is an arbitrary shell command that
+can be used to gain finer control over how the update is done or integrate with
+external tools. The command should take one argument namely the sha1 of the
+commit to update to. The config variable `submodule.$name.update` can be set
+to `exec` along with `submodule.$name.update.command`.
 
 summary::
 	Show commit summary between the given commit (defaults to HEAD) and
diff --git a/git-submodule.sh b/git-submodule.sh
index eb58c8e..44631c8 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,7 +9,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
-   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--exec <command>] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
@@ -32,6 +32,7 @@ nofetch=
 update=
 prefix=
 custom_name=
+update_exec_command=
 
 # The function takes at most 2 arguments. The first argument is the
 # URL that navigates to the submodule origin repo. When relative, this URL
@@ -677,6 +678,12 @@ cmd_update()
 		--checkout)
 			update="checkout"
 			;;
+		--exec)
+			case "$2" in '') usage ;; esac
+			update="exec"
+			update_exec_command="$2"
+			shift
+			;;
 		--)
 			shift
 			break
@@ -718,6 +725,11 @@ cmd_update()
 			update_module=$(git config submodule."$name".update)
 		fi
 
+		if test -z "$update_exec_command"
+		then
+			exec_command=$(git config submodule."$name".update.command)
+		fi
+
 		if test "$update_module" = "none"
 		then
 			echo "Skipping submodule '$prefix$sm_path'"
@@ -799,6 +811,14 @@ Maybe you want to use 'update --init'?")"
 				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
 				must_die_on_failure=yes
 				;;
+			exec)
+				test -n "$exec_command" || \
+					die "$(eval_gettext "Unable to update '\$prefix\$sm_path' exec command not specified")"
+				command="$exec_command"
+				die_msg="$(eval_gettext "Unable to exec '\$command \$sha1' in submodule path '\$prefix\$sm_path'")"
+				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': '\$command \$sha1'")"
+				must_die_on_failure=yes
+				;;
 			*)
 				command="git checkout $subforce -q"
 				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$prefix\$sm_path'")"
-- 
1.8.3.1.643.gebeea52.dirty

Re: [RFC/PATCH] submodule: add 'exec' option to submodule update

[Stefan Näwe]

Am 28.06.2013 11:53, schrieb Chris Packham:
> This allows the user some finer grained control over how the update is
> done. The primary motivation for this was interoperability with stgit
> however being able to intercept the submodule update process may prove
> useful for integrating or extending other tools.
> 
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> --
> Hi,
> 
> At $dayjob we have a number of users that are accustomed to using stgit.
> Stgit doesn't play nicely with git rebase which would be the logical
> setting for submodule.*.update for our usage. Instead we need to run
> 'stg rebase --merged' on those submodules that have been initialised
> with stgit.
> 
> Our current solution is an in-house script which is a poor substitute
> for git submodule update. I'd much rather replace our script with git
> submodule update but we do have a requirement to keep stgit for the
> foreseeable future.  Rather than narrowing in on stgit it seems logical
> to allow an arbitrary update command to be executed.

Hhmmm...
Can't the same be accomplished with 
 
  git submodule foreach 'your-update-script $sha1'

Am I missing something?

Stefan
-- 
----------------------------------------------------------------
/dev/random says: Preserve nature... pickle a squirrel.
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"

Re: [RFC/PATCH] submodule: add 'exec' option to submodule update

[Fredrik Gustafsson]

Hi,

On Fri, Jun 28, 2013 at 09:53:10PM +1200, Chris Packham wrote:
> This allows the user some finer grained control over how the update is
> done. The primary motivation for this was interoperability with stgit
> however being able to intercept the submodule update process may prove
> useful for integrating or extending other tools.
> 
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> --
> Hi,
> 
> At $dayjob we have a number of users that are accustomed to using stgit.
> Stgit doesn't play nicely with git rebase which would be the logical
> setting for submodule.*.update for our usage. Instead we need to run
> 'stg rebase --merged' on those submodules that have been initialised
> with stgit.
> 
> Our current solution is an in-house script which is a poor substitute
> for git submodule update. I'd much rather replace our script with git
> submodule update but we do have a requirement to keep stgit for the
> foreseeable future.  Rather than narrowing in on stgit it seems logical
> to allow an arbitrary update command to be executed.
> ---
>  Documentation/git-submodule.txt |  8 +++++++-
>  git-submodule.sh                | 22 +++++++++++++++++++++-
>  2 files changed, 28 insertions(+), 2 deletions(-)
> 

technically it looks fine to me (except for the lack of tests) but I'm
not sure I follow the use case.

In your case, you want to run a script to determinate if that certain
submodule should use merge or rebase depending on "whatever". And this
can't be done with git submodule foreach because you want to know the
sha1 to update to. Have I understood you correctly?
-- 
Med vänliga hälsningar
Fredrik Gustafsson

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

Re: [RFC/PATCH] submodule: add 'exec' option to submodule update

[Chris Packham]

On 28/06/13 22:42, Fredrik Gustafsson wrote:
> Hi,
> 
> On Fri, Jun 28, 2013 at 09:53:10PM +1200, Chris Packham wrote:
>> This allows the user some finer grained control over how the update is
>> done. The primary motivation for this was interoperability with stgit
>> however being able to intercept the submodule update process may prove
>> useful for integrating or extending other tools.
>>
>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>> --
>> Hi,
>>
>> At $dayjob we have a number of users that are accustomed to using stgit.
>> Stgit doesn't play nicely with git rebase which would be the logical
>> setting for submodule.*.update for our usage. Instead we need to run
>> 'stg rebase --merged' on those submodules that have been initialised
>> with stgit.
>>
>> Our current solution is an in-house script which is a poor substitute
>> for git submodule update. I'd much rather replace our script with git
>> submodule update but we do have a requirement to keep stgit for the
>> foreseeable future.  Rather than narrowing in on stgit it seems logical
>> to allow an arbitrary update command to be executed.
>> ---
>>  Documentation/git-submodule.txt |  8 +++++++-
>>  git-submodule.sh                | 22 +++++++++++++++++++++-
>>  2 files changed, 28 insertions(+), 2 deletions(-)
>>
> 
> technically it looks fine to me (except for the lack of tests) but I'm
> not sure I follow the use case.
> 
> In your case, you want to run a script to determinate if that certain
> submodule should use merge or rebase depending on "whatever". And this
> can't be done with git submodule foreach because you want to know the
> sha1 to update to. Have I understood you correctly?
>

Correct. We tend to have submodules that are just normal detached heads
which we don't usually touch and others that are actively developed
where we would use submodule.x.update=rebase (I personally do) but some
developers want to use stgit on those repositories.

Another approach could be to do a 'git pull --no-recurse-submodule' then
use 'git submodule foreach script-that-does-the-rebase'. The benefit of
the patch I sent is that it can be setup using the config variables[1]
and updated the normal way along with the detached HEADs and those using
plain git branches.

There may be other use-cases for integration with other tools as well
(e.g. something that updates a review tool when commits get rebased).

--
[1] I'm not crazy about the name of submodule.*.update.command but I
couldn't think of a better one.

Re: [RFC/PATCH] submodule: add 'exec' option to submodule update

[Jens Lehmann]

Am 29.06.2013 11:11, schrieb Chris Packham:
> On 28/06/13 22:42, Fredrik Gustafsson wrote:
>> technically it looks fine to me (except for the lack of tests) but I'm
>> not sure I follow the use case.
>>
>> In your case, you want to run a script to determinate if that certain
>> submodule should use merge or rebase depending on "whatever". And this
>> can't be done with git submodule foreach because you want to know the
>> sha1 to update to. Have I understood you correctly?
> 
> Correct. We tend to have submodules that are just normal detached heads
> which we don't usually touch and others that are actively developed
> where we would use submodule.x.update=rebase (I personally do) but some
> developers want to use stgit on those repositories.
> 
> Another approach could be to do a 'git pull --no-recurse-submodule' then
> use 'git submodule foreach script-that-does-the-rebase'. The benefit of
> the patch I sent is that it can be setup using the config variables[1]
> and updated the normal way along with the detached HEADs and those using
> plain git branches.

Wouldn't a "stgit submodule update" (which would do the Right Thing for
submodules initialized with stgit by maybe just using the pull & foreach
logic you described) be a better UI for solving your problem?

> There may be other use-cases for integration with other tools as well
> (e.g. something that updates a review tool when commits get rebased).
> 
> --
> [1] I'm not crazy about the name of submodule.*.update.command but I
> couldn't think of a better one.

Hmm, if we go that route, why not do the same we do for aliases? If
the submodule.*.update setting is prefixed with a '!', we just execute
the shell command following. This would give everyone the freedom to
do arbitrary stuff if the current none, checkout, merge & rebase won't
do the trick without having to add another config option.

Re: [RFC/PATCH] submodule: add 'exec' option to submodule update

[Chris Packham]

On 01/07/13 03:30, Jens Lehmann wrote:
> Am 29.06.2013 11:11, schrieb Chris Packham:
>> On 28/06/13 22:42, Fredrik Gustafsson wrote:
>>> technically it looks fine to me (except for the lack of tests) but I'm
>>> not sure I follow the use case.
>>>
>>> In your case, you want to run a script to determinate if that certain
>>> submodule should use merge or rebase depending on "whatever". And this
>>> can't be done with git submodule foreach because you want to know the
>>> sha1 to update to. Have I understood you correctly?
>>
>> Correct. We tend to have submodules that are just normal detached heads
>> which we don't usually touch and others that are actively developed
>> where we would use submodule.x.update=rebase (I personally do) but some
>> developers want to use stgit on those repositories.
>>
>> Another approach could be to do a 'git pull --no-recurse-submodule' then
>> use 'git submodule foreach script-that-does-the-rebase'. The benefit of
>> the patch I sent is that it can be setup using the config variables[1]
>> and updated the normal way along with the detached HEADs and those using
>> plain git branches.
> 
> Wouldn't a "stgit submodule update" (which would do the Right Thing for
> submodules initialized with stgit by maybe just using the pull & foreach
> logic you described) be a better UI for solving your problem?

Yeah I guess so. I was hoping to stay away from an stgit specific
solution but I'm not hearing any other voices looking for such flexibility.

>> There may be other use-cases for integration with other tools as well
>> (e.g. something that updates a review tool when commits get rebased).
>>
>> --
>> [1] I'm not crazy about the name of submodule.*.update.command but I
>> couldn't think of a better one.
> 
> Hmm, if we go that route, why not do the same we do for aliases? If
> the submodule.*.update setting is prefixed with a '!', we just execute
> the shell command following. This would give everyone the freedom to
> do arbitrary stuff if the current none, checkout, merge & rebase won't
> do the trick without having to add another config option.
>

Make sense. I'll give it a go.

[RFC/PATCHv2] submodule: add ability to configure update command

[Chris Packham]

Users can set submodule.$name.update to '!command' which will cause
'command' to be run instead of checkout/merge/rebase.  This allows the
user some finer grained control over how the update is done. The primary
motivation for this was interoperability with stgit however being able
to intercept the submodule update process may prove useful for
integrating or extending other tools.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---

On 01/07/13 03:30, Jens Lehmann wrote:
>
> Hmm, if we go that route, why not do the same we do for aliases? If
> the submodule.*.update setting is prefixed with a '!', we just execute
> the shell command following. This would give everyone the freedom to
> do arbitrary stuff if the current none, checkout, merge & rebase won't
> do the trick without having to add another config option.
>

And here's an implementation of this. Actually I like this a lot better
to the v1 patch. Still lacks tests but if there is enough interest I can
add some.

 Documentation/git-submodule.txt | 5 ++++-
 git-submodule.sh                | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index e576713..0befc20 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -159,7 +159,9 @@ update::
 	This will make the submodules HEAD be detached unless `--rebase` or
 	`--merge` is specified or the key `submodule.$name.update` is set to
 	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`.
+	`--checkout`. Setting the key `submodule.$name.update` to `!command`
+	will cause `command` to be run. `command` can be any arbitrary shell
+	command that takes a single argument, namely the sha1 to update to.
 +
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
@@ -172,6 +174,7 @@ If `--force` is specified, the submodule will be checked out (using
 `git checkout --force` if appropriate), even if the commit specified in the
 index of the containing repository already matches the commit checked out in
 the submodule.
++
 
 summary::
 	Show commit summary between the given commit (defaults to HEAD) and
diff --git a/git-submodule.sh b/git-submodule.sh
index eb58c8e..680cb68 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -799,6 +799,12 @@ Maybe you want to use 'update --init'?")"
 				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
 				must_die_on_failure=yes
 				;;
+			!*)
+				command="$(expr "$update_module" : '!\(.*\)')"
+				die_msg="$(eval_gettext "Unable to exec '\$command \$sha1' in submodule path '\$prefix\$sm_path'")"
+				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': '\$command \$sha1'")"
+				must_die_on_failure=yes
+				;;
 			*)
 				command="git checkout $subforce -q"
 				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$prefix\$sm_path'")"
-- 
1.8.3.1.644.g533062f.dirty

Re: [RFC/PATCHv2] submodule: add ability to configure update command

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> Subject: Re: [RFC/PATCHv2] submodule: add ability to configure update command

"ability to configure" wants to be rephrased to make it more useful
in the shortlog output to help me update release notes ;-)

    Subject: [PATCH] submodule update: allow custom update command

perhaps?  We already allow you to configure update command between
merge, rebase and checkout-to-detach, and the value of the change is
to let an arbitrary custom command to be used there.

> And here's an implementation of this. Actually I like this a lot better
> to the v1 patch.

I agree that this looks much more sensible.

Thanks.

Re: [RFC/PATCHv2] submodule: add ability to configure update command

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> +			!*)
> +				command="$(expr "$update_module" : '!\(.*\)')"

	command=${command#!}

Re: [RFC/PATCHv2] submodule: add ability to configure update command

[Chris Packham]

On 02/07/13 04:48, Junio C Hamano wrote:
> Chris Packham <judge.packham@gmail.com> writes:
> 
>> +			!*)
>> +				command="$(expr "$update_module" : '!\(.*\)')"
> 
> 	command=${command#!}
> 

Thanks, expr was my attempt to avoid using a ${command:1} bash-ism. v3
on it's way.

[RFC/PATCHv3] submodule update: allow custom update command

[Chris Packham]

Users can set submodule.$name.update to '!command' which will cause
'command' to be run instead of checkout/merge/rebase.  This allows the
user some finer grained control over how the update is done. The primary
motivation for this was interoperability with stgit however being able
to intercept the submodule update process may prove useful for
integrating or extending other tools.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
v3 updated as per Junio's review.

Still needs tests. Any suggestions? I've been manually testing by setting
submodule.$name.update to '!echo'. I haven't looked to see if there are
existing 'submodule update' tests yet.

 Documentation/git-submodule.txt | 5 ++++-
 git-submodule.sh                | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index e576713..0befc20 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -159,7 +159,9 @@ update::
 	This will make the submodules HEAD be detached unless `--rebase` or
 	`--merge` is specified or the key `submodule.$name.update` is set to
 	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`.
+	`--checkout`. Setting the key `submodule.$name.update` to `!command`
+	will cause `command` to be run. `command` can be any arbitrary shell
+	command that takes a single argument, namely the sha1 to update to.
 +
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
@@ -172,6 +174,7 @@ If `--force` is specified, the submodule will be checked out (using
 `git checkout --force` if appropriate), even if the commit specified in the
 index of the containing repository already matches the commit checked out in
 the submodule.
++
 
 summary::
 	Show commit summary between the given commit (defaults to HEAD) and
diff --git a/git-submodule.sh b/git-submodule.sh
index eb58c8e..a7c2375 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -799,6 +799,12 @@ Maybe you want to use 'update --init'?")"
 				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
 				must_die_on_failure=yes
 				;;
+			!*)
+				command="${update_module#!}"
+				die_msg="$(eval_gettext "Unable to exec '\$command \$sha1' in submodule path '\$prefix\$sm_path'")"
+				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': '\$command \$sha1'")"
+				must_die_on_failure=yes
+				;;
 			*)
 				command="git checkout $subforce -q"
 				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$prefix\$sm_path'")"
-- 
1.8.3.1.644.gfaf2af3

Re: [RFC/PATCHv3] submodule update: allow custom update command

[Jens Lehmann]

Am 02.07.2013 12:12, schrieb Chris Packham:
> Users can set submodule.$name.update to '!command' which will cause
> 'command' to be run instead of checkout/merge/rebase.  This allows the
> user some finer grained control over how the update is done. The primary
> motivation for this was interoperability with stgit however being able
> to intercept the submodule update process may prove useful for
> integrating or extending other tools.
> 
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> v3 updated as per Junio's review.

Thanks, a few comments below.

> Still needs tests. Any suggestions? I've been manually testing by setting
> submodule.$name.update to '!echo'. I haven't looked to see if there are
> existing 'submodule update' tests yet.

t7406-submodule-update.sh should be the right place.

>  Documentation/git-submodule.txt | 5 ++++-
>  git-submodule.sh                | 6 ++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index e576713..0befc20 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -159,7 +159,9 @@ update::
>  	This will make the submodules HEAD be detached unless `--rebase` or
>  	`--merge` is specified or the key `submodule.$name.update` is set to
>  	`rebase`, `merge` or `none`. `none` can be overridden by specifying
> -	`--checkout`.
> +	`--checkout`. Setting the key `submodule.$name.update` to `!command`
> +	will cause `command` to be run. `command` can be any arbitrary shell
> +	command that takes a single argument, namely the sha1 to update to.
>  +
>  If the submodule is not yet initialized, and you just want to use the
>  setting as stored in .gitmodules, you can automatically initialize the
> @@ -172,6 +174,7 @@ If `--force` is specified, the submodule will be checked out (using
>  `git checkout --force` if appropriate), even if the commit specified in the
>  index of the containing repository already matches the commit checked out in
>  the submodule.
> ++
>  
>  summary::
>  	Show commit summary between the given commit (defaults to HEAD) and

I'm not sure this change is necessary ;-)

> diff --git a/git-submodule.sh b/git-submodule.sh
> index eb58c8e..a7c2375 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -799,6 +799,12 @@ Maybe you want to use 'update --init'?")"
>  				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
>  				must_die_on_failure=yes
>  				;;
> +			!*)
> +				command="${update_module#!}"
> +				die_msg="$(eval_gettext "Unable to exec '\$command \$sha1' in submodule path '\$prefix\$sm_path'")"

Hmm, "Unable to exec" does not quite cut it, as the command was executed
but returned an error, right? Maybe something like this:

   Execution of '\$command \$sha1' failed in submodule  path '\$prefix\$sm_path'

> +				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': '\$command \$sha1'")"
> +				must_die_on_failure=yes
> +				;;
>  			*)
>  				command="git checkout $subforce -q"
>  				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$prefix\$sm_path'")"
> 

Re: [RFC/PATCHv3] submodule update: allow custom update command

[Chris Packham]

On Wed, Jul 3, 2013 at 4:56 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 02.07.2013 12:12, schrieb Chris Packham:
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -159,7 +159,9 @@ update::
>>       This will make the submodules HEAD be detached unless `--rebase` or
>>       `--merge` is specified or the key `submodule.$name.update` is set to
>>       `rebase`, `merge` or `none`. `none` can be overridden by specifying
>> -     `--checkout`.
>> +     `--checkout`. Setting the key `submodule.$name.update` to `!command`
>> +     will cause `command` to be run. `command` can be any arbitrary shell
>> +     command that takes a single argument, namely the sha1 to update to.
>>  +
>>  If the submodule is not yet initialized, and you just want to use the
>>  setting as stored in .gitmodules, you can automatically initialize the
>> @@ -172,6 +174,7 @@ If `--force` is specified, the submodule will be checked out (using
>>  `git checkout --force` if appropriate), even if the commit specified in the
>>  index of the containing repository already matches the commit checked out in
>>  the submodule.
>> ++
>>
>>  summary::
>>       Show commit summary between the given commit (defaults to HEAD) and
>
> I'm not sure this change is necessary ;-)
>

Not necessary because it should be documented in
Documentation/config.txt instead, or not necessary because it's a
niche feature that doesn't need to be advertised?

>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index eb58c8e..a7c2375 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -799,6 +799,12 @@ Maybe you want to use 'update --init'?")"
>>                               say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
>>                               must_die_on_failure=yes
>>                               ;;
>> +                     !*)
>> +                             command="${update_module#!}"
>> +                             die_msg="$(eval_gettext "Unable to exec '\$command \$sha1' in submodule path '\$prefix\$sm_path'")"
>
> Hmm, "Unable to exec" does not quite cut it, as the command was executed
> but returned an error, right? Maybe something like this:
>
>    Execution of '\$command \$sha1' failed in submodule  path '\$prefix\$sm_path'
>

Will include in v4 once I write some tests.

Re: [RFC/PATCHv3] submodule update: allow custom update command

[Jens Lehmann]

Am 03.07.2013 01:26, schrieb Chris Packham:
> On Wed, Jul 3, 2013 at 4:56 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 02.07.2013 12:12, schrieb Chris Packham:
>>> --- a/Documentation/git-submodule.txt
>>> +++ b/Documentation/git-submodule.txt
>>> @@ -159,7 +159,9 @@ update::
>>>       This will make the submodules HEAD be detached unless `--rebase` or
>>>       `--merge` is specified or the key `submodule.$name.update` is set to
>>>       `rebase`, `merge` or `none`. `none` can be overridden by specifying
>>> -     `--checkout`.
>>> +     `--checkout`. Setting the key `submodule.$name.update` to `!command`
>>> +     will cause `command` to be run. `command` can be any arbitrary shell
>>> +     command that takes a single argument, namely the sha1 to update to.
>>>  +
>>>  If the submodule is not yet initialized, and you just want to use the
>>>  setting as stored in .gitmodules, you can automatically initialize the

The above hunk is perfectly fine ...

>>> @@ -172,6 +174,7 @@ If `--force` is specified, the submodule will be checked out (using
>>>  `git checkout --force` if appropriate), even if the commit specified in the
>>>  index of the containing repository already matches the commit checked out in
>>>  the submodule.
>>> ++
>>>
>>>  summary::
>>>       Show commit summary between the given commit (defaults to HEAD) and

... but I don't understand the extra '+'-line added here.

>> I'm not sure this change is necessary ;-)
> 
> Not necessary because it should be documented in
> Documentation/config.txt instead, or not necessary because it's a
> niche feature that doesn't need to be advertised?

Sorry for the confusion, I should have been more specific here.

>>> diff --git a/git-submodule.sh b/git-submodule.sh
>>> index eb58c8e..a7c2375 100755
>>> --- a/git-submodule.sh
>>> +++ b/git-submodule.sh
>>> @@ -799,6 +799,12 @@ Maybe you want to use 'update --init'?")"
>>>                               say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
>>>                               must_die_on_failure=yes
>>>                               ;;
>>> +                     !*)
>>> +                             command="${update_module#!}"
>>> +                             die_msg="$(eval_gettext "Unable to exec '\$command \$sha1' in submodule path '\$prefix\$sm_path'")"
>>
>> Hmm, "Unable to exec" does not quite cut it, as the command was executed
>> but returned an error, right? Maybe something like this:
>>
>>    Execution of '\$command \$sha1' failed in submodule  path '\$prefix\$sm_path'
>>
> 
> Will include in v4 once I write some tests.

Thanks.

Re: [RFC/PATCHv3] submodule update: allow custom update command

[Chris Packham]

On 03/07/13 18:55, Jens Lehmann wrote:
> Am 03.07.2013 01:26, schrieb Chris Packham:
>> On Wed, Jul 3, 2013 at 4:56 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>>> Am 02.07.2013 12:12, schrieb Chris Packham:
>>>> --- a/Documentation/git-submodule.txt
>>>> +++ b/Documentation/git-submodule.txt
>>>> @@ -159,7 +159,9 @@ update::
>>>>       This will make the submodules HEAD be detached unless `--rebase` or
>>>>       `--merge` is specified or the key `submodule.$name.update` is set to
>>>>       `rebase`, `merge` or `none`. `none` can be overridden by specifying
>>>> -     `--checkout`.
>>>> +     `--checkout`. Setting the key `submodule.$name.update` to `!command`
>>>> +     will cause `command` to be run. `command` can be any arbitrary shell
>>>> +     command that takes a single argument, namely the sha1 to update to.
>>>>  +
>>>>  If the submodule is not yet initialized, and you just want to use the
>>>>  setting as stored in .gitmodules, you can automatically initialize the
> 
> The above hunk is perfectly fine ...
> 
>>>> @@ -172,6 +174,7 @@ If `--force` is specified, the submodule will be checked out (using
>>>>  `git checkout --force` if appropriate), even if the commit specified in the
>>>>  index of the containing repository already matches the commit checked out in
>>>>  the submodule.
>>>> ++
>>>>
>>>>  summary::
>>>>       Show commit summary between the given commit (defaults to HEAD) and
> 
> ... but I don't understand the extra '+'-line added here.
> 
>>> I'm not sure this change is necessary ;-)
>>
>> Not necessary because it should be documented in
>> Documentation/config.txt instead, or not necessary because it's a
>> niche feature that doesn't need to be advertised?
> 
> Sorry for the confusion, I should have been more specific here.
> 

Ah that makes sense. It's left over from the '--exec' option in v1. Will
be cleaned up in v4.

On a related note should I be updating Documentation/config.txt as well?
Even if it's a statement that this feature exists refer to
git-submodule(1) for details.

>>>> diff --git a/git-submodule.sh b/git-submodule.sh
>>>> index eb58c8e..a7c2375 100755
>>>> --- a/git-submodule.sh
>>>> +++ b/git-submodule.sh
>>>> @@ -799,6 +799,12 @@ Maybe you want to use 'update --init'?")"
>>>>                               say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
>>>>                               must_die_on_failure=yes
>>>>                               ;;
>>>> +                     !*)
>>>> +                             command="${update_module#!}"
>>>> +                             die_msg="$(eval_gettext "Unable to exec '\$command \$sha1' in submodule path '\$prefix\$sm_path'")"
>>>
>>> Hmm, "Unable to exec" does not quite cut it, as the command was executed
>>> but returned an error, right? Maybe something like this:
>>>
>>>    Execution of '\$command \$sha1' failed in submodule  path '\$prefix\$sm_path'
>>>
>>
>> Will include in v4 once I write some tests.
> 
> Thanks.
> 

Re: [RFC/PATCHv3] submodule update: allow custom update command

[Chris Packham]

On 03/07/13 19:54, Chris Packham wrote:
> On a related note should I be updating Documentation/config.txt as well?
> Even if it's a statement that this feature exists refer to
> git-submodule(1) for details.
> 

Answering my own question. While 'update' is mentioned it's possible
values are not.

[RFC/PATCHv4] submodule update: allow custom update command

[Chris Packham]

Users can set submodule.$name.update to '!command' which will cause
'command' to be run instead of checkout/merge/rebase.  This allows the
user some finer grained control over how the update is done. The primary
motivation for this was interoperability with stgit however being able
to intercept the submodule update process may prove useful for
integrating with or extensions to other tools.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
v4 adds a couple of simple tests - an equivalent of update=checkout and a test
to make sure we detect a failure reported by the update command.

 Documentation/git-submodule.txt |  4 +++-
 git-submodule.sh                |  6 ++++++
 t/t7406-submodule-update.sh     | 29 +++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index e576713..2f18f7d 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -159,7 +159,9 @@ update::
 	This will make the submodules HEAD be detached unless `--rebase` or
 	`--merge` is specified or the key `submodule.$name.update` is set to
 	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`.
+	`--checkout`. Setting the key `submodule.$name.update` to `!command`
+	will cause `command` to be run. `command` can be any arbitrary shell
+	command that takes a single argument, namely the sha1 to update to.
 +
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
diff --git a/git-submodule.sh b/git-submodule.sh
index eb58c8e..e7579f0 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -799,6 +799,12 @@ Maybe you want to use 'update --init'?")"
 				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
 				must_die_on_failure=yes
 				;;
+			!*)
+				command="${update_module#!}"
+				die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule  path '\$prefix\$sm_path'")"
+				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': '\$command \$sha1'")"
+				must_die_on_failure=yes
+				;;
 			*)
 				command="git checkout $subforce -q"
 				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$prefix\$sm_path'")"
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index a4ffea0..48d1279 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -279,6 +279,35 @@ test_expect_success 'submodule update - checkout in .git/config' '
 	)
 '
 
+test_expect_success 'submodule update - command in .git/config' '
+	(cd super &&
+	 git config submodule.submodule.update "!git checkout"
+	) &&
+	(cd super/submodule &&
+	  git reset --hard HEAD^
+	) &&
+	(cd super &&
+	 (cd submodule &&
+	  compare_head
+	 ) &&
+	 git submodule update submodule &&
+	 cd submodule &&
+	 ! compare_head
+	)
+'
+
+test_expect_success 'submodule update - command in .git/config catches failure' '
+	(cd super &&
+	 git config submodule.submodule.update "!false"
+	) &&
+	(cd super/submodule &&
+	  git reset --hard HEAD^
+	) &&
+	(cd super &&
+	 test_must_fail git submodule update submodule
+	)
+'
+
 test_expect_success 'submodule init picks up rebase' '
 	(cd super &&
 	 git config -f .gitmodules submodule.rebasing.update rebase &&
-- 
1.8.3.1.644.ge86575b

Re: [RFC/PATCHv3] submodule update: allow custom update command

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> On 03/07/13 19:54, Chris Packham wrote:
>> On a related note should I be updating Documentation/config.txt as well?
>> Even if it's a statement that this feature exists refer to
>> git-submodule(1) for details.
>> 
>
> Answering my own question. While 'update' is mentioned it's possible
> values are not.

Yeah, I think it is good as-is.

The entry in config.txt points at git-submodule(1) and your patch
updates the latter by adding another possible value, so the values
are discoverable; we do not have to maintain two separate lists and
keep them in sync that way.

Re: [RFC/PATCHv4] submodule update: allow custom update command

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> Users can set submodule.$name.update to '!command' which will cause
> 'command' to be run instead of checkout/merge/rebase.  This allows the
> user some finer grained control over how the update is done. The primary
> motivation for this was interoperability with stgit however being able
> to intercept the submodule update process may prove useful for
> integrating with or extensions to other tools.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> v4 adds a couple of simple tests - an equivalent of update=checkout and a test
> to make sure we detect a failure reported by the update command.

I think this can drop RFC/ now ;-)

Will replace what was queued and merge to 'next' unless I hear
otherwise within a few days.

Thanks.

>
>  Documentation/git-submodule.txt |  4 +++-
>  git-submodule.sh                |  6 ++++++
>  t/t7406-submodule-update.sh     | 29 +++++++++++++++++++++++++++++
>  3 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index e576713..2f18f7d 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -159,7 +159,9 @@ update::
>  	This will make the submodules HEAD be detached unless `--rebase` or
>  	`--merge` is specified or the key `submodule.$name.update` is set to
>  	`rebase`, `merge` or `none`. `none` can be overridden by specifying
> -	`--checkout`.
> +	`--checkout`. Setting the key `submodule.$name.update` to `!command`
> +	will cause `command` to be run. `command` can be any arbitrary shell
> +	command that takes a single argument, namely the sha1 to update to.
>  +
>  If the submodule is not yet initialized, and you just want to use the
>  setting as stored in .gitmodules, you can automatically initialize the
> diff --git a/git-submodule.sh b/git-submodule.sh
> index eb58c8e..e7579f0 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -799,6 +799,12 @@ Maybe you want to use 'update --init'?")"
>  				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': merged in '\$sha1'")"
>  				must_die_on_failure=yes
>  				;;
> +			!*)
> +				command="${update_module#!}"
> +				die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule  path '\$prefix\$sm_path'")"
> +				say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': '\$command \$sha1'")"
> +				must_die_on_failure=yes
> +				;;
>  			*)
>  				command="git checkout $subforce -q"
>  				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$prefix\$sm_path'")"
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index a4ffea0..48d1279 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -279,6 +279,35 @@ test_expect_success 'submodule update - checkout in .git/config' '
>  	)
>  '
>  
> +test_expect_success 'submodule update - command in .git/config' '
> +	(cd super &&
> +	 git config submodule.submodule.update "!git checkout"
> +	) &&
> +	(cd super/submodule &&
> +	  git reset --hard HEAD^
> +	) &&
> +	(cd super &&
> +	 (cd submodule &&
> +	  compare_head
> +	 ) &&
> +	 git submodule update submodule &&
> +	 cd submodule &&
> +	 ! compare_head
> +	)
> +'
> +
> +test_expect_success 'submodule update - command in .git/config catches failure' '
> +	(cd super &&
> +	 git config submodule.submodule.update "!false"
> +	) &&
> +	(cd super/submodule &&
> +	  git reset --hard HEAD^
> +	) &&
> +	(cd super &&
> +	 test_must_fail git submodule update submodule
> +	)
> +'
> +
>  test_expect_success 'submodule init picks up rebase' '
>  	(cd super &&
>  	 git config -f .gitmodules submodule.rebasing.update rebase &&