Re: [PATCH/RFC 1/7] git-submodule: Extract functions module_info and module_url

From: Junio C Hamano <junio@pobox.com>
Date: Apr 11, 2008, 22:30 UTC
Message-ID: <7v4pa8qb2h.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1207842625-9210-2-git-send-email-pkufranky@gmail.com>

Ping Yin <pkufranky@gmail.com> writes:

Show 14 quoted lines

> @@ -232,12 +251,11 @@ cmd_init()
>  		shift
>  	done
>  
> -	git ls-files --stage -- "$@" | grep '^160000 ' |
> -	while read mode sha1 stage path
> +	module_info "$@" |
> +	while read sha1 path name url
>  	do
> +		test -n "$name" || exit
>  		# Skip already registered paths
> -		name=$(module_name "$path") || exit
> -		url=$(git config submodule."$name".url)
>  		test -z "$url" || continue

This is not a new bug in this round of patch (i.e. the original code already did the same), but I have to wonder if exiting the loop silently when $name is not set (i.e. .gitmodules does not have an entry to describe the submodule yet) is a good idea.

If an entry with mode 160000 in the index is an error if it does not have a corresponding entry in .gitmodules, then this code should say so when exiting the loop prematurely. If it is not, I think it should silently continue just like missing URL case.

The user may be right in the process of manually adding a new submodule, has done "git add" of the submodule path already but hasn't yet decided what the name of the submodule or where the final published URL would be. In such a case, you would have 160000 entry in the index that does not have a corresponding entry in .gitmodules and that is perfectly valid.

So I tend to think that you should treat "missing name" and "missing url" as an non-error case.

cmd_init would obviously need to _notice_ "missing url" and refrain from adding the missing remote URL to the config, but I do not think it should error out. Warning might be appropriate in cases, but I dunno.

Same comment applies to cmd_update() and cmd_status(). I would strongly suspect that status may want to ignore missing name/url and show the usual diff, as it does not even have to require the submodule to interact with any remote repository at all. The user may be privately using the submodule and does not even need to push it out nor pull it from elsewhere, and in such a case, .gitmodules may not even be populated with an entry for that submodule, ever, not just as a "right in the middle of adding" status.

When the command should be usable without .gitmodules entry, it should not require one.