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

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

[Ping Yin]

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

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

[Junio C Hamano]

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).

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

[Yin Ping]

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

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

[Junio C Hamano]

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

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

[Yin Ping]

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

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

[Yin Ping]

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

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

[Yin Ping]

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

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

[Junio C Hamano]

"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.

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

[Yin Ping]

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

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

[Junio C Hamano]

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

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

[Yin Ping]

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

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

[Ping Yin]

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

This commit teaches git status/commit to also show commits of user-cared
modified submodules since HEAD (or HEAD^ if --amend option is on).
Notes:
	1. Submodules already checked out are considered to be user-cared ones.
	2. For submodules deleted or initially added, commits are not shown.

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 the original output) to show how to change from commit C (in HEAD) to
commit E (in index) for submodule sm1: backward ('<<<') to commit A, and then
forward ('>>>') to commit E. Similar illustration for output of sm2 is omitted.

	#
	# 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

For sm1, if the commit recorded in HEAD/index (say commit C/E) is not found in
the work tree (say sm1 respository in the work tree), a warning will be issued.

	#
	# submodule modifiled: sm1
	#
	# * sm1 354cd45...3f751e5:
	#   Warn: sm1 doesn't contains commit 354cd45
	#
	# On branch master
	# Changes to be committed:
	#   (use "git reset HEAD <file>..." to unstage)
	#
	#	modified:   sm1

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 git-commit.sh |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index fcb8443..29f2ebe 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -33,6 +33,68 @@ save_index () {
 	cp -p "$THIS_INDEX" "$NEXT_INDEX"
 }
 
