All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee <derrickstolee@github.com>,
	Adam Spiers <git@adamspiers.org>, Jeff King <peff@peff.net>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 03/13] init: unconditionally create the "info" directory
Date: Mon, 20 Dec 2021 17:13:19 +0100	[thread overview]
Message-ID: <211220.86tuf3utv9.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <db6f47a3-0df3-505b-b391-6ca289fd61b5@gmail.com>


On Mon, Dec 20 2021, Derrick Stolee wrote:

> 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.

>> 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.

  reply	other threads:[~2021-12-20 16:24 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-12 20:13 [PATCH 00/13] tests + init: don't rely on templates & add --no-template + config Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 01/13] t0001: fix gaps in "TEMPLATE DIRECTORY" coverage Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 02/13] init: split out template population from create_default_files() Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 03/13] init: unconditionally create the "info" directory Ævar Arnfjörð Bjarmason
2021-12-20 15:59   ` Derrick Stolee
2021-12-20 16:13     ` Ævar Arnfjörð Bjarmason [this message]
2021-12-20 17:39       ` Derrick Stolee
2021-12-20 18:16         ` Ævar Arnfjörð Bjarmason
2021-12-20 19:06         ` Junio C Hamano
2021-12-21  1:15           ` Ævar Arnfjörð Bjarmason
2021-12-21  2:10             ` Junio C Hamano
2021-12-21  2:39               ` Ævar Arnfjörð Bjarmason
2021-12-21  6:38                 ` Junio C Hamano
2021-12-24 17:26                   ` Ævar Arnfjörð Bjarmason
2021-12-25  1:58                     ` Junio C Hamano
2022-01-12 12:42         ` Ævar Arnfjörð Bjarmason
2022-01-18 19:43           ` Derrick Stolee
2022-01-19  1:00             ` Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 04/13] t0008: don't rely on default ".git/info/exclude" Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 05/13] init & clone: add a --no-template option Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 06/13] init & clone: add init.templateDir=[bool] Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 07/13] test-lib: create test data with "git init --no-template" (almost) Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 08/13] tests: don't depend on template-created .git/branches Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 09/13] t5540: don't rely on "hook/post-update.sample" Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 10/13] test-lib-functions: add and use a "write_hook" wrapper Ævar Arnfjörð Bjarmason
2021-12-13 14:15   ` Eric Sunshine
2021-12-13 16:29     ` Ævar Arnfjörð Bjarmason
2021-12-13 16:45       ` Eric Sunshine
2021-12-13 19:37         ` Ævar Arnfjörð Bjarmason
2021-12-13 21:33           ` Eric Sunshine
2021-12-12 20:13 ` [PATCH 11/13] tests: change "cat && chmod +x" to use "write_hook" Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 12/13] tests: migrate miscellaneous "write_script" to "write_hooks" Ævar Arnfjörð Bjarmason
2021-12-12 20:13 ` [PATCH 13/13] tests: don't depend on template-created .git/hooks Ævar Arnfjörð Bjarmason
2022-06-03 11:15 ` [PATCH v2 0/7] tests: don't depend on "git init" using the template Ævar Arnfjörð Bjarmason
2022-06-03 11:15   ` [PATCH v2 1/7] t0008: don't rely on default ".git/info/exclude" Ævar Arnfjörð Bjarmason
2022-06-03 11:15   ` [PATCH v2 2/7] tests: don't depend on template-created .git/branches Ævar Arnfjörð Bjarmason
2022-06-03 11:15   ` [PATCH v2 3/7] tests: don't assume a .git/info for .git/info/grafts Ævar Arnfjörð Bjarmason
2022-06-03 11:15   ` [PATCH v2 4/7] tests: don't assume a .git/info for .git/info/attributes Ævar Arnfjörð Bjarmason
2022-06-03 11:15   ` [PATCH v2 5/7] tests: don't assume a .git/info for .git/info/refs Ævar Arnfjörð Bjarmason
2022-06-03 11:15   ` [PATCH v2 6/7] tests: don't assume a .git/info for .git/info/exclude Ævar Arnfjörð Bjarmason
2022-06-03 11:15   ` [PATCH v2 7/7] tests: don't assume a .git/info for .git/info/sparse-checkout Ævar Arnfjörð Bjarmason
2022-06-03 19:17   ` [PATCH v2 0/7] tests: don't depend on "git init" using the template Junio C Hamano
2022-06-04  0:41     ` Ævar Arnfjörð Bjarmason
2022-06-06 19:08       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=211220.86tuf3utv9.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=derrickstolee@github.com \
    --cc=git@adamspiers.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.