Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.

From: Jeff King <peff@peff.net>
Date: Jan 20, 2017, 19:42 UTC
Message-ID: <20170120194224.vikzovupwqx53x2c@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kaJQefSDkV-LKxRCTtSepsNsX7U+AZqy3Z_YCd1xsmTxQ@mail.gmail.com>

On Fri, Jan 20, 2017 at 11:33:45AM -0800, Stefan Beller wrote:

Show 9 quoted lines

> > I'd rather see it in the commands themselves. Especially given the
> > "ideal" in your status example, which requires command-specific
> > knowledge.
> 
> So you rather want to go bottom up, i.e. add it to each command individually
> for which it makes sense, instead of rather first having a catch-it-all like
> this and then we can have a flag similar to RUN_SETUP, e.g.
> ALLOW_IN_UNPOP_SUBMODULE, which allows commands to
> take over the responsibility to act responsibly in this case?

Yes. I know it's "less safe" in the sense that commands have to make an effort to detect the situation, but I feel like only they'll know what the sensible behavior is. And they can also do the check at a time when they would be reading the index anyway.

> status may be the first command for going that route; I wonder if we'd
> want to add this feature unconditionally or only in the porcelain case.
> (In plumbing you're supposed to know what you're doing... so there is
> no need as well as our promise to not change it)

Yeah. The reason that it would be so painful to load the index for every rev-parse is not just that it probably doesn't otherwise need the index, but that scripts may make a _ton_ of rev-parse (or other plumbing) calls.

One alternative would be to make the check cheaper. Could we reliably tell from the submodule.foo.* block in the config that path "foo" is a submodule? I think that would work after "submodule init" but not right after "git clone". So the index really is the source of truth there.

I guess there could be an index extension "these are the gitlinks I contain" and in theory we could read just that extension. I dunno.

-Peff