+# Show log of modified submodule (index modification since HEAD or $1)
+# $1 is the commit to be compared, default 'HEAD'
+show_module_log () {
+	cwd=$(pwd)
+	cd_to_toplevel 
+
+	# get modified modules which have been checked out (i.e. cared by user)
+	modules=$(git diff --cached --name-only $1 |
+	(
+		IFS=''	# handle the module name containing space or tab
+		while read name
+		do
+			git ls-files --stage "$name" | grep '^160000 ' >&/dev/null &&
+			GIT_DIR="$name/.git" git-rev-parse --git-dir >&/dev/null &&
+			echo "$name"
+		done
+	)
+	)
+
+	# TODO: quote module names containing space or tab
+	test -n "$modules" && echo -e "#\n# submodule modifiled: "$modules"\n#"
+	OLDIFS=$IFS
+	IFS=$'\n\r'	# '\r' for mac os 
+	for name in $modules
+	do
+		range=$(git diff --cached -- "$name" | sed -n '/^index.*160000$/ p' | awk '{print $2}')
+		indexone=${range#*..}
+		headone=${range%..*}
+		(
+			echo "* $name $headone...$indexone:"
+			headfail=
+			indexfail=
+			GIT_DIR="$name/.git" git-rev-parse $headone >&/dev/null || headfail='t'
+			GIT_DIR="$name/.git" git-rev-parse $indexone >&/dev/null || indexfail='t'
+			case "$headfail,$indexfail" in
+			t,)
+				echo "  Warn: $name dosn't contains commit $headone"
+				;;
+			,t)
+				echo "  Warn: $name dosn't contains commit $indexone"
+				;;
+			t,t)
+				echo "  Warn: $name dosn't contains commits $headone and $indexone"
+				;;
+			*)
+				left=$(GIT_DIR="$name/.git" git log --pretty=oneline $indexone..$headone 2>&1 |
+				sed 's/^\w\+ /  \t/')
+				right=$(GIT_DIR="$name/.git" git log --pretty=oneline --reverse  $headone..$indexone 2>&1 |
+				sed 's/^\w\+ /  \t/')
+
+				test -n "$left" && echo -e "  <<<\n$left"
+				test -n "$right" && echo -e "  >>>\n$right"
+				;;
+			esac
+			echo
+		) | sed 's/^/# /'
+	done
+	IFS=$OLDIFS
+
+	cd "$cwd"
+}
+
 run_status () {
 	# If TMP_INDEX is defined, that means we are doing
 	# "--only" partial commit, and that index file is used
@@ -55,6 +117,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

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

[Sven Verdoolaege]

On Sun, Nov 11, 2007 at 03:27:43AM +0800, Ping Yin wrote:
> This commit teaches git status/commit to also show commits of user-cared

Does it?  It looks like you only changed git-commit.
Shouldn't this be put in wt_status_print, if anywhere?

Also, you have some typos:

> +	test -n "$modules" && echo -e "#\n# submodule modifiled: "$modules"\n#"
[..]
> +				echo "  Warn: $name dosn't contains commit $headone"

skimo

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

[Sven Verdoolaege]

On Sat, Nov 10, 2007 at 08:55:09PM +0100, Sven Verdoolaege wrote:
> On Sun, Nov 11, 2007 at 03:27:43AM +0800, Ping Yin wrote:
> > This commit teaches git status/commit to also show commits of user-cared
> 
> Does it?  It looks like you only changed git-commit.

OK.  Never mind about this one.

skimo

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

[Junio C Hamano]

Ping Yin <pkufranky@gmail.com> writes:

> 	# 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

I think this presentation order is horrible.

 * I think everbody preferes to have "On branch master" at the
   very beginning, to avoid committing to a wrong branch by
   mistake.

 * As I understand it, in the real life use, there will be quite
   many commits from the submodule updates when a new commit is
   bound to a submodule in the superproject, as _the_ point of
   having a submodule is to bind a more or less independent
   project that progresses at quite a different pace as a
   submodule to the superproject.  In other words, by design,
   the superproject can stay behind from the tip of subproject
   and rebind it to a different commit only when there are
   significant changes of the subproject that need to be there
   to allow the other parts of the superproject (either
   superproject itself or another submodule) to use the features
   and/or fixes the submodule updates provides.

   Which means it will not be uncommon have hundreds of "one
   line message" for the submodules at the very beginning of the
   commit log message buffer, and your prsentation order will
   make that part overwhelm the overview of what changed _in_
   the supermodule itself (the "Changes to be committed:"
   lines), which gives the birds-eye view.

   And I think it is more important to give the birds-eye view
   of the supermodule itself first, when you are helping to
   prepare a commit message for the supermodule.  The user would
   start the commit log for the superproject with "This updates
   the new frotz feature.  It uses the updated API from the
   submodules A and B so we now use updated versions of them."
   and then continue "Notable changes in submodule A are ...".
   And the new part you are adding would help the user to write
   the latter description.

I also find "<<< lines then >>> other lines" format very hard to
read.  Maybe formatting it like this would make it a bit more
readable and more space efficient?

 	# * 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

Note that if you swap the order and move this at the tail
(perhaps before "Untracked files:" section, if you do not have a
decent .gitignore set up), you can also lose the "submodules
modified: sm1 sm2" line and the blank line before it, which
would make the output even shorter without losing any useful
information.

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

[Lars Hjemli]

On Nov 10, 2007 8:27 PM, Ping Yin <pkufranky@gmail.com> wrote:
> This commit teaches git status/commit to also show commits of user-cared
> modified submodules since HEAD (or HEAD^ if --amend option is on).

Some nitpicks:
-we'll need a config option to enable/disable this output in git-status
-the feature should probably be implemented in git-submodule.sh

--
larsh

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

[Yin Ping]

On Nov 11, 2007 3:55 AM, Sven Verdoolaege <skimo@kotnet.org> wrote:
> On Sun, Nov 11, 2007 at 03:27:43AM +0800, Ping Yin wrote:
> > This commit teaches git status/commit to also show commits of user-cared
>
> Does it?  It looks like you only changed git-commit.
> Shouldn't this be put in wt_status_print, if anywhere?
>
git-commit and git-status correspond to the same script git-commit.sh
> Also, you have some typos:
>
> > +     test -n "$modules" && echo -e "#\n# submodule modifiled: "$modules"\n#"
> [..]
> > +                             echo "  Warn: $name dosn't contains commit $headone"
>
> skimo
>
Oops, i'll fix it



-- 
Ping Yin

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

[Yin Ping]

On Nov 11, 2007 5:14 AM, Junio C Hamano <gitster@pobox.com> wrote:
>  * I think everbody preferes to have "On branch master" at the
>    very beginning

Reasonable.

>
>  * As I understand it, in the real life use,
>    the superproject can stay behind from the tip of subproject
>    and rebind it to a different commit only when there are
>    significant changes of the subproject that need to be there
>    to allow the other parts of the superproject (either
>    superproject itself or another submodule) to use the features
>    and/or fixes the submodule updates provides.

I think it's this kind of case in most open-source project. However,
in a company environment, superprojects may be not so super. A
superproject may bind very tightly with submodules (such as the html
template files which change very frequently) and the developer of a superproject
and its submodules may be the same guy(s). In these cases, a long list
of commits
for submodules are expected be reviewed when commiting the superproject.
>
>    And I think it is more important to give the birds-eye view
>    of the supermodule itself first, when you are helping to
>    prepare a commit message for the supermodule.
>    and then continue "Notable changes in submodule A are ...".
>    And the new part you are adding would help the user to write
>    the latter description.
I agree.
>
> I also find "<<< lines then >>> other lines" format very hard to
> read.  Maybe formatting it like this would make it a bit more
> readable and more space efficient?
>
>         # * 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
>
I have struggled between these two kinds of presentation and finally
choose the '<<<' one.
IMHO, '-/+' one each line will distract and less space/size efficient
(100 '+/-' for 100 lines of messages).

However, it's not a big matter. I'll change the presentation if
everyone prefers the
patch-like one.

> Note that if you swap the order and move this at the tail
> (perhaps before "Untracked files:" section, if you do not have a
> decent .gitignore set up), you can also lose the "submodules
> modified: sm1 sm2" line and the blank line before it, which
> would make the output even shorter without losing any useful
> information.
>
So following is ok?
        # On branch master
        # Changes to be committed:
        #   (use "git reset HEAD <file>..." to unstage)
        #
        #       modified:   sm1
        #       modified:   sm2
        #       modified:   sm3
        #
        # Changed but not updated:
        #   (use "git add/rm <file>..." to update what will be committed)
        #
        #       modified:   file1
        #
        # Submodules modifiled: sm1 sm2 sm3
        #
        # * sm1 354cd45...3f751e5:
        #   - one line message for C
        #   - one line message for B
        #   + one line message for D
        #   + one line message for E
        # * sm2 354cd46...3f751e7:
        #   - one line message
        # * sm3 354cd47...3f751e8:
        #   Warn: sm1 doesn't contains commit 354cd45
        #
        # Untracked files:
        #   (use "git add <file>..." to include in what will be committed)
        #
        #       file2
        #



-- 
Ping Yin

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

[Yin Ping]

On Nov 11, 2007 8:07 AM, Lars Hjemli <hjemli@gmail.com> wrote:
> On Nov 10, 2007 8:27 PM, Ping Yin <pkufranky@gmail.com> wrote:
> > This commit teaches git status/commit to also show commits of user-cared
> > modified submodules since HEAD (or HEAD^ if --amend option is on).
>
> Some nitpicks:
> -we'll need a config option to enable/disable this output in git-status
agree. default off?
> -the feature should probably be implemented in git-submodule.sh
>
I'll want to see the commits of submodules when editing commit msg. So
i implemented
this in git-commit.sh. May be a configuration/option can added to turn
this on or off.

> --
> larsh
>



-- 
Ping Yin

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

[Lars Hjemli]

On Nov 11, 2007 7:24 AM, Yin Ping <pkufranky@gmail.com> wrote:
> On Nov 11, 2007 8:07 AM, Lars Hjemli <hjemli@gmail.com> wrote:
> > On Nov 10, 2007 8:27 PM, Ping Yin <pkufranky@gmail.com> wrote:
> > > This commit teaches git status/commit to also show commits of user-cared
> > > modified submodules since HEAD (or HEAD^ if --amend option is on).
> >
> > Some nitpicks:
> > -we'll need a config option to enable/disable this output in git-status
> agree. default off?

That would be nice.

> > -the feature should probably be implemented in git-submodule.sh
> >
> I'll want to see the commits of submodules when editing commit msg.

If git-commit.sh uses git-submodule.sh to get this information, the
feature is still available in git-submodule even if it's disabled for
git-status.

--
larsh

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

[Junio C Hamano]

"Yin Ping" <pkufranky@gmail.com> writes:

> I think it's this kind of case in most open-source project. However,
> in a company environment, superprojects may be not so super.

Let's not say "most open-source" nor "company", because I think
nobody said anything that substantiates that the commit density
characteristics I described is typical for most open-source, nor
what you said is typical for corporate development projects, in
this thread so far.

If "superprojects is not so super", why are you using submodule
to bind these, instead of using a single project that tracks
developments of such closely tied parts?

I am not saying that it is wrong to use submodule to track such
groups of source trees whose versions are very closely tied
together.  At least not yet.

I am just trying to find out what benefit you are getting out of
the submodule support, after rejecting one of the most visible
and advertised benefit of submodule support, which is to enable
binding "related but not that closely tied together" projects.

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

[Ping Yin]

On Nov 12, 2007 4:34 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> If "superprojects is not so super", why are you using submodule
> to bind these, instead of using a single project that tracks
> developments of such closely tied parts?
>
For small modules crossing over mutiple projects, the submodule way
may be more suitable. And such submodule (such as common config files,
common templates) may be binded tightly with superproject

>



-- 
Ping Yin

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

[Johannes Sixt]

Junio C Hamano schrieb:
> "Yin Ping" <pkufranky@gmail.com> writes:
> 
>> I think it's this kind of case in most open-source project. However,
>> in a company environment, superprojects may be not so super.
> 
> Let's not say "most open-source" nor "company", because I think
> nobody said anything that substantiates that the commit density
> characteristics I described is typical for most open-source, nor
> what you said is typical for corporate development projects, in
> this thread so far.
> 
> If "superprojects is not so super", why are you using submodule
> to bind these, instead of using a single project that tracks
> developments of such closely tied parts?

Because the a monolithic project is just too large? Think of KDE!

> I am not saying that it is wrong to use submodule to track such
> groups of source trees whose versions are very closely tied
> together.  At least not yet.

In KDE, the supermodule will actually just be a container that binds the 
submodules together. The essential development will happen in the 
submodules, and the supermodule will receive a commit quite frequently. In 
this case, there will often be only a few or a few dozen commits listed, and 
I anticipate that the integrator who is going to make the commit (to the 
supermodule) will probably like the summary. So I'm all for it.

-- Hannes

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

[Johan Herland]

On Sunday 11 November 2007, Junio C Hamano wrote:
> "Yin Ping" <pkufranky@gmail.com> writes:
> 
> > I think it's this kind of case in most open-source project. However,
> > in a company environment, superprojects may be not so super.
> 
> Let's not say "most open-source" nor "company", because I think
> nobody said anything that substantiates that the commit density
> characteristics I described is typical for most open-source, nor
> what you said is typical for corporate development projects, in
> this thread so far.
> 
> If "superprojects is not so super", why are you using submodule
> to bind these, instead of using a single project that tracks
> developments of such closely tied parts?
> 
> I am not saying that it is wrong to use submodule to track such
> groups of source trees whose versions are very closely tied
> together.  At least not yet.
> 
> I am just trying to find out what benefit you are getting out of
> the submodule support, after rejecting one of the most visible
> and advertised benefit of submodule support, which is to enable
> binding "related but not that closely tied together" projects.

At $dayjob, we are working on a codebase roughly the same size as current 
linux-kernel with about 8 years of history in CVS. I'm currently looking at 
how suitable git would be for our revision control purposes (and so far I'm 
lovin' it).

The codebase is divided into CVS modules; most modules (aka. "core" modules) 
each have their own in-house maintainer and have internal releases with 
variable frequency. The other modules (aka. "platform/product" modules) 
each pull together a carefully chosen set of "core" modules as submodules, 
and add platform code to create - in the end - a complete product (with its 
own release frequency). Specifically:

- All the modules required by the product must be present in the checkout 
before a build can be made

- All the modules are independently developed, with different 
development/release timelines

- The "core" people only focus on 1-2 modules at a time, but 
the "platform/product" people might make changes in _many_ modules during a 
workday.

When investigating how to mesh this workflow with git, I naturally ended up 
with converting each CVS module to a git repository, and making 
the "platform/product" repos include the required "core" repos as 
submodules. This decision has the following effect from git's POV:

- "superproject is not so super" in that _all_ required modules must be 
checked out before a build can be made. In other words: all the submodules 
in a repo are "interesting"

- The modules are "related but not that closely tied together" since they 
follow separate development schedules, with separate releases, etc.

- The "platform/product" people will most certainly want to have commands 
like "git diff", "git status", and maybe even "git log" and "git-commit" 
recurse into submodules.

- The "core" people will probably not want "recurse-into-submodules" 
behaviour, although I can see places where it could be useful for them as 
well.


A possible solution to the above problem is to add 
a '--recurse-into-submodules' option to all relevant git commands. At the 
same time, the actual implementation of submodule recursion should probably 
be kept in the vicinity of "git-submodule" (instead of spreading it across 
the other git commands).

Probably unrealistic: Maybe we could solve the problem by adding
"--recurse-into-submodules" to the toplevel 'git' command itself, and make 
it re-invoke itself recursively in each submodule.


Hope this gives you insight into how _some_ people would like to use git's 
submodule support.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

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

[Johannes Schindelin]

Hi,

On Mon, 12 Nov 2007, Johannes Sixt wrote:

> Junio C Hamano schrieb:
>
> > I am not saying that it is wrong to use submodule to track such groups 
> > of source trees whose versions are very closely tied together.  At 
> > least not yet.
> 
> In KDE, the supermodule will actually just be a container that binds the 
> submodules together. The essential development will happen in the 
> submodules, and the supermodule will receive a commit quite frequently. 
> In this case, there will often be only a few or a few dozen commits 
> listed, and I anticipate that the integrator who is going to make the 
> commit (to the supermodule) will probably like the summary. So I'm all 
> for it.

I like it, too.  And we can make the number of shown commits configurable, 
just like for the merge summary.  But I'd rather see the code in 
wt-status.c than in git-submodule.sh.

Ciao,
Dscho

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

[Johannes Sixt]

Junio C Hamano schrieb:
> I also find "<<< lines then >>> other lines" format very hard to
> read.  Maybe formatting it like this would make it a bit more
> readable and more space efficient?
> 
>  	# * 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

How about the equivalent of

	git log --left-right --pretty=oneline --topo-order 354cd45...3f751e5

which would be

   	# * sm1 354cd45...3f751e5:
   	#   <one line message for C
   	#   <one line message for B
   	#   >one line message for D
   	#   >one line message for E

-- Hannes

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Mon, 12 Nov 2007, Johannes Sixt wrote:
>
>> Junio C Hamano schrieb:
>>
>> > I am not saying that it is wrong to use submodule to track such groups 
>> > of source trees whose versions are very closely tied together.  At 
>> > least not yet.
>> 
>> In KDE, the supermodule will actually just be a container that binds the 
>> submodules together. The essential development will happen in the 
>> submodules, and the supermodule will receive a commit quite frequently. 
>> In this case, there will often be only a few or a few dozen commits 
>> listed, and I anticipate that the integrator who is going to make the 
>> commit (to the supermodule) will probably like the summary. So I'm all 
>> for it.
>
> I like it, too.  And we can make the number of shown commits configurable, 
> just like for the merge summary.

Very good point.  In the case J6t uses for his illustration
above, changing the submodule bound to the superproject is more
or less like merging.

> But I'd rather see the code in wt-status.c than in
> git-submodule.sh.

I do not have a strong preference either way, but submodule-loving
people may want to say "git submodule shortlog <path>" or whatever
from the command line.  

Making a standalone function that takes two commits from the
subproject and produces the output, and calling that function
from both git-submodule (to implement the above "shortlog"
subcommand) and from wt-status.c (to show what Yin wants to add,
only when "status.submodule" is set), would be a reasonable
implementation.