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

["Ping Yin" <pkufranky@gmail.com>]

On Sat, Apr 12, 2008 at 6:30 AM, Junio C Hamano <junio@pobox.com> wrote:
> 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.

This patch just does refactor, i do this in my sixth patch "Don't die
when command fails for one submodule"


-- 
Ping Yin