Weak option parsing in `git submodule`
- Date: Sun, 06 May 2018 23:40:43 +0530
- From: Kaartic Sivaraam <kaartic.sivaraam@xxxxxxxxx>
- Subject: 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:
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
eval_gettextln "The following path is ignored by one of your .gitignore files:
Use -f if you really want to add it." >&2
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
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.
“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
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!