Re: [PATCH 03/13] init: unconditionally create the "info" directory

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 20, 2021, 16:13 UTC
Message-ID: <211220.86tuf3utv9.gmgdl@evledraar.gmail.com>
In-Reply-To: <db6f47a3-0df3-505b-b391-6ca289fd61b5@gmail.com>

On Mon, Dec 20 2021, Derrick Stolee wrote:

Show 39 quoted lines

> On 12/12/2021 3:13 PM, Ævar Arnfjörð Bjarmason wrote:
>> In preceding commits the test suite has been taught to run without a
>> template directory, but in doing so we needed to fix code that relied
>> on the "hooks" and "branches" directories.
>> 
>> The "hooks" code was all specific to our own test suite. The
>> "branches" directory is intentionally created, but has been "slightly
>> deprecated" for a while, so it's not created when not using the
>> default template.
>> 
>> However "info" is different. Trying to omit its creation would lead to
>> a lot of test suite failures. Many of these we should arguably fix,
>> the common pattern being to add an exclude to "info/excludes".
>
> This would be painful to add because of the impact on the test suite.
> That I understand.
>  
>> But we've also grown a hard dependency on this directory within git
>> itself. Since 94c0956b609 (sparse-checkout: create builtin with 'list'
>> subcommand, 2019-11-21) released with v2.25.0 the "git
>> sparse-checkout" command has wanted to add exclusions to
>> "info/sparse-checkout". It didn't check or create the leading
>> directory, so if it's omitted the command will die.
>
>> Even if that behavior were fixed we'd be left with older versions of
>> "git" dying if that was attempted if they used a repository
>> initialized without a template.
>
> This, I don't understand. Why can't we add a
> safe_create_leading_directories() to any place where we try to
> create a sparse-checkout file?
>
> This would fix situations where older versions were init'd with a
> different template or if the user deleted the info dir. The change
> you've made here doesn't fix those cases, which is what you are
> claiming is the reason to not do the other fix that seems like it
> would.
>
> What am I misunderstanding here?

I'll clarify that a bit in any re-roll.

Pedantically nothing changes, i.e. you can create a repository with an empty template now, and it'll break on both the sparse-checkout on that version, and any previous version that had that un-noticed issue.

But in practice I think it wouldn't have been a big deal, because while you could omit or specify a custom template it was somewhat of a hassle, and even somewhat undocumented.

Whereas with this series allowing you to easily configure it with init.templateDir=false it becomes trivial. That was another motivation of mine for adding this, I'd like to not have N copies of that template crud all over my systems.

So I think in practice we need to be more conservative about cross-version interaction here. It's not just a matter of "if", but also of a new "how" making that "if" more common. I.e. needing to interact with an empty-template .git directory.

We also have non-git.git code to worry about, e.g. us breaking any user-custom script that might do:

    #!/bin/sh -e
    git init "$1"
    cd "$1"
    # *Boom* under init.templateDir=false, unless we keep ".git/info"
    echo <my ignores> >.git/info/excludes

So I just don't think it's worth it. Let's just create .git/info unconditionally like we create .git/objects/{info,pack} now.

It's unrelated to this, but if this gets in I would eventually like to submit a change to make some version of init.templateDir=false the default. That's a bit more untandling, but I think ultimately beneficial. E.g. the sample hooks are documented in "git help githooks" along with general hook documentation. Such a change would ideally involve splitting that out (maybe to a a "gitrepository-sample-hooks(5)"). That's another reason for why I'd like to make init.templateDir=false play nicely with existing in-tree and out-of-tree expectations.

Show 6 quoted lines

>> So let's just bite the bullet and make the "info" directory mandatory,
>> and document it as such. Let's also note that in the documentation
>> that this doesn't apply to the "hooks" and "branches" directories.
>
> I have no objection to this approach, but we should still do the
> other thing.

All that being said I don't really mind not creating a .git/info if the consensus sways that way.

It's a bit more painful when it comes to the tests, but not *that* painful. I had it mostly working before abandoning it for this approach.