Re: What should "git submodule summary" give before an initial commit?

[Johan Herland <johan@herland.net>]

On Wednesday 03 March 2010, Junio C Hamano wrote:
> "git status" collects the changes for both the index (since HEAD) and the
> working tree files (since the index), summarizes and shows them.  When it
> is run before the first commit is made, it uses a logic different from
> the one used in the normal case to gather the information on the index,
> as we don't have HEAD yet, i.e. instead of "diff-index HEAD", we would
> run "diff-index emtpy-tree".
> 
> How should status.submodulesummary integrate into this framework?
> 
> Currently, it blindly runs "git submodule summary", and it gives an extra
> error message about HEAD not being a commit, and people (me included)
> misguidedly have spent time on squelching the message.
> 
> But I think that was _all wrong_.  I do not think "git submodule summary"
> should fail even when you haven't made your first commit.
> 
> If you are before the first commit, we say everything you have in the
> index is a change you are adding with your next commit (which will be
> your initial one).  If you added a submodule commit to the index,
> shouldn't "git submodule summary" say "you'll be committing the addition
> of this subproject"?  IOW, shouldn't we be comparing an empty tree to
> find added submodules, like this, when we haven't made the first commit?
> 
>  git-submodule.sh |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 5869c00..0397f9d 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -556,10 +556,10 @@ cmd_summary() {
>  	if rev=$(git rev-parse -q --verify --default HEAD ${1+"$1"})
>  	then
>  		head=$rev
> -		shift
> +		test $# = 0 || shift
>  	elif test -z "$1" -o "$1" = "HEAD"
>  	then
> -		return
> +		head=4b825dc642cb6eb9a060e54bf8d69288fbee4904
>  	else
>  		head="HEAD"
>  	fi

Acked-by: Johan Herland <johan@herland.net>

If you're planning to revert 3deea89 (although the above patch suggests 
you're not), then please don't revert the t7401 testcases added by that 
commit. The testcase is useful in any case.


...Johan

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