mailing list mirror (one of many)
 help / Atom feed
From: Stefan Beller <>
To: Kaartic Sivaraam <>
Cc: Git Mailing list <>,
	Jens Lehmann <>
Subject: Re: Weak option parsing in `git submodule`
Date: Mon, 7 May 2018 12:05:41 -0700
Message-ID: <> (raw)
In-Reply-To: <>

On Sun, May 6, 2018 at 11:10 AM, Kaartic Sivaraam
<> wrote:
> 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.

Yuck, that is bad UX.

> 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.

(from the man page)
git submodule [--quiet] add [<options>] [--] <repository> [<path>]

When options are given after <repository> or <path> we can count
the arguments and know something is up. (The number of arguments
must be 1 or 2. If it is 3 or above, something fishy is going on), which
I would suggest as a first step.

>     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`.

The second part of this suggestion seems to me as the way to go.
Do you want to write a patch?

>     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.

Yeah, bringing it to C, would be nice as a long term solution instead
of piling up more and more shell features. :)

Thanks for such a well written bug report with suggested bug fixes. :)

  reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-06 18:10 Kaartic Sivaraam
2018-05-07 19:05 ` Stefan Beller [this message]
2018-05-08  7:56   ` Kaartic Sivaraam

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:

 note: .onion URLs require Tor:
       or Tor2web:

AGPL code for this site: git clone public-inbox