Web lists-archives.com

Weak option parsing in `git submodule`




I recently faced the consequence of the weak option parsing done by in
`git submodule`. Initially tried to add a new submodule using the `git
submodule add` sub-command in the following way,

    $ git submodule add ./foo/bar

This cloned the submodule into a new directory named 'bar' in the
present directory. This was initially confusing as I expected `git
submodule` to use the actual directory itself as it resides inside a
sub-directory the main project and thus avoiding the creation of a new
clone. Then I came to know that `git submodule` wasn't smart enough yet
to identify this, by reading the documentation. Then, after realizing
that I would have to specify the path in the command to avoid the
redundant clone, I corrected the command without consulting the
documentation (thoroughly) to,

    $ git submodule add ./foo/bar --path ./foo/bar

expecting that the path needs to be specified after an argument. This
is what triggered the weird consequence of weak option parsing. The
output I got for the above command was:

    The following path is ignored by one of your .gitignore files:
    --path
    Use -f if you really want to add it.

That really confused me pretty much (being a person who doesn't know
much about how `git submodule` works). Instead of complaining about an
inexistent option '--path', it considers '--path' to be the <path>
argument which seems to be bad. It doesn't even complain about the
extra argument. I also dug to find the reason behind this totally weird
message. It seems to be a consequence of the following lousy check
($sm_path is the normalized <path> argument):

    if test -z "$force" &&
            ! git add --dry-run --ignore-missing --no-warn-embedded-repo "$sm_path" > /dev/null 2>&1
    then
            eval_gettextln "The following path is ignored by one of your .gitignore files:
    \$sm_path
    Use -f if you really want to add it." >&2
            exit 1
    fi

    The lack of checking for the reason behind why `git add` fails seems to
    be the reason behind that weird message.

    Ways to fix this:

    1. Fix this particular issue by adding a '--' after the '--no-warn-
    embedded-repo' option in the above check. But that would also
    require that we allow other parts of the script to accept weird
    paths such as '--path'. Not so useful/helpful.

    2. Check for the actual return value of `git add` in the check and
    act accordingly. Also, check if there are unnecessary arguments for
    `submodule add`.

    3. Tighten option parsing in the `git-submodule` script to ensure
    this doesn't happen in the long term and helps users to get more
    relevant error messages.

    I find 3 to be a long term solution but not sure if it's worth trying
    as there are efforts towards porting `git submodule` to C. So, I guess
    we should at least do 2 as a short-term solution.

    Thoughts??

    -- 
    Sivaraam

    QUOTE:

    “The most valuable person on any team is the person who makes everyone
    else on the team more valuable, not the person who knows the most.”

          - Joel Spolsky


    Sivaraam?

    You possibly might have noticed that my signature recently changed from
    'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
    new signature to be better for several reasons one of which is that the
    former signature has a lot of ambiguities as it is a common name (NOTE:
    it's not a common spelling, just a common name). So, I switched
    signatures before it's too late.

    That said, I won't mind you calling me 'Kaartic' if you like it [of
    course ;-)]. You can always call me using either of the names.


    KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

    As I'm not a native English speaker myself, there might be mistaeks in
    my usage of English. I apologise for any mistakes that I make.

    It would be "helpful" if you take the time to point out the mistakes.

    It would be "super helpful" if you could provide suggestions about how
    to correct those mistakes.

    Thanks in advance!