> I haven't taken a through look at this patch but I think you may want to
> base it off of 'origin/bw/pathspec-cleanup' series as the changes made in this
> patch now conflict with that series.
eh right, I forgot to mention this in the notes, it requires
sb/submodule-embed-gitdir as well, so I'll have to figure that out.
>
> Also I still don't really think this solves the problem of telling the
> user what is wrong, which is that the submodule's gitdir is gone.
>
The "git dir gone" is not a big deal IMHO as a deinitialized submodule
is perfectly fine (e.g. not initialized). The errors as I tested in Gerrit,
a superproject that contains submodules in plugins/* :
: gerrit/plugins/cookbook-plugin$ git add .
fatal: Pathspec '.' is in submodule 'plugins/cookbook-plugin'
: gerrit/plugins/cookbook-plugin$ cd ..
: gerrit/plugins$ git add cookbook-plugin/a
fatal: Pathspec 'cookbook-plugin/a' is in submodule
'plugins/cookbook-plugin'
: gerrit/plugins$ git add cookbook-plugin/.
: gerrit/plugins$ git add cookbook-plugin/./.
: gerrit/plugins$
I think that is perfect behavior for now, as it reliably detects
(a) the submodule being there and (b) if you are in there, no
matter if there is a .git dir or not.
The same error coming up if the submodule is initialized and valid, e.g.
: gerrit/plugins$ git submodule update --init cookbook-plugin
: gerrit/plugins$ git add cookbook-plugin/a/.
fatal: Pathspec 'cookbook-plugin/a/.' is in submodule
'plugins/cookbook-plugin'
So I think this is pretty much exactly what we want for now:
* if PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE is set
we keep the behavior as is and do the expensive thing
* if the caller wants to use path inside of a submodule no matter
the git dir of the submodule, then set the CHEAP flag instead
* in case of the assert (that I originally wanted to fix), we fall back to the
EXPENSIVE thing reporting the error message that we already reported
in such cases.
TL;DR: I was rather asking about the code being a viable;
by now I am convinced this is the correct behavior. ;)