On Sat, Apr 12, 2008 at 6:30 AM, Junio C Hamano <junio@xxxxxxxxx> wrote: > 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. This patch just does refactor, i do this in my sixth patch "Don't die when command fails for one submodule" -- Ping Yin -- 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
