Re: [PATCH] better git-submodule status output

[Junio C Hamano <gitster@pobox.com>]

Sylvain Joyeux <sylvain.joyeux@dfki.de> writes:

> This patch makes the output of git-submodule more useful to handle the
> management of a repository using multiple submodules. Namely, it
> displays informations about how the current checked out version relates
> to the registered one (i.e. direct parent, direct child, "just
> different", registered revision does not exist), and displays if the
> submodules contain uncommitted changes.
>
> This (among other things) allows to do git-submodule update while
> knowing exactly what will happen.
> --
> Sylvain
>
>>From 16553a9b210a956b0af961d55a9cf06f1b9b8114 Mon Sep 17 00:00:00 2001
> From: Sylvain Joyeux <sylvain.joyeux@dfki.de>
> Date: Tue, 1 Jul 2008 16:01:01 +0200
> Subject: [PATCH] more information in git-submodule status output
>
> This commit adds more information in the 'status' output of
> git-submodule. More specifically, it displays different flags if the
> submodule and the registered revision are direct parents (> and <,
> depending on which is the ancestor), if they are not direct parents (+)
> or if the registered revision cannot be found (i.e. if submodule update
> would fail, '!')
>
> Finally, it shows if the submodule contains uncommitted changes (M flag)

Which one is the commit message ;-)?

People who rely on working submodule support, do you have any feedback on
this patch?  I do not use submodule myself, so it is hard for me to judge
how much value (if any) this patch is adding to the real world use of the
status subcommand.

> @@ -97,7 +110,7 @@ for details.
>  
>  AUTHOR
>  ------
> -Written by Lars Hjemli <hjemli@gmail.com>
> +Written by Lars Hjemli <hjemli@gmail.com> and Sylvain Joyeux <sylvain.joyeux@m4x.org>

That is somehow inconsistent with what

	git-shortlog -s -n -e --no-merges git-submodule.sh 

tells me.

Honestly, I'd prefer (1) drop these "AUTHOR" lines, or (2) only list the
primary author or two.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 3eb78cc..e2b91f6 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -562,20 +566,40 @@ cmd_status()
> ...
>  		if git diff-files --quiet -- "$path"
>  		then
> -			say " $sha1 $path$revname"
> +			say " $unclean $sha1 $path$revname"
>  		else
> +                        head=$(unset GIT_DIR; cd "$path" && git rev-parse --verify HEAD)
> +                        common=$(unset GIT_DIR; cd "$path" && git merge-base HEAD $sha1)
> +                        if test -z "$common"; then
> +                            common=$(unset GIT_DIR; cd "$path" && git-fetch -q &&
> +                                    git merge-base HEAD $sha1)
> +                        fi

This "fetch" feels very wrong.  The user did not ask you to change the
state of the repository, but this will silently change the remote tracking
branches.  The repository after all might be unreachable.