threads / patch / 10588

patchstatus&commit: Teach them to show commits of modified submodules.

Subject: [PATCH] status&commit: Teach them to show commits of modified submodules.

## tl;dr

11 messages between Nov 2, 2007 and Nov 7, 2007. Diffs are folded; open one to read it.

replies: 10people: 2as markdown or json

Ping Yin· Nov 2, 2007, 11:53 UTC · lore

git status/commit just treats submodules as ordinary files when reporting status changes. However, one may also wonder how the submodules change.

This commit teaches git status/commit to also show commits of modified submodules since HEAD (or HEAD^ if --amend option is on).

For example, when commiting, submodule sm1 and sm2 are both changed. sm1 has commit C in HEAD and
commit E in index. The history of sm1 is
	--A-->B-->C (in HEAD:354cd45)
	  \
	   -->D->E (in index:3f751e5)

git status will give the following output (just output commits of submodules before normal output) to show how to change from commit C (in HEAD) to commit E (in index): backward ('<<<') to commit A, and then forward ('>>>') to commit E.

	#
	# submodule modifiled: sm1 sm2
	#
	# * sm1 354cd45...3f751e5:
	#   <<<
	#   	one line message for C
	#   	one line message for B
	#   >>>
	#   	one line message for D
	#   	one line message for E
	#
	# * sm2 5c8bfb5...ac46d84:
	#   <<<
	#   	msg
	#
	# On branch master
	# Changes to be committed:
	#   (use "git reset HEAD <file>..." to unstage)
	#
	#	modified:   sm1
	#	modified:   sm2
---
 git-commit.sh |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)
Show changes to git-commit.sh +40 −0
diff --git a/git-commit.sh b/git-commit.sh
index fcb8443..d362caa 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -33,6 +33,40 @@ save_index () {
 	cp -p "$THIS_INDEX" "$NEXT_INDEX"
 }
 
