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

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

Ping Yin <pkufranky@gmail.com> writes:

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