git-submodule: add support for --rebase.

'git submodule update --rebase' rebases onto the current branch rather than detaching the HEAD.

I use git-submodule to keep track of repositories that I only infrequently commit to. I keep them to have a set that is known to work.

git submodule update is annoying, as it (in my case needlessly) detaches the HEAD, making the workflow more complicated when I do have to commit (checkout master, rebase onto master, then commit).

This patch adds a "--rebase" flag to git submodule update that calls git-rebase instead of git-checkout.

git-submodule.sh +2 -3

@@ -367,11 +372,19 @@ cmd_update()
 				die "Unable to fetch in submodule path '$path'"
 			fi
 
-			(unset GIT_DIR; cd "$path" &&
-				  git-checkout $force -q "$sha1") ||
-			die "Unable to checkout '$sha1' in submodule path '$path'"
+			if test -z "$rebase"
+			then

Disclaimer: if you are offended by constructive criticism, or likely to answer with insults to the comments I offer, please stop reading this mail now (and please do not answer my mail, either). :-)

Still with me? Good. Nice to meet you.

Just for the record: responding to a patch is my strongest way of saying that I appreciate your work.

Quoted 2 lines

'git submodule update --rebase' rebases onto the current branch rather than detaching the HEAD.

I know what you want to do, but this text is wrong: it should rather read something like this:

'git submodule update --rebase' rebases your local branch on top of what would have been checked out to a detached HEAD otherwise.

Maybe some of these native English speakers on this list can come up with something even better ;-)

Quoted 7 lines

I use git-submodule to keep track of repositories that I only infrequently commit to. I keep them to have a set that is known to work.

git submodule update is annoying, as it (in my case needlessly) detaches the HEAD, making the workflow more complicated when I do have to commit (checkout master, rebase onto master, then commit).

Quoted 2 lines

This patch adds a "--rebase" flag to git submodule update that calls git-rebase instead of git-checkout.

Maybe a non-first person version of this comment could go into the commit message, too? I found this highly informative.

Quoted 8 lines

Documentation/git-submodule.txt +0 -0

index 3b8df44..117ad3d 100644
@@ -177,6 +178,12 @@ OPTIONS
 	This option is only valid for the update command.
 	Don't fetch new objects from the remote site.
 

Documentation/git-submodule.txt OPTIONS · line 182

Quoted 2 lines

Documentation/git-submodule.txt +2 -0

+--rebase::
+	This option is only valid for the update command.

This is unnecessary, it was mentioned in the synopsis.

Documentation/git-submodule.txt OPTIONS · line 183

Quoted 1 line

Documentation/git-submodule.txt +1 -0

+	Forward-port local commits to the index of the containing repository.

This is a bit misleading/unclear. I'd rather have it read like this:

Instead of detaching the HEAD to the revision committed in the superproject, rebase the current branch onto that revision.

Quoted 5 lines

Documentation/git-submodule.txt +3 -0

+	If a a merge failure prevents this process, you will have to resolve
+	these failures with linkgit:git-rebase[1].
+
 <path>...::
 	Paths to submodule(s). When specified this will restrict the command

git-submodule.sh

Quoted 5 lines

Documentation/git-submodule.txt +0 -0

 	to only operate on the submodules found at the specified paths.

git-submodule.sh +0 -0

index 7c2e060..6180bf4 100755

You might want to error out when --rebase was passed with another command than "update".

Quoted 9 lines

git-submodule.sh +2 -3

@@ -367,11 +372,19 @@ cmd_update()
 				die "Unable to fetch in submodule path '$path'"
 			fi
 
-			(unset GIT_DIR; cd "$path" &&
-				  git-checkout $force -q "$sha1") ||
-			die "Unable to checkout '$sha1' in submodule path '$path'"
+			if test -z "$rebase"
+			then

git-submodule.sh cmd_update() · line 386

Quoted 10 lines

git-submodule.sh +10 -0

+				(unset GIT_DIR; cd "$path" &&
+					  git-checkout $force -q "$sha1") ||
+				die "Unable to checkout '$sha1' in submodule path '$path'"
+				say "Submodule path '$path': checked out '$sha1'"
+			else
+				(unset GIT_DIR; cd "$path" &&
+					git-rebase "$sha1") ||
+					die "Unable to rebase onto '$sha1' in submodule path '$path'"
+				say "Submodule path '$path': rebased onto '$sha1'"
+			fi

I'd actually put the "(unset GIT_DIR; cd "$path" &&" in front of the "if" due to the DRY principle (Don't Repeat Yourself).

In the same spirit, I'd set a variable "action" to "checkout" or "rebase onto" and munge the error/info message to use $action.

Maybe you want to add a single primitive test case to make sure this feature will not get broken in the future?

Other than that: nice! very nice!

Ciao,
Dscho

P.S.: the next patch is obvious, too: add support to specify desire to rebase the submodule in .gitmodules and .git/config.

From b5ed1a17aa9b6042f68a546ceb3990b65d0862e0 Mon Sep 17 00:00:00 2001

Quoted 10 lines · Johannes Schindelin

Peter wrote originally:

Quoted 8 lines

Documentation/git-submodule.txt +0 -0

index 3b8df44..117ad3d 100644
@@ -177,6 +178,12 @@ OPTIONS
 	This option is only valid for the update command.
 	Don't fetch new objects from the remote site.
 

Quoted 4 lines

Documentation/git-submodule.txt OPTIONS · line 182

Quoted 2 lines

Documentation/git-submodule.txt +2 -0

+--rebase::
+	This option is only valid for the update command.

This is unnecessary, it was mentioned in the synopsis.

Correct, but in the same file the options --cached, --no-fetch, and --summary-limit do state that they are only valid for the update/status/summary commands, respectively. For consistency, we should either add this sentence to --rebase, or remove them from the others. I personally prefer having it there, just to make it more obvious.

Quoted 6 lines

Documentation/git-submodule.txt OPTIONS · line 183

Quoted 1 line

Documentation/git-submodule.txt +1 -0

+	Forward-port local commits to the index of the containing repository.

This is a bit misleading/unclear. I'd rather have it read like this:

Instead of detaching the HEAD to the revision committed in the superproject, rebase the current branch onto that revision.

Hehe. I had something like this before and then decided to copy/paste the line from the git-rebase man page :)

Changed to: "Rebase the current branch onto the index of the containing repository instead of detaching the HEAD." (I vaguely remember the rule that sentences are easier to understand if you have "blah instead of foo" rather than "instead of foo, blah")

The phrase "index of the containing repository" is commonly used in this man page, so I think it's best to stick with it. That better now?

Quoted 5 lines

Quoted 5 lines

Documentation/git-submodule.txt +3 -0

+	If a a merge failure prevents this process, you will have to resolve
+	these failures with linkgit:git-rebase[1].
+
 <path>...::
 	Paths to submodule(s). When specified this will restrict the command

Quoted 8 lines

git-submodule.sh

Quoted 5 lines

Documentation/git-submodule.txt +0 -0

 	to only operate on the submodules found at the specified paths.

git-submodule.sh +0 -0

index 7c2e060..6180bf4 100755

You might want to error out when --rebase was passed with another command than "update".

cmd_init(), cmd_summary(), etc. have catch-all rules for unknown options to display the usage, so this is covered. (e.g. line 439, git-submodule.sh)