+# Show log of modified submodule (index modification since HEAD or $1)
+show_module_log () {
+	modules=`git diff --cached --name-only $1 |
+	while read name
+	do
+		git ls-files --stage $name
+	done |
+	grep '^160000 ' | awk '{print $4}'`
+	test -z "$modules" && return
+
+	modules=$(echo $modules)
+	echo -e "#\n# submodule modifiled: $modules\n#"
+	for name in $modules
+	do
+		range=`git diff --cached -- $name | sed -n '2 p' | awk '{print $2}'`
+		indexone=${range#*..}
+		headone=${range%..*}
+		(
+			echo "* $name $headone...$indexone:"
+			cd $name >&/dev/null || { echo "  Warning: fail to chdir to $name" && exit; }
+			left="`git log --pretty=oneline $indexone..$headone 2>&1 | sed 's/^\w\+ /  \t/'`"
+			right="`git log --pretty=oneline --reverse  $headone..$indexone 2>&1 | sed 's/^\w\+ /  \t/'`"
+			if echo "$left$right" | grep 'unknown revision' >&/dev/null
+			then
+				echo "  Warning: $name is not a repository or dosn't contains commit $headone/$indexone"
+			else
+				test -n "$left" && echo -e "  <<<\n$left"
+				test -n "$right" && echo -e "  >>>\n$right"
+			fi
+			echo
+		) | sed 's/^/# /'
+	done
+}
+
 run_status () {
 	# If TMP_INDEX is defined, that means we are doing
 	# "--only" partial commit, and that index file is used
@@ -55,6 +89,12 @@ run_status () {
 	else
 		color=--nocolor
 	fi
+	if test -z "$amend"
+	then
+		show_module_log
+	else
+		show_module_log "HEAD^"
+	fi
 	git runstatus ${color} \
 		${verbose:+--verbose} \
 		${amend:+--amend} \
-- 
1.5.3.4
Junio C Hamano· Nov 2, 2007, 20:29 UTC · re: Ping Yin · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

How does this work when you are a toplevel developer and do not have the submodule cloned and checked out?

Our code should treat having the submodule directory and not having it when there is a mode 160000 entry in the index equally likely. Cloning and checking-out is _not_ the norm (nor the exception).

Yin Ping· Nov 2, 2007, 23:50 UTC · re: Junio C Hamano · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> How does this work when you are a toplevel developer and do not
> have the submodule cloned and checked out?
>
> Our code should treat having the submodule directory and not
> having it when there is a mode 160000 entry in the index equally
> likely.  Cloning and checking-out is _not_ the norm (nor the
> exception).
>

When submodule is not cheched out, it is never modified. In this case modules variable will be empty ( since git diff --cached is used) and the corrosponding module directory will not be checked. So the output will be the same as the original output.

-- 
franky
Junio C Hamano· Nov 3, 2007, 00:01 UTC · re: Yin Ping · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

"Yin Ping" <pkufranky@gmail.com> writes:
Show 10 quoted lines
> On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:
>> How does this work when you are a toplevel developer and do not
>> have the submodule cloned and checked out?
>>
>> Our code should treat having the submodule directory and not
>> having it when there is a mode 160000 entry in the index equally
>> likely.  Cloning and checking-out is _not_ the norm (nor the
>> exception).
>>
> When submodule is not cheched out, it is never modified.
How so?  Can't you update the index alone?
Yin Ping· Nov 4, 2007, 09:22 UTC · re: Junio C Hamano · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> "Yin Ping" <pkufranky@gmail.com> writes:
>
> > On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:
> >> How does this work when you are a toplevel developer and do not
> >> have the submodule cloned and checked out?
> >>
> >> Our code should treat having the submodule directory and not
> >> having it when there is a mode 160000 entry in the index equally
> >> likely.  Cloning and checking-out is _not_ the norm (nor the
> >> exception).
> >>
> > When submodule is not cheched out, it is never modified.
>
> How so?  Can't you update the index alone?
>
Sorry i have been away and come back just now.

I think it's a rare case that the submodule entry in the index is different from HEAD when the submodule is not checked out.

This may only happen when git-reset --soft is called. However, it may be a wrong operation to commit the changed submodule index entry while the submodule is never checked out.

There is another similar case, 'git pull' update the submodule entry in both HEAD and index, but the submodule checked out is not updated. In this case, 'git-log since..until' in the submodule directory will fail since 'until' commit is not checked out yet. I think it may be also a user mistake.

In both case, i think the user should be notified about the inconsistence. My patch handle this by two warning messages as follows (where $name is module name)

+                       cd $name >&/dev/null || { echo "  Warning:
fail to chdir to $name" && exit; }
+                       if echo "$left$right" | grep 'unknown
revision' >&/dev/null
+                       then
+                               echo "  Warning: $name is not a
repository or dosn't contains commit $headone/$indexone"
+                       else
-- 
franky
Yin Ping· Nov 4, 2007, 09:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

sorry, this is the reformated message of last one On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines
> "Yin Ping" <pkufranky@gmail.com> writes:
>
> > On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:
> >> How does this work when you are a toplevel developer and do not
> >> have the submodule cloned and checked out?
> >>
> >> Our code should treat having the submodule directory and not
> >> having it when there is a mode 160000 entry in the index equally
> >> likely.  Cloning and checking-out is _not_ the norm (nor the
> >> exception).
> >>
> > When submodule is not cheched out, it is never modified.
>
> How so?  Can't you update the index alone?
>
Sorry i have been away and come back just now.

I think it's a rare case that the submodule entry in the index is different from HEAD when the submodule is not checked out.

This may only happen when git-reset --soft is called. However, it may be a wrong operation to commit the changed submodule index entry while the submodule is never checked out.

There is another similar case, 'git pull' update the submodule entry in both HEAD and index, but the submodule checked out is not updated. In this case, 'git-log since..until' in the submodule directory will fail since 'until' commit is not checked out yet. I think it may be also a user mistake.

In both case, i think the user should be notified about the inconsistence. My patch handle this by two warning messages as follows (where $name is module name)

+                       cd $name >&/dev/null || { echo "  Warning:
fail to chdir to $name" && exit; }
+                       if echo "$left$right" | grep 'unknown
revision' >&/dev/null
+                       then
+                               echo "  Warning: $name is not a
repository or doesn't contains commit $headone/$indexone"
+                       else
-- 
franky
Yin Ping· Nov 4, 2007, 09:56 UTC · re: Yin Ping · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

On 11/4/07, Yin Ping <pkufranky@gmail.com> wrote:
> sorry, this is the reformated message of last one

Oh, I'm beaten my the mail client (gmail client will automatically wrap line to width 80, which is not suitable for patch). Just ignore my last messge and bear the wrapped patch line. Sorry for the inconvience again

-- 
franky
Junio C Hamano· Nov 4, 2007, 11:41 UTC · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

"Yin Ping" <pkufranky@gmail.com> writes:
Show 6 quoted lines
> In both case, i think the user should be notified about the inconsistence.
> My patch given in the first letter handles this by two warning messages as
> follows (where $name is module name)
>
> +                       cd $name >&/dev/null || { echo "  Warning: fail to
> chdir to $name" && exit; }

My point was that it is wrong to make this "you do not have that submodule checked out" a warning.

Think about somebody who does _not_ care about that particular submodule. He is working on one branch, and tries to merge another branch that has some interesting changes outside the submodule, but that branch also has updates to the submodule.

As he is _not_ interested in the submodule, he does not have it checked out. He has not changed the commit bound in his index with 160000 mode bits for that submodule since the common ancestor, either (that's what "he does not care about the submodule" means).

The merge will take the submodule commit object name from the other branches change, just like the ordinary three-way merge rule would do (one side changed, the other did not do anything, so take the change). Suppose some files in the project proper, outside the submodule, had conflicts. He has an unmerged index, that has the submodule already correctly merged. But other parts of his index are unmerged. git-status would guide him what are conflicting, and git-commit would show him what he would be committing.

In that situation, all he needs to know, with respect to the submodule, is that the submodule has been updated since his HEAD (and that is given by the runstatus output). He does not _care_ about what the individual commits in the submodule were. It is not an error that the information from the submodule cannot be shown to him. He _chose_ to ignore the details of that submodule by not checking it out to begin with.

Something like this, to be totally quiet, would be more appropriate.

	for name in $modules
        do
                (
                	... do the range, indexone, headone stuff
                        cd "$name" 2>/dev/null || exit 0
                	echo "* $name $headone...$indexone:"
			... whatever log you show
		) | sed ...
	done

By the way, I do not know about the quoting issues with $modules variable in the above illustration, as I am not (yet) discussing about the implementation level of details.

Yin Ping· Nov 4, 2007, 13:17 UTC · re: Junio C Hamano · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

On 11/4/07, Junio C Hamano <gitster@pobox.com> wrote:
Show 30 quoted lines
> "Yin Ping" <pkufranky@gmail.com> writes:
>
> > In both case, i think the user should be notified about the inconsistence.
> > My patch given in the first letter handles this by two warning messages as
> > follows (where $name is module name)
> >
> > +                       cd $name >&/dev/null || { echo "  Warning: fail to
> > chdir to $name" && exit; }
>
> In that situation, all he needs to know, with respect to the
> submodule, is that the submodule has been updated since his HEAD
> (and that is given by the runstatus output).  He does not _care_
> about what the individual commits in the submodule were.  It is
> not an error that the information from the submodule cannot be
> shown to him.  He _chose_ to ignore the details of that
> submodule by not checking it out to begin with.
>
> Something like this, to be totally quiet, would be more
> appropriate.
>
>        for name in $modules
>        do
>                (
>                        ... do the range, indexone, headone stuff
>                        cd "$name" 2>/dev/null || exit 0
>                        echo "* $name $headone...$indexone:"
>                        ... whatever log you show
>                ) | sed ...
>        done
>

Your point is in some cases people think the warning messages are annoying because they don't care the change details for submodules. However, in some cases these messages are helpful. And a third kind of cases is that people care about only part of all submodules.

So, maybe some an switch can be used to turn this on or off (default off)? For example # only sm1 and sm2 are cared git commit --submodule=sm1,sm2

# all submodules are cared git commit --submodule='*'

> By the way, I do not know about the quoting issues with $modules
> variable in the above illustration, as I am not (yet) discussing
> about the implementation level of details.
>
-- 
franky
Junio C Hamano· Nov 6, 2007, 02:22 UTC · re: Yin Ping · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

"Yin Ping" <pkufranky@gmail.com> writes:
Show 5 quoted lines
> However, in some cases these messages are helpful. And a third kind of
> cases is that people care about only part of all submodules.
>
> So, maybe some an switch can be used to turn this on or off (default
> off)?

I personally think that is overengineering. Isn't having/not having the submodule directory cloned a good enough indicator of which submodules are interested in by the user?

Yin Ping· Nov 7, 2007, 15:20 UTC · re: Junio C Hamano · lore

Re: [PATCH] status&commit: Teach them to show commits of modified submodules.

On 11/6/07, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> "Yin Ping" <pkufranky@gmail.com> writes:
>
> > However, in some cases these messages are helpful. And a third kind of
> > cases is that people care about only part of all submodules.
> >
> > So, maybe some an switch can be used to turn this on or off (default
> > off)?
>
> I personally think that is overengineering.  Isn't having/not
> having the submodule directory cloned a good enough indicator
> of which submodules are interested in by the user?
>
Good suggestion. I'll give my new patch later.
-- 
franky

← back to recent threads