On Mon, Dec 20 2021, Derrick Stolee wrote:

On 12/12/2021 3:13 PM, Ævar Arnfjörð Bjarmason wrote:

quoted

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.
 

quoted

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.

quoted

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.

quoted

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.