Re: [PATCH] t/Makefile: make sure that file names are truly platform-independent

From: Jeff King <peff@peff.net>
Date: Aug 15, 2016, 18:43 UTC
Message-ID: <20160815184315.cyhln2bxg465lq2l@sigill.intra.peff.net>
In-Reply-To: <xmqq4m6mhsvj.fsf@gitster.mtv.corp.google.com>

On Mon, Aug 15, 2016 at 09:57:52AM -0700, Junio C Hamano wrote:

Show 22 quoted lines

> I wonder if we already have a good mechanism to allow a project and
> its participants (say, "me") to declare "in this project, pathnames
> must conform to this rule" and help them avoid creating a tree that
> violates the rule customized to their project.
> 
> I guess "write_index_as_tree()" would be one of the central places
> to hook into and that covers an individual contributor or a patch
> applier who ends up adding offending paths to the project, as well
> as a merge made in response to a pull request (unless it is a
> fast-forward) [*1*].  The pre-receive hook can also be used to
> inspect and reject an attempt to push an offending tree into the
> history.
> 
> Such a mechanism would allow a project that wants participation by
> folks with case insensitive filesystems to ensure that they do not
> create a directory that has both xt_TCPMSS.h and xt_tcpmss.h at the
> same time, for example, but the mechanism needs to allow visibility
> into more than just a single path when the custom check is made
> (e.g. a hook run in "write_index_as_tree()" can see all entries in
> the index to make the decision; if we were to also hook into
> "add_to_index()", the hook must be able to see other entries in the
> index to which the new entry is being added).

I am not convinced this mechanism needs to be built into git. Because it happens to be about filenames, git at least has a hope of making sense of the various project rules.

But conceptually, I don't think this is really any different than "don't check in code which does not build on platform X", or "do not check in code that does not meet our style guide". We already have general hooks where such checks can be made[1], and this could be checked in the same place.

I actually think the whitespace-checking done by diff and apply is in a similar boat, though it is useful in practice. OTOH, I think primarily it is used as a tool to feed information to policy hooks, rather than as an enforcement mechanism itself (maybe --whitespace=fix on apply is an exception there, though).

-Peff

[1] Obviously we have pre-commit for local enforcement and pre-receive
    for central enforcement. But people with workflows around CI-style
    tests would want to be able to fetch, check "does this meet our
    policy", and return a yes/no answer on whether the result is OK to
    merge.