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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ping Yin <pkufranky@xxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux