[RFC] git-submodule update: Add --commit option

Subsystems: the rest

3 messages, 1 author, 2016-06-15 · open the first message on its own page

[RFC] git-submodule update: Add --commit option

From: W. Trevor King <hidden>
Date: 2016-06-15 22:55:27

This option triggers automatic commits when `submodule update` changes
any gitlinked submodule SHA-1s.  The commit message contains a
`shortlog` summary of the changes for each changed submodule.
---

On Tue, Nov 27, 2012 at 07:51:42PM +0100, Heiko Voigt wrote:
BTW, I am more and more convinced that an automatically manufactured
commit on update with --branch should be the default. What do other
think? Sascha raised a concern that he would not want this, but as far as
I understood he let the CI-server do that so I see no downside to
natively adding that to git. People who want to manually craft those
commits can still amend the generated commit. Since this is all about
helping people keeping their submodules updated why not go the full way?
Here's a first pass (without documentation) for automatic commits on
submodule updates.  There have been a number of requests for
automatically-committed submodule updates due to submodule upstreams.
This patch shows how you can do that (if applied with my `submodule
update --remote` series), and reuse the same logic to automatically
commit changes due to local submodule changes (as shown here in the
new test).

I think the logic is pretty good, but the implementation is pretty
ugly due to POSIX shell variable limitations.  I'm basically trying to
pass an array of [(name, sm_path, sha1, subsha1), ...] into
commit_changes().  I though about perling-out in commit_changes(), but
I lack sufficient perl-fu to know how to tie clear_local_git_env, cd,
and shortlog up in a single open2 call.  If anyone can give me some
implementation pointers, that would be very helpful.

This is against v1.8.0 (without my --remote series).  To apply on top
of the --remote series, you'd have to save the original gitlinked
$sha1 and use that original value when constructing changed_modules.
I can attach this to the end of the --remote series if desired, but I
think this patch could also stand on its own.

Obviously this still needs documentation, etc., but I wanted feedback
on the implementation before I started digging into that.

Cheers,
Trevor

---
 git-submodule.sh            | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t7406-submodule-update.sh | 19 +++++++++++++
 2 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..d9a59af 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
-   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--commit] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--] [<path>...]"
@@ -21,6 +21,7 @@ require_work_tree
 command=
 branch=
 force=
+commit=
 reference=
 cached=
 recursive=
@@ -240,6 +241,52 @@ module_clone()
 }
 
 #
+# Commit changed submodule gitlinks
+#
+# $1 = name-a;sha1-a;subsha1-a\n[name-b;sha1-b;subsha1-b\n...]
+#
+commit_changes()
+{
+	echo "commiting $1"
+	OIFS="$IFS"
+	IFS=";"
+	paths=$(echo "$1" |
+		while read name sm_path sha1 subsha1
+		do
+			echo "$sm_path"
+		done
+		)
+	names=$(echo "$1" |
+		while read name sm_path sha1 subsha1
+		do
+			printf ' %s' "$name"
+		done
+		)
+	summary="$(eval_gettext "Updated submodules:")$names"
+	body=$(echo "$1" |
+		while read name sm_path sha1 subsha1
+		do
+			if test "$name" = "$sm_path"
+			then
+				printf 'Changes to %s:\n\n' "$name"
+			else
+				printf 'Changes to %s (%s):\n\n' "$name" "$sm_path"
+			fi
+			(
+				clear_local_git_env
+				cd "$sm_path" &&
+				git shortlog "${sha1}..${subsha1}" ||
+				die "$(eval_gettext "Unable to generate shortlog in submodule path '\$sm_path'")"
+			)
+		done
+		)
+	IFS="$OIFS"
+	message="$(printf '%s\n\n%s\n' "$summary" "$body")"
+	echo "message: [$message]"
+	git commit -m "$message" $paths
+}
+
+#
 # Add a new submodule to the working tree, .gitmodules and the index
 #
 # $@ = repo path
@@ -515,6 +562,9 @@ cmd_update()
 		-f|--force)
 			force=$1
 			;;
+		--commit)
+			commit=1
+			;;
 		-r|--rebase)
 			update="rebase"
 			;;
@@ -557,6 +607,7 @@ cmd_update()
 	fi
 
 	cloned_modules=
+	changed_modules=
 	module_list "$@" | {
 	err=
 	while read mode sha1 stage sm_path
@@ -660,6 +711,15 @@ Maybe you want to use 'update --init'?")"
 				err="${err};$die_msg"
 				continue
 			fi
+
+			subsha1=$(clear_local_git_env; cd "$sm_path" &&
+				git rev-parse --verify HEAD) ||
+			die "$(eval_gettext "Unable to find new revision in submodule path '\$sm_path'")"
+
+			if test "$subsha1" != "$sha1"
+			then
+				changed_modules=$(printf '%s%s\n' "$changed_modules" "$name;$sm_path;$sha1;$subsha1")
+			fi
 		fi
 
 		if test -n "$recursive"
@@ -680,6 +740,11 @@ Maybe you want to use 'update --init'?")"
 		fi
 	done
 
+	if test -z "$err" -a -n "$commit" -a -n "$changed_modules"
+	then
+		commit_changes "$changed_modules"
+	fi
+
 	if test -n "$err"
 	then
 		OIFS=$IFS
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 1542653..4c8bb5d 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -163,6 +163,25 @@ test_expect_success 'submodule update --merge staying on master' '
 	)
 '
 
+test_expect_success 'submodule update --commit --rebase should commit gitlink changes' '
+	(cd super/submodule &&
+	 git reset --hard HEAD~1 &&
+	 echo "local change" > local-file &&
+	 git add local-file &&
+	 test_tick &&
+	 git commit -m "local change"
+	) &&
+	(cd super &&
+	 git submodule update --commit --rebase submodule &&
+	 test "$(git log -1 --oneline)" = "bbdbe2d Updated submodules: submodule"
+	) &&
+	(cd submodule &&
+	 git remote add super-submodule ../super/submodule &&
+	 git pull super-submodule master
+	) &&
+  test "a" = "b"
+'
+
 test_expect_success 'submodule update - rebase in .git/config' '
 	(cd super &&
 	 git config submodule.submodule.update rebase
-- 
1.8.0.1.gaaf2ac7.dirty

Re: [RFC] git-submodule update: Add --commit option

From: W. Trevor King <hidden>
Date: 2016-06-15 22:55:27

On Thu, Nov 29, 2012 at 11:12:16AM -0500, W. Trevor King wrote:
+  test "a" = "b"
This kills the test (with --immediate) so you can look at the
generated commit.  If you actually want the test to pass (e.g. if this
becomes a PATCH and not an RFC), this line should be removed.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

Re: [RFC] git-submodule update: Add --commit option

From: W. Trevor King <hidden>
Date: 2016-06-15 22:55:27

On Thu, Nov 29, 2012 at 11:12:16AM -0500, W. Trevor King wrote:
+	 test "$(git log -1 --oneline)" = "bbdbe2d Updated submodules: submodule"
s/bbdbe2d/cd69713/

I forgot to update the SHA-1 here after tweaking the commit message
format.  I'd like to rewrite this test so it won't use the SHA-1, but
this was the quickest way to check that the commit message and gitlink
were both changed appropriately.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help