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

11 messages from 2007-11-02 to 2007-11-07. Participants: Ping Yin, Junio C Hamano, Yin Ping.
Thread: https://gitlist.dev/t/10588

## Ping Yin, 2007-11-02 11:53

Subject: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <1194004427-26934-1-git-send-email-pkufranky@gmail.com>
URL: https://gitlist.dev/e/1194004427-26934-1-git-send-email-pkufranky%40gmail.com

```
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(-)

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, 2007-11-02 20:29

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <7vd4us1jds.fsf@gitster.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vd4us1jds.fsf%40gitster.siamese.dyndns.org
In-Reply-To: <1194004427-26934-1-git-send-email-pkufranky@gmail.com>

```
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, 2007-11-02 23:50

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <46dff0320711021650q4e56d025q63a961176c682a14@mail.gmail.com>
URL: https://gitlist.dev/e/46dff0320711021650q4e56d025q63a961176c682a14%40mail.gmail.com
In-Reply-To: <7vd4us1jds.fsf@gitster.siamese.dyndns.org>

```
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. 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, 2007-11-03 00:01

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <7vfxzoyz7f.fsf@gitster.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vfxzoyz7f.fsf%40gitster.siamese.dyndns.org
In-Reply-To: <46dff0320711021650q4e56d025q63a961176c682a14@mail.gmail.com>

```
"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?

```

## Yin Ping, 2007-11-04 09:22

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <46dff0320711040122o271b3da4wb16b0317224d3ac3@mail.gmail.com>
URL: https://gitlist.dev/e/46dff0320711040122o271b3da4wb16b0317224d3ac3%40mail.gmail.com
In-Reply-To: <7vfxzoyz7f.fsf@gitster.siamese.dyndns.org>

```
On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:
> "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, 2007-11-04 09:25

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <46dff0320711040125v111c75davb49e9822537f4b19@mail.gmail.com>
URL: https://gitlist.dev/e/46dff0320711040125v111c75davb49e9822537f4b19%40mail.gmail.com
In-Reply-To: <7vfxzoyz7f.fsf@gitster.siamese.dyndns.org>

```
sorry, this is the reformated message of last one
On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:
> "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, 2007-11-04 09:56

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <46dff0320711040156s7a780a8dp7c76f1cbf3fd6e60@mail.gmail.com>
URL: https://gitlist.dev/e/46dff0320711040156s7a780a8dp7c76f1cbf3fd6e60%40mail.gmail.com
In-Reply-To: <46dff0320711040125v111c75davb49e9822537f4b19@mail.gmail.com>

```
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, 2007-11-04 11:41

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <7vmytus0ff.fsf@gitster.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vmytus0ff.fsf%40gitster.siamese.dyndns.org
In-Reply-To: <46dff0320711040145k1edb1fcaq1daa5469c1158e81@mail.gmail.com>

```
"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; }

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, 2007-11-04 13:17

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <46dff0320711040517r6da5d7aaid849ff06df1b5bb6@mail.gmail.com>
URL: https://gitlist.dev/e/46dff0320711040517r6da5d7aaid849ff06df1b5bb6%40mail.gmail.com
In-Reply-To: <7vmytus0ff.fsf@gitster.siamese.dyndns.org>

```
On 11/4/07, Junio C Hamano <gitster@pobox.com> wrote:
> "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, 2007-11-06 02:22

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <7vwsswjeom.fsf@gitster.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vwsswjeom.fsf%40gitster.siamese.dyndns.org
In-Reply-To: <46dff0320711040517r6da5d7aaid849ff06df1b5bb6@mail.gmail.com>

```
"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?

```

## Yin Ping, 2007-11-07 15:20

Subject: Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
Message-ID: <46dff0320711070720j2af83fabm28cdbf03b82d7862@mail.gmail.com>
URL: https://gitlist.dev/e/46dff0320711070720j2af83fabm28cdbf03b82d7862%40mail.gmail.com
In-Reply-To: <7vwsswjeom.fsf@gitster.siamese.dyndns.org>

```
On 11/6/07, Junio C Hamano <gitster@pobox.com> wrote:
> "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

```