[PATCH 00/13] tests + init: don't rely on templates & add --no-template + config

[PATCH 00/13] tests + init: don't rely on templates & add --no-template + config

[Ævar Arnfjörð Bjarmason]

[This series does a couple of different things that could be split up,
but I thought that it was easier to review it in one piece].

This series changes our tests to not rely on the default template that
"git init" populates new repositories with. This reliance hid a bug
where a recently added mode of "sparse-checkout" had a hard dependency
on git's default template, i.e. it would potentially break on
repositories with a custom --template.

So this changes all the tests that relied on those, and declares that
we'll always create the "info" directory whatever the template says
(which will fix that sparse-checkout isssue).

We also add a new "git [init|clone] --no-template" option, and make
the existing init.templateDir accept "false" as a way of doing what
the existing-but-undocumented --template= would do, i.e. use no
template at all.

I would like to eventually follow-up and make something like
init.templateDir=false the default (or at least something closer to
it). Notably we litter the rather large sample hooks in every
repository ever cloned, but this series doesn't chane anything about
what we do by default.

This topic was mentioned (item 16) in the recent contributor
summit[1]. The notes only reflect that I mentioned .git/branches, but
I think it was in reply to someone's question about that or the
default templates.

1. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2110211148400.56@tvgsbejvaqbjf.bet/

Ævar Arnfjörð Bjarmason (13):
  t0001: fix gaps in "TEMPLATE DIRECTORY" coverage
  init: split out template population from create_default_files()
  init: unconditionally create the "info" directory
  t0008: don't rely on default ".git/info/exclude"
  init & clone: add a --no-template option
  init & clone: add init.templateDir=[bool]
  test-lib: create test data with "git init --no-template" (almost)
  tests: don't depend on template-created .git/branches
  t5540: don't rely on "hook/post-update.sample"
  test-lib-functions: add and use a "write_hook" wrapper
  tests: change "cat && chmod +x" to use "write_hook"
  tests: migrate miscellaneous "write_script" to "write_hooks"
  tests: don't depend on template-created .git/hooks

 Documentation/git-clone.txt            |   8 +-
 Documentation/git-init.txt             |  31 +++++++-
 Documentation/gitrepository-layout.txt |  17 ++++-
 builtin/clone.c                        |  15 +++-
 builtin/init-db.c                      |  75 +++++++++++++------
 cache.h                                |   2 +
 config.c                               |  34 +++++++++
 config.h                               |  17 +++++
 t/t0001-init.sh                        | 100 ++++++++++++++++++++++---
 t/t0008-ignores.sh                     |  10 +--
 t/t1416-ref-transaction-hooks.sh       |  14 ++--
 t/t3412-rebase-root.sh                 |  18 ++---
 t/t3413-rebase-hook.sh                 |  18 ++---
 t/t3430-rebase-merges.sh               |   5 +-
 t/t5401-update-hooks.sh                |  62 +++++++--------
 t/t5402-post-merge-hook.sh             |  16 ++--
 t/t5406-remote-rejects.sh              |   2 +-
 t/t5407-post-rewrite-hook.sh           |  14 ++--
 t/t5409-colorize-remote-messages.sh    |   2 +-
 t/t5411-proc-receive-hook.sh           |   4 +-
 t/t5505-remote.sh                      |   2 +
 t/t5510-fetch.sh                       |   6 +-
 t/t5516-fetch-push.sh                  |  26 +++----
 t/t5521-pull-options.sh                |   4 +-
 t/t5540-http-push-webdav.sh            |   4 +-
 t/t5541-http-push-smart.sh             |   4 +-
 t/t5547-push-quarantine.sh             |   4 +-
 t/t5548-push-porcelain.sh              |   2 +-
 t/t5601-clone.sh                       |   4 +-
 t/t6500-gc.sh                          |   4 +-
 t/t7450-bad-git-dotfiles.sh            |   1 +
 t/test-lib-functions.sh                |  30 ++++++++
 t/test-lib.sh                          |   5 +-
 wrap-for-bin.sh                        |   4 +-
 34 files changed, 391 insertions(+), 173 deletions(-)

-- 
2.34.1.1020.gb1392dd1877

[PATCH 01/13] t0001: fix gaps in "TEMPLATE DIRECTORY" coverage

[Ævar Arnfjörð Bjarmason]

Add tests to assert that the priority order described in the "TEMPLATE
DIRECTORY" section in the "init" documentation is correct.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0001-init.sh | 85 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 3 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 7603ad2f82b..e0b965cdc8f 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -173,6 +173,55 @@ test_expect_success 'reinit' '
 	test_must_be_empty again/err2
 '
 
+setup_template_priority() {
+	test_when_finished "rm -rf template" &&
+	mkdir template &&
+	touch template/file &&
+
+	test_when_finished "rm -rf template2" &&
+	mkdir template2 &&
+	touch template2/file2 &&
+
+	# Created by the caller
+	test_when_finished "rm -rf repo"
+}
+
+test_expect_success 'usage priority: --template only' '
+	setup_template_priority &&
+	git init --template=template repo &&
+	test_path_is_file repo/.git/file
+'
+
+test_expect_success 'usage priority: --template takes precedence over GIT_TEMPLATE_DIR' '
+	setup_template_priority &&
+	GIT_TEMPLATE_DIR="$PWD/template2" git init --template=template repo &&
+	test_path_is_file repo/.git/file
+'
+
+test_expect_success 'usage priority: --template takes precedence over init.templateDir' '
+	setup_template_priority &&
+	git -c init.templateDir="$PWD/template2" init --template=template repo &&
+	test_path_is_file repo/.git/file
+'
+
+test_expect_success 'usage priority: --no-template takes precedence over init.templateDir' '
+	setup_template_priority &&
+	git -c init.templateDir="$PWD/template" init --no-template repo &&
+	test_path_is_missing repo/.git/file
+'
+
+test_expect_success 'usage priority: --no-template takes precedence over GIT_TEMPLATE_DIR' '
+	setup_template_priority &&
+	GIT_TEMPLATE_DIR="$PWD/template" git init --no-template repo &&
+	test_path_is_missing repo/.git/file
+'
+
+test_expect_success 'usage priority: GIT_NO_TEMPLATE_DIR=true takes precedence over GIT_TEMPLATE_DIR' '
+	setup_template_priority &&
+	GIT_TEMPLATE_DIR="$PWD/template" GIT_NO_TEMPLATE_DIR=true git init repo &&
+	test_path_is_missing repo/.git/file
+'
+
 test_expect_success 'init with --template' '
 	mkdir template-source &&
 	echo content >template-source/file &&
@@ -187,11 +236,15 @@ test_expect_success 'init with --template (blank)' '
 	test_path_is_missing template-blank/.git/info/exclude
 '
 
+no_templatedir_env () {
+	sane_unset GIT_TEMPLATE_DIR &&
+	NO_SET_GIT_TEMPLATE_DIR=t &&
+	export NO_SET_GIT_TEMPLATE_DIR
+}
+
 init_no_templatedir_env () {
 	(
-		sane_unset GIT_TEMPLATE_DIR &&
-		NO_SET_GIT_TEMPLATE_DIR=t &&
-		export NO_SET_GIT_TEMPLATE_DIR &&
+		no_templatedir_env &&
 		git init "$1"
 	)
 }
@@ -214,6 +267,32 @@ test_expect_success 'init with init.templatedir using ~ expansion' '
 	test_cmp templatedir-source/file templatedir-expansion/.git/file
 '
 
+test_expect_success 'init with init.templateDir=does-not-exist' '
+	test_when_finished "rm -rf repo" &&
+	(
+		no_templatedir_env &&
+
+		cat >expect <<-\EOF &&
+		warning: templates not found in does-not-exist
+		EOF
+		git -c init.templateDir=does-not-exist init repo 2>actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'init with init.templateDir=[bool]' '
+	test_when_finished "rm -rf repo" &&
+	(
+		no_templatedir_env &&
+
+		cat >expect <<-\EOF &&
+		warning: templates not found in false
+		EOF
+		git -c init.templateDir=false init repo 2>actual &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'init --bare/--shared overrides system/global config' '
 	test_config_global core.bare false &&
 	test_config_global core.sharedRepository 0640 &&
-- 
2.34.1.1020.gb1392dd1877

[PATCH 02/13] init: split out template population from create_default_files()

[Ævar Arnfjörð Bjarmason]

The create_default_files() function only has one caller,
init_db(). Let's have it call a separate create_template_files()
function.

This refactoring changes nothing about how the repository is
initialized, but makes subsequent changes to create_template_files()
easier to read, e.g. because new variables we'll need will be scoped
to that function.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/init-db.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 2167796ff2a..3cf834eddd2 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -187,21 +187,9 @@ void initialize_repository_version(int hash_algo, int reinit)
 		git_config_set_gently("extensions.objectformat", NULL);
 }
 
-static int create_default_files(const char *template_path,
-				const char *original_git_dir,
-				const char *initial_branch,
-				const struct repository_format *fmt,
-				int quiet)
+static void create_template_files(const char *template_path)
 {
-	struct stat st1;
-	struct strbuf buf = STRBUF_INIT;
-	char *path;
-	char junk[2];
-	int reinit;
-	int filemode;
-	struct strbuf err = STRBUF_INIT;
 	const char *init_template_dir = NULL;
-	const char *work_tree = get_git_work_tree();
 
 	/*
 	 * First copy the templates -- we might have the default
@@ -218,6 +206,21 @@ static int create_default_files(const char *template_path,
 	git_config_clear();
 	reset_shared_repository();
 	git_config(git_default_config, NULL);
+}
+
+static int create_default_files(const char *original_git_dir,
+				const char *initial_branch,
+				const struct repository_format *fmt,
+				int quiet)
+{
+	struct stat st1;
+	struct strbuf buf = STRBUF_INIT;
+	char *path;
+	char junk[2];
+	int reinit;
+	int filemode;
+	struct strbuf err = STRBUF_INIT;
+	const char *work_tree = get_git_work_tree();
 
 	/*
 	 * We must make sure command-line options continue to override any
@@ -425,7 +428,8 @@ int init_db(const char *git_dir, const char *real_git_dir,
 
 	validate_hash_algorithm(&repo_fmt, hash);
 
-	reinit = create_default_files(template_dir, original_git_dir,
+	create_template_files(template_dir);
+	reinit = create_default_files(original_git_dir,
 				      initial_branch, &repo_fmt,
 				      flags & INIT_DB_QUIET);
 	if (reinit && initial_branch)
-- 
2.34.1.1020.gb1392dd1877

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

[Ævar Arnfjörð Bjarmason]

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

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.

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/gitrepository-layout.txt | 17 ++++++++++++++++-
 builtin/init-db.c                      |  6 ++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index 1a2ef4c1505..eb58ab08817 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -160,7 +160,10 @@ branches::
 	and not likely to be found in modern repositories. This
 	directory is ignored if $GIT_COMMON_DIR is set and
 	"$GIT_COMMON_DIR/branches" will be used instead.
-
++
+This directory is created by the default linkgit:git-init[1]
+template. It will not be created when using a custom template that
+doesn't contain it.
 
 hooks::
 	Hooks are customization scripts used by various Git
@@ -171,6 +174,10 @@ hooks::
 	Read linkgit:githooks[5] for more details about
 	each hook. This directory is ignored if $GIT_COMMON_DIR is set
 	and "$GIT_COMMON_DIR/hooks" will be used instead.
++
+This directory is created by the default linkgit:git-init[1]
+template. It will not be created when using a custom template that
+doesn't contain it.
 
 common::
 	When multiple working trees are used, most of files in
@@ -190,6 +197,14 @@ info::
 	Additional information about the repository is recorded
 	in this directory. This directory is ignored if $GIT_COMMON_DIR
 	is set and "$GIT_COMMON_DIR/info" will be used instead.
++
+This directory is created by the default linkgit:git-init[1]
+template.
++
+It will be created even when when using a custom template that doesn't
+contain it. On older versions of git this was not the case, as various
+tools came to rely on its creation (including parts of git itself)
+it's now unconditionally (re-)created on 'git init'.
 
 info/refs::
 	This file helps dumb transports discover what refs are
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 3cf834eddd2..75495c9c8c6 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -222,6 +222,12 @@ static int create_default_files(const char *original_git_dir,
 	struct strbuf err = STRBUF_INIT;
 	const char *work_tree = get_git_work_tree();
 
+	/*
+	 * We may not have a info/ if the template explicitly omitted
+	 * it.
+	 */
+	safe_create_dir(git_path("info"), 1);
+
 	/*
 	 * We must make sure command-line options continue to override any
 	 * values we might have just re-read from the config.
-- 
2.34.1.1020.gb1392dd1877

[PATCH 04/13] t0008: don't rely on default ".git/info/exclude"

[Ævar Arnfjörð Bjarmason]

Change a test added in 368aa52952d (add git-check-ignore sub-command,
2013-01-06) to clobber .git/info/exclude rather than append to
it.

These tests would break if the "templates/info--exclude" file added in
d3af621b147 (Redo the templates generation and installation.,
2005-08-06) wasn't exactly 6 lines (of only comments).

Let's instead clobber the default .git/info/excludes file, and test
only our own expected content. This is not strictly needed for
anything in this series, but is a good cleanup while we're at it.

As discussed in the preceding commit a lot of things depend on the
"info" directory being created, but this was the only test that relied
on the specific content in the "templates/info--exclude" file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0008-ignores.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 42d23148049..6bc08ee2537 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -225,7 +225,7 @@ test_expect_success 'setup' '
 		!globaltwo
 		globalthree
 	EOF
-	cat <<-\EOF >>.git/info/exclude
+	cat <<-\EOF >.git/info/exclude
 		per-repo
 	EOF
 '
@@ -543,9 +543,9 @@ test_expect_success_multi 'submodule from subdirectory' '' '
 
 test_expect_success 'global ignore not yet enabled' '
 	expect_from_stdin <<-\EOF &&
-		.git/info/exclude:7:per-repo	per-repo
+		.git/info/exclude:1:per-repo	per-repo
 		a/.gitignore:2:*three	a/globalthree
-		.git/info/exclude:7:per-repo	a/per-repo
+		.git/info/exclude:1:per-repo	a/per-repo
 	EOF
 	test_check_ignore "-v globalone per-repo a/globalthree a/per-repo not-ignored a/globaltwo"
 '
@@ -566,10 +566,10 @@ test_expect_success 'global ignore with -v' '
 	enable_global_excludes &&
 	expect_from_stdin <<-EOF &&
 		$global_excludes:1:globalone	globalone
-		.git/info/exclude:7:per-repo	per-repo
+		.git/info/exclude:1:per-repo	per-repo
 		$global_excludes:3:globalthree	globalthree
 		a/.gitignore:2:*three	a/globalthree
-		.git/info/exclude:7:per-repo	a/per-repo
+		.git/info/exclude:1:per-repo	a/per-repo
 		$global_excludes:2:!globaltwo	globaltwo
 	EOF
 	test_check_ignore "-v globalone per-repo globalthree a/globalthree a/per-repo not-ignored globaltwo"
-- 
2.34.1.1020.gb1392dd1877

[PATCH 05/13] init & clone: add a --no-template option

[Ævar Arnfjörð Bjarmason]

Add a new "--no-template" convenience option to "git init" and "git
clone".

This option is functionally equivalent to the long-standing "trick" of
providing an empty parameter to --template, i.e. "--template=". See
172035f044e (init: handle empty "template" parameter, 2008-07-28).

But that long-standing trick has never been documented, and isn't
obvious. Instead of documenting it let's provide an alternate way of
doing this that conforms with how we usually handle other such cases.

Let's also add a GIT_NO_TEMPLATE_DIR environment variable. For now
this is for consistency with the existing "GIT_TEMPLATE_DIR", but in a
subsequent commit we'll make use of it within "t/test-lib.sh".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-clone.txt |  8 +++++++-
 Documentation/git-init.txt  | 24 +++++++++++++++++++++---
 builtin/clone.c             | 15 ++++++++++++---
 builtin/init-db.c           | 34 ++++++++++++++++++++++++----------
 cache.h                     |  2 ++
 t/t0001-init.sh             |  4 ++++
 6 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 9685ea06915..0227ee9a4c0 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -9,7 +9,9 @@ git-clone - Clone a repository into a new directory
 SYNOPSIS
 --------
 [verse]
-'git clone' [--template=<template-directory>]
+
+
+'git clone' [--no-template | --template=<template-directory>]
 	  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
 	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
 	  [--dissociate] [--separate-git-dir <git-dir>]
@@ -211,6 +213,10 @@ objects from the source repository into a pack in the cloned repository.
 	via ssh, this specifies a non-default path for the command
 	run on the other end.
 
+--no-template:
+	Specify that no template directory will be used;
+	(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
+
 --template=<template-directory>::
 	Specify the directory from which templates will be used;
 	(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index ad921fe782e..d0ce9b185a5 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -9,7 +9,8 @@ git-init - Create an empty Git repository or reinitialize an existing one
 SYNOPSIS
 --------
 [verse]
-'git init' [-q | --quiet] [--bare] [--template=<template-directory>]
+'git init' [-q | --quiet] [--bare]
+	  [--no-template | --template=<template-directory>]
 	  [--separate-git-dir <git-dir>] [--object-format=<format>]
 	  [-b <branch-name> | --initial-branch=<branch-name>]
 	  [--shared[=<permissions>]] [<directory>]
@@ -62,6 +63,12 @@ include::object-format-disclaimer.txt[]
 Specify the directory from which templates will be used.  (See the "TEMPLATE
 DIRECTORY" section below.)
 
+--no-template::
+
+Do not use any template directory. On older versions of git this can
+be emulated by providing a `--template' argument where
+'<template_directory>' is a path to an empty directory.
+
 --separate-git-dir=<git-dir>::
 
 Instead of initializing the repository as a directory to either `$GIT_DIR` or
@@ -132,10 +139,21 @@ does not exist, it will be created.
 TEMPLATE DIRECTORY
 ------------------
 
-Files and directories in the template directory whose name do not start with a
+If a template directory is in use files and directories in the
+template directory whose name do not start with a
 dot will be copied to the `$GIT_DIR` after it is created.
 
-The template directory will be one of the following (in order):
+Using a template template directory can be disabled by any of of:
+
+ - The `--no-template` option being given. This option is incompatible
+   with `--template`.
+
+ - The `GIT_NO_TEMPLATE_DIR` variable being set to `true` in the
+   environment (or any other `true` value. See the discussion of
+   boolean canonicalization in linkgit:git-config[1]).
+
+If none of those conditions are true, then the template directory
+will be one of the following (in order):
 
  - the argument given with the `--template` option;
 
diff --git a/builtin/clone.c b/builtin/clone.c
index fb377b27657..a57ba4da31d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -53,6 +53,7 @@ static int option_shallow_submodules;
 static int option_reject_shallow = -1;    /* unspecified */
 static int config_reject_shallow = -1;    /* unspecified */
 static int deepen;
+static int option_no_template;
 static char *option_template, *option_depth, *option_since;
 static char *option_origin = NULL;
 static char *remote_name = NULL;
@@ -113,8 +114,11 @@ static struct option builtin_clone_options[] = {
 	OPT_ALIAS(0, "recursive", "recurse-submodules"),
 	OPT_INTEGER('j', "jobs", &max_jobs,
 		    N_("number of submodules cloned in parallel")),
-	OPT_STRING(0, "template", &option_template, N_("template-directory"),
-		   N_("directory from which templates will be used")),
+	OPT_BOOL_F(0, "no-template", &option_no_template, N_("do not use a template"),
+		   PARSE_OPT_NONEG | PARSE_OPT_HIDDEN),
+	OPT_STRING_F(0, "template", &option_template, N_("template-directory"),
+		     N_("directory from which templates will be used"),
+		     PARSE_OPT_NONEG),
 	OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
 			N_("reference repository")),
 	OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
@@ -907,6 +911,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		option_no_checkout = 1;
 	}
 
+	if (option_no_template && option_template)
+		die(_("--no-template and --template are incompatible."));
+
 	repo_name = argv[0];
 
 	path = get_repo_path(repo_name, &is_bundle);
@@ -1037,7 +1044,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
+	init_db(git_dir, real_git_dir,
+		option_no_template, option_template,
+		GIT_HASH_UNKNOWN, NULL,
 		INIT_DB_QUIET);
 
 	if (real_git_dir) {
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 75495c9c8c6..4c4ff6fe412 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -93,7 +93,8 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 	}
 }
 
-static void copy_templates(const char *template_dir, const char *init_template_dir)
+static void copy_templates(int no_template, const char *template_dir,
+			   const char *init_template_dir)
 {
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf template_path = STRBUF_INIT;
@@ -103,6 +104,11 @@ static void copy_templates(const char *template_dir, const char *init_template_d
 	DIR *dir;
 	char *to_free = NULL;
 
+	if (no_template)
+		return;
+	if (!template_dir && !init_template_dir &&
+	    git_env_bool(GIT_NO_TEMPLATE_DIR_ENVIRONMENT, 0))
+		return;
 	if (!template_dir)
 		template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
 	if (!template_dir)
@@ -187,7 +193,7 @@ void initialize_repository_version(int hash_algo, int reinit)
 		git_config_set_gently("extensions.objectformat", NULL);
 }
 
-static void create_template_files(const char *template_path)
+static void create_template_files(int no_template, const char *template_path)
 {
 	const char *init_template_dir = NULL;
 
@@ -201,7 +207,7 @@ static void create_template_files(const char *template_path)
 	 * disk).
 	 */
 	git_config_get_pathname("init.templatedir", &init_template_dir);
-	copy_templates(template_path, init_template_dir);
+	copy_templates(no_template, template_path, init_template_dir);
 	free((char *)init_template_dir);
 	git_config_clear();
 	reset_shared_repository();
@@ -391,7 +397,8 @@ static void validate_hash_algorithm(struct repository_format *repo_fmt, int hash
 }
 
 int init_db(const char *git_dir, const char *real_git_dir,
-	    const char *template_dir, int hash, const char *initial_branch,
+	    int no_template, const char *template_dir,
+	    int hash, const char *initial_branch,
 	    unsigned int flags)
 {
 	int reinit;
@@ -434,7 +441,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
 
 	validate_hash_algorithm(&repo_fmt, hash);
 
-	create_template_files(template_dir);
+	create_template_files(no_template, template_dir);
 	reinit = create_default_files(original_git_dir,
 				      initial_branch, &repo_fmt,
 				      flags & INIT_DB_QUIET);
@@ -525,7 +532,7 @@ static int shared_callback(const struct option *opt, const char *arg, int unset)
 }
 
 static const char *const init_db_usage[] = {
-	N_("git init [-q | --quiet] [--bare] [--template=<template-directory>] [--shared[=<permissions>]] [<directory>]"),
+	N_("git init [-q | --quiet] [--bare] [--no-template | --template=<template-directory>] [--shared[=<permissions>]] [<directory>]"),
 	NULL
 };
 
@@ -540,14 +547,18 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	const char *git_dir;
 	const char *real_git_dir = NULL;
 	const char *work_tree;
+	int no_template = 0;
 	const char *template_dir = NULL;
 	unsigned int flags = 0;
 	const char *object_format = NULL;
 	const char *initial_branch = NULL;
 	int hash_algo = GIT_HASH_UNKNOWN;
 	const struct option init_db_options[] = {
-		OPT_STRING(0, "template", &template_dir, N_("template-directory"),
-				N_("directory from which templates will be used")),
+		OPT_BOOL_F(0, "no-template", &no_template, N_("do not use a template"),
+			   PARSE_OPT_NONEG),
+		OPT_STRING_F(0, "template", &template_dir, N_("template-directory"),
+			     N_("directory from which templates will be used"),
+			     PARSE_OPT_NONEG),
 		OPT_SET_INT(0, "bare", &is_bare_repository_cfg,
 				N_("create a bare repository"), 1),
 		{ OPTION_CALLBACK, 0, "shared", &init_shared_repository,
@@ -572,6 +583,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	if (real_git_dir && !is_absolute_path(real_git_dir))
 		real_git_dir = real_pathdup(real_git_dir, 1);
 
+	if (no_template && template_dir)
+		die(_("--no-template and --template are incompatible"));
 	if (template_dir && *template_dir && !is_absolute_path(template_dir)) {
 		template_dir = absolute_pathdup(template_dir);
 		UNLEAK(template_dir);
@@ -701,6 +714,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	UNLEAK(work_tree);
 
 	flags |= INIT_DB_EXIST_OK;
-	return init_db(git_dir, real_git_dir, template_dir, hash_algo,
-		       initial_branch, flags);
+	return init_db(git_dir, real_git_dir,
+		       no_template, template_dir,
+		       hash_algo, initial_branch, flags);
 }
diff --git a/cache.h b/cache.h
index d5cafba17d4..d2c7b5e1899 100644
--- a/cache.h
+++ b/cache.h
@@ -498,6 +498,7 @@ static inline enum object_type object_type(unsigned int mode)
 #define GRAFT_ENVIRONMENT "GIT_GRAFT_FILE"
 #define GIT_SHALLOW_FILE_ENVIRONMENT "GIT_SHALLOW_FILE"
 #define TEMPLATE_DIR_ENVIRONMENT "GIT_TEMPLATE_DIR"
+#define GIT_NO_TEMPLATE_DIR_ENVIRONMENT "GIT_NO_TEMPLATE_DIR"
 #define CONFIG_ENVIRONMENT "GIT_CONFIG"
 #define CONFIG_DATA_ENVIRONMENT "GIT_CONFIG_PARAMETERS"
 #define CONFIG_COUNT_ENVIRONMENT "GIT_CONFIG_COUNT"
@@ -656,6 +657,7 @@ int path_inside_repo(const char *prefix, const char *path);
 #define INIT_DB_EXIST_OK 0x0002
 
 int init_db(const char *git_dir, const char *real_git_dir,
+	    int no_template,
 	    const char *template_dir, int hash_algo,
 	    const char *initial_branch, unsigned int flags);
 void initialize_repository_version(int hash_algo, int reinit);
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index e0b965cdc8f..388c28062c2 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -173,6 +173,10 @@ test_expect_success 'reinit' '
 	test_must_be_empty again/err2
 '
 
+test_expect_success 'usage: init with --no-template --template' '
+	test_expect_code 128 git init --no-template --template=$PWD
+'
+
 setup_template_priority() {
 	test_when_finished "rm -rf template" &&
 	mkdir template &&
-- 
2.34.1.1020.gb1392dd1877

[PATCH 06/13] init & clone: add init.templateDir=[bool]

[Ævar Arnfjörð Bjarmason]

Add the ability to specify init.templateDir=false in the configuration
to always initialize or clone repositories as though "--no-template"
was provided. This makes it easy to entirely opt-out of getting
default template content such as the sample hooks.

Not all of what's being added here to the config API is needed for
this change, but let's be consistent with the existing boilerplate
there and add the full set of relevant functions.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-init.txt |  7 ++++++-
 builtin/clone.c            |  4 ++--
 builtin/init-db.c          | 10 +++++++---
 config.c                   | 34 ++++++++++++++++++++++++++++++++++
 config.h                   | 17 +++++++++++++++++
 t/t0001-init.sh            |  8 +++-----
 6 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index d0ce9b185a5..ef933ee9145 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -148,6 +148,9 @@ Using a template template directory can be disabled by any of of:
  - The `--no-template` option being given. This option is incompatible
    with `--template`.
 
+ - The `init.templateDirectory' option being set to 'false' (or
+   another stand-in for 'false', see linkgit:git-config[1]).
+
  - The `GIT_NO_TEMPLATE_DIR` variable being set to `true` in the
    environment (or any other `true` value. See the discussion of
    boolean canonicalization in linkgit:git-config[1]).
@@ -159,7 +162,9 @@ will be one of the following (in order):
 
  - the contents of the `$GIT_TEMPLATE_DIR` environment variable;
 
- - the `init.templateDir` configuration variable; or
+ - the `init.templateDir` configuration variable, if set to 'true' (or
+   another stand-in for 'true', see linkgit:git-config[1]) the default
+   template directory; or
 
  - the default template directory: `/usr/share/git-core/templates`.
 
diff --git a/builtin/clone.c b/builtin/clone.c
index a57ba4da31d..671d8356f1c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -53,7 +53,7 @@ static int option_shallow_submodules;
 static int option_reject_shallow = -1;    /* unspecified */
 static int config_reject_shallow = -1;    /* unspecified */
 static int deepen;
-static int option_no_template;
+static int option_no_template = -1; /* unspecified */
 static char *option_template, *option_depth, *option_since;
 static char *option_origin = NULL;
 static char *remote_name = NULL;
@@ -911,7 +911,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		option_no_checkout = 1;
 	}
 
-	if (option_no_template && option_template)
+	if (option_no_template != -1 && option_template)
 		die(_("--no-template and --template are incompatible."));
 
 	repo_name = argv[0];
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 4c4ff6fe412..fcf538193c8 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -196,6 +196,7 @@ void initialize_repository_version(int hash_algo, int reinit)
 static void create_template_files(int no_template, const char *template_path)
 {
 	const char *init_template_dir = NULL;
+	int is_bool, ret;
 
 	/*
 	 * First copy the templates -- we might have the default
@@ -206,7 +207,10 @@ static void create_template_files(int no_template, const char *template_path)
 	 * values (since we've just potentially changed what's available on
 	 * disk).
 	 */
-	git_config_get_pathname("init.templatedir", &init_template_dir);
+	ret = git_config_get_bool_or_pathname("init.templatedir", &is_bool,
+					      &init_template_dir);
+	if (no_template == -1)
+		no_template = is_bool ? !ret : 0;
 	copy_templates(no_template, template_path, init_template_dir);
 	free((char *)init_template_dir);
 	git_config_clear();
@@ -547,7 +551,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	const char *git_dir;
 	const char *real_git_dir = NULL;
 	const char *work_tree;
-	int no_template = 0;
+	int no_template = -1;
 	const char *template_dir = NULL;
 	unsigned int flags = 0;
 	const char *object_format = NULL;
@@ -583,7 +587,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	if (real_git_dir && !is_absolute_path(real_git_dir))
 		real_git_dir = real_pathdup(real_git_dir, 1);
 
-	if (no_template && template_dir)
+	if (no_template != -1 && template_dir)
 		die(_("--no-template and --template are incompatible"));
 	if (template_dir && *template_dir && !is_absolute_path(template_dir)) {
 		template_dir = absolute_pathdup(template_dir);
diff --git a/config.c b/config.c
index c5873f3a706..f64ce2a1e1e 100644
--- a/config.c
+++ b/config.c
@@ -1260,6 +1260,18 @@ int git_config_pathname(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_bool_or_pathname(const char **dest, const char *name,
+				const char *value, int *is_bool)
+{
+	int v = git_parse_maybe_bool_text(value);
+	if (0 <= v) {
+		*is_bool = 1;
+		return v;
+	}
+	*is_bool = 0;
+	return git_config_pathname(dest, name, value);
+}
+
 int git_config_expiry_date(timestamp_t *timestamp, const char *var, const char *value)
 {
 	if (!value)
@@ -2251,6 +2263,16 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha
 		return 1;
 }
 
+int git_configset_get_bool_or_pathname(struct config_set *cs, const char *key,
+				       int *is_bool, const char **dest)
+{
+	const char *value;
+	if (!git_configset_get_value(cs, key, &value))
+		return git_config_bool_or_pathname(dest, key, value, is_bool);
+	else
+		return 1;
+}
+
 /* Functions use to read configuration from a repository */
 static void repo_read_config(struct repository *repo)
 {
@@ -2384,6 +2406,13 @@ int repo_config_get_pathname(struct repository *repo,
 	return ret;
 }
 
+int repo_config_get_bool_or_pathname(struct repository *repo,
+				     const char *key, int *is_bool, const char **dest)
+{
+	git_config_check_init(repo);
+	return git_configset_get_bool_or_pathname(repo->config, key, is_bool, dest);
+}
+
 /* Functions used historically to read configuration from 'the_repository' */
 void git_config(config_fn_t fn, void *data)
 {
@@ -2445,6 +2474,11 @@ int git_config_get_pathname(const char *key, const char **dest)
 	return repo_config_get_pathname(the_repository, key, dest);
 }
 
+int git_config_get_bool_or_pathname(const char *key, int *is_bool, const char **dest)
+{
+	return repo_config_get_bool_or_pathname(the_repository, key, is_bool, dest);
+}
+
 int git_config_get_expiry(const char *key, const char **output)
 {
 	int ret = git_config_get_string(key, (char **)output);
diff --git a/config.h b/config.h
index f119de01309..d482d55d9ae 100644
--- a/config.h
+++ b/config.h
@@ -241,6 +241,15 @@ int git_config_string(const char **, const char *, const char *);
  */
 int git_config_pathname(const char **, const char *, const char *);
 
+/**
+ * Same as `git_config_pathname`, except that we'll first try to parse
+ * the value as a boolean. Check the `is_bool` to see if it was a
+ * boolen. Otherwise it'll have the same semantics as
+ * `git_config_pathname`.
+ */
+int git_config_bool_or_pathname(const char **dest, const char *name,
+				const char *value, int *is_bool);
+
 int git_config_expiry_date(timestamp_t *, const char *, const char *);
 int git_config_color(char *, const char *, const char *);
 int git_config_set_in_file_gently(const char *, const char *, const char *);
@@ -493,6 +502,8 @@ int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);
 int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest);
 int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest);
 int git_configset_get_pathname(struct config_set *cs, const char *key, const char **dest);
+int git_configset_get_bool_or_pathname(struct config_set *cs, const char *key,
+				       int *is_bool, const char **dest);
 
 /* Functions for reading a repository's config */
 struct repository;
@@ -517,6 +528,9 @@ int repo_config_get_maybe_bool(struct repository *repo,
 			       const char *key, int *dest);
 int repo_config_get_pathname(struct repository *repo,
 			     const char *key, const char **dest);
+int repo_config_get_bool_or_pathname(struct repository *repo,
+				     const char *key, int *is_bool,
+				     const char **dest);
 
 /**
  * Querying For Specific Variables
@@ -607,6 +621,9 @@ int git_config_get_maybe_bool(const char *key, int *dest);
  */
 int git_config_get_pathname(const char *key, const char **dest);
 
+int git_config_get_bool_or_pathname(const char *key, int *is_bool,
+				    const char **dest);
+
 int git_config_get_index_threads(int *dest);
 int git_config_get_split_index(void);
 int git_config_get_max_percent_split_change(void);
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 388c28062c2..39cf132e9a0 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -289,11 +289,9 @@ test_expect_success 'init with init.templateDir=[bool]' '
 	(
 		no_templatedir_env &&
 
-		cat >expect <<-\EOF &&
-		warning: templates not found in false
-		EOF
-		git -c init.templateDir=false init repo 2>actual &&
-		test_cmp expect actual
+		git -c init.templateDir=false init repo 2>err &&
+		test_must_be_empty err &&
+		test_path_is_missing repo/.git/description
 	)
 '
 
-- 
2.34.1.1020.gb1392dd1877

[PATCH 07/13] test-lib: create test data with "git init --no-template" (almost)

[Ævar Arnfjörð Bjarmason]

Change "t/test-lib.sh" to set the newly added
"GIT_NO_TEMPLATE_DIR=true" option in the environment, which makes any
"git init" invocation take the equivalent of a "--no-template" option,
unless explicitly overridden by config or CLI option.

Well, "almost". We're also adding a test-only
"GIT_TEST_BARE_TEMPLATE=true", when it's set we'll interpret
"GIT_NO_TEMPLATE_DIR=true" as meaning no template, except for empty
"hooks" and "branches" directories.

This is because we've added various implicit dependencies on having
these created by "git init". In subsequent commits we'll address some
of those. More on that below.

The code being added here in "builtin/init-db.c" doesn't require
careful review, since this lazy_mkdir_strbuf_or_die_setlen() function
and the "no_template" condition will be gone in a few commits as we
fix up a few tests to create their own "hooks" and "branches"
directories.

This change reduces the I/O the test suite generates by quite a
bit. Before this running it with --debug (so that trash is retained)
results in ~770MB of trash* (according to "du -shc"), after it's
reduced to around 590M, so almost 1/4 bytes we wrote were the same
sample hooks and other repetitive data. The number of files created
went down from ~151k to ~110k, which around the same reduction
of (more than) 1/4.

This will also make our tests more reliable as we're now forced to
check whether our software works with any arbitrary --template that
may be in use in the wild, as opposed to our relatively "fat" current
default.

On the "more on that below", these are the directories we're
creating (only in the test suite) for now:

- "hooks": Removing it will be relatively simple, mostly a matter of a
  few tests needing a "mkdir .git/hooks", or the tests assume that
  they'll need to explicitly disable .git/hooks.

- "branches": Similarly trivial. The last attempt to get rid of it was
  aborted in c8a58ac5a52 (Revert "Don't create the $GIT_DIR/branches
  directory on init", 2009-10-31), but only a couple of tests files
  depend on it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/init-db.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 t/t0001-init.sh   | 19 ++++++++-----------
 t/test-lib.sh     |  6 ++++--
 wrap-for-bin.sh   |  4 +---
 4 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index fcf538193c8..8dddb47bdc4 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -93,10 +93,36 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 	}
 }
 
+static void lazy_mkdir_strbuf_or_die_setlen(struct strbuf *path, size_t oldlen,
+					    const char *dir)
+{
+	strbuf_addstr(path, dir);
+	if (mkdir(path->buf, 0777) < 0) {
+		int saved_errno = errno;
+		struct stat st;
+
+		/*
+		 * Unfortunately there's no EEXIST_{DIR,FILE}, and
+		 * we'd like to pass these only if the path is already
+		 * what we want it to be, not if it's a normal.
+		 */
+		if (lstat(path->buf, &st))
+			die_errno(_("cannot stat '%s'"), path->buf);
+		else if (S_ISDIR(st.st_mode))
+			goto cleanup;
+
+		errno = saved_errno;
+		die_errno(_("cannot mkdir '%s'"), path->buf);
+	}
+cleanup:
+	strbuf_setlen(path, oldlen);
+}
+
 static void copy_templates(int no_template, const char *template_dir,
 			   const char *init_template_dir)
 {
 	struct strbuf path = STRBUF_INIT;
+	size_t len;
 	struct strbuf template_path = STRBUF_INIT;
 	size_t template_len;
 	struct repository_format template_format = REPOSITORY_FORMAT_INIT;
@@ -108,7 +134,7 @@ static void copy_templates(int no_template, const char *template_dir,
 		return;
 	if (!template_dir && !init_template_dir &&
 	    git_env_bool(GIT_NO_TEMPLATE_DIR_ENVIRONMENT, 0))
-		return;
+		goto no_template;
 	if (!template_dir)
 		template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
 	if (!template_dir)
@@ -157,6 +183,19 @@ static void copy_templates(int no_template, const char *template_dir,
 	strbuf_release(&path);
 	strbuf_release(&template_path);
 	clear_repository_format(&template_format);
+	return;
+no_template:
+	if (!git_env_bool("GIT_TEST_BARE_TEMPLATE", 0))
+		return;
+
+	strbuf_addstr(&path, get_git_common_dir());
+	strbuf_complete(&path, '/');
+	len = path.len;
+
+	lazy_mkdir_strbuf_or_die_setlen(&path, len, "hooks");
+	lazy_mkdir_strbuf_or_die_setlen(&path, len, "branches");
+
+	strbuf_release(&path);
 }
 
 /*
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 39cf132e9a0..a91de28aadc 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -235,7 +235,7 @@ test_expect_success 'init with --template' '
 
 test_expect_success 'init with --template (blank)' '
 	git init template-plain &&
-	test_path_is_file template-plain/.git/info/exclude &&
+	test_path_is_dir template-plain/.git/info &&
 	git init --template= template-blank &&
 	test_path_is_missing template-blank/.git/info/exclude
 '
@@ -246,19 +246,12 @@ no_templatedir_env () {
 	export NO_SET_GIT_TEMPLATE_DIR
 }
 
-init_no_templatedir_env () {
-	(
-		no_templatedir_env &&
-		git init "$1"
-	)
-}
-
 test_expect_success 'init with init.templatedir set' '
 	mkdir templatedir-source &&
 	echo Content >templatedir-source/file &&
 	test_config_global init.templatedir "${HOME}/templatedir-source" &&
 
-	init_no_templatedir_env templatedir-set &&
+	git init templatedir-set &&
 	test_cmp templatedir-source/file templatedir-set/.git/file
 '
 
@@ -267,7 +260,7 @@ test_expect_success 'init with init.templatedir using ~ expansion' '
 	echo Content >templatedir-source/file &&
 	test_config_global init.templatedir "~/templatedir-source" &&
 
-	init_no_templatedir_env templatedir-expansion &&
+	git init templatedir-expansion &&
 	test_cmp templatedir-source/file templatedir-expansion/.git/file
 '
 
@@ -563,15 +556,19 @@ test_expect_success 'remote init from does not use config from cwd' '
 '
 
 test_expect_success 're-init from a linked worktree' '
-	git init main-worktree &&
 	(
+		git init main-worktree &&
+
 		cd main-worktree &&
 		test_commit first &&
 		git worktree add ../linked-worktree &&
+		>empty &&
+		cp empty .git/info/exclude &&
 		mv .git/info/exclude expected-exclude &&
 		cp .git/config expected-config &&
 		find .git/worktrees -print | sort >expected &&
 		git -C ../linked-worktree init &&
+		cp empty .git/info/exclude &&
 		test_cmp expected-exclude .git/info/exclude &&
 		test_cmp expected-config .git/config &&
 		find .git/worktrees -print | sort >actual &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 57efcc5e97a..bd09d691da3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1355,11 +1355,13 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 		PATH="$GIT_BUILD_DIR:$GIT_BUILD_DIR/t/helper:$PATH"
 	fi
 fi
-GIT_TEMPLATE_DIR="$GIT_BUILD_DIR"/templates/blt
+GIT_NO_TEMPLATE_DIR=true
+GIT_TEST_BARE_TEMPLATE=true
+export GIT_NO_TEMPLATE_DIR GIT_TEST_BARE_TEMPLATE
 GIT_CONFIG_NOSYSTEM=1
 GIT_ATTR_NOSYSTEM=1
 GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/.."
-export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_ATTR_NOSYSTEM GIT_CEILING_DIRECTORIES
+export PATH GIT_EXEC_PATH GIT_CONFIG_NOSYSTEM GIT_ATTR_NOSYSTEM GIT_CEILING_DIRECTORIES
 
 if test -z "$GIT_TEST_CMP"
 then
diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
index 95851b85b6b..26efe0b60ac 100644
--- a/wrap-for-bin.sh
+++ b/wrap-for-bin.sh
@@ -7,10 +7,8 @@
 # @@BUILD_DIR@@ and @@PROG@@.
 
 GIT_EXEC_PATH='@@BUILD_DIR@@'
-if test -n "$NO_SET_GIT_TEMPLATE_DIR"
+if test -z "$GIT_TEMPLATE_DIR" && test -z "$GIT_NO_TEMPLATE_DIR"
 then
-	unset GIT_TEMPLATE_DIR
-else
 	GIT_TEMPLATE_DIR='@@BUILD_DIR@@/templates/blt'
 	export GIT_TEMPLATE_DIR
 fi
-- 
2.34.1.1020.gb1392dd1877

[PATCH 08/13] tests: don't depend on template-created .git/branches

[Ævar Arnfjörð Bjarmason]

As noted in c8a58ac5a52 (Revert "Don't create the $GIT_DIR/branches
directory on init", 2009-10-31) there was an attempt long ago in
0cc5691a8b0 (Don't create the $GIT_DIR/branches directory on init,
2009-10-30) to get rid of the legacy "branches" directory.

We should probably get rid of its creation by removing the
"templates/branches--" file, but whatever we do with that we don't
need to be creating it in the templates that drive our own tests.

By removing this dependency it'll be more obvious what tests depend on
the existence of ".git/branches", and we can remove the first of the
three special-cases added to copy_template() in a preceding commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/init-db.c     | 1 -
 t/t5505-remote.sh     | 2 ++
 t/t5516-fetch-push.sh | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 8dddb47bdc4..3700a6b854e 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -193,7 +193,6 @@ static void copy_templates(int no_template, const char *template_dir,
 	len = path.len;
 
 	lazy_mkdir_strbuf_or_die_setlen(&path, len, "hooks");
-	lazy_mkdir_strbuf_or_die_setlen(&path, len, "branches");
 
 	strbuf_release(&path);
 }
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index e6e3c8f552c..3bd8ec05802 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -929,6 +929,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 	(
 		cd six &&
 		git remote rm origin &&
+		mkdir .git/branches &&
 		echo "$origin_url#main" >.git/branches/origin &&
 		git remote rename origin origin &&
 		test_path_is_missing .git/branches/origin &&
@@ -943,6 +944,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches (2)'
 	(
 		cd seven &&
 		git remote rm origin &&
+		mkdir .git/branches &&
 		echo "quux#foom" > .git/branches/origin &&
 		git remote rename origin origin &&
 		test_path_is_missing .git/branches/origin &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 7831a38ddef..eea191d042f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -921,6 +921,7 @@ test_expect_success 'fetch with branches' '
 	mk_empty testrepo &&
 	git branch second $the_first_commit &&
 	git checkout second &&
+	mkdir testrepo/.git/branches &&
 	echo ".." > testrepo/.git/branches/branch1 &&
 	(
 		cd testrepo &&
@@ -934,6 +935,7 @@ test_expect_success 'fetch with branches' '
 
 test_expect_success 'fetch with branches containing #' '
 	mk_empty testrepo &&
+	mkdir testrepo/.git/branches &&
 	echo "..#second" > testrepo/.git/branches/branch2 &&
 	(
 		cd testrepo &&
@@ -948,6 +950,7 @@ test_expect_success 'fetch with branches containing #' '
 test_expect_success 'push with branches' '
 	mk_empty testrepo &&
 	git checkout second &&
+	mkdir .git/branches &&
 	echo "testrepo" > .git/branches/branch1 &&
 	git push branch1 &&
 	(
-- 
2.34.1.1020.gb1392dd1877

[PATCH 09/13] t5540: don't rely on "hook/post-update.sample"

[Ævar Arnfjörð Bjarmason]

Change code added in a87679339c0 (test: rename http fetch and push
test files, 2014-02-06) to stop relying on the "exec git
update-server-info" in "templates/hooks--post-update.sample", let's
instead inline the expected hook in the test itself.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5540-http-push-webdav.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/t5540-http-push-webdav.sh b/t/t5540-http-push-webdav.sh
index 8b68bb38a44..8b1f76fb3b8 100755
--- a/t/t5540-http-push-webdav.sh
+++ b/t/t5540-http-push-webdav.sh
@@ -36,7 +36,9 @@ test_expect_success 'setup remote repository' '
 	git clone --bare test_repo test_repo.git &&
 	cd test_repo.git &&
 	git --bare update-server-info &&
-	mv hooks/post-update.sample hooks/post-update &&
+	write_script "hooks/post-update" <<-\EOF &&
+	exec git update-server-info
+	EOF
 	ORIG_HEAD=$(git rev-parse --verify HEAD) &&
 	cd - &&
 	mv test_repo.git "$HTTPD_DOCUMENT_ROOT_PATH"
-- 
2.34.1.1020.gb1392dd1877

[PATCH 10/13] test-lib-functions: add and use a "write_hook" wrapper

[Ævar Arnfjörð Bjarmason]

Add a "write_hook" wrapper for the common case of "write_script
.git/hooks/<NAME>". This also accepts a "-C" option like
"test_commit". Let's convert various trivial cases of "write_script"
over to it.

For now this doesn't have much of an advantage, but in a subsequent
commit we'll implicitly create the leading "hooks" directory. This
will help us get rid of our --template dependency for "hooks".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t1416-ref-transaction-hooks.sh    | 14 +++++++-------
 t/t5406-remote-rejects.sh           |  2 +-
 t/t5409-colorize-remote-messages.sh |  2 +-
 t/t5411-proc-receive-hook.sh        |  4 ++--
 t/t5510-fetch.sh                    |  6 +++---
 t/t5516-fetch-push.sh               |  4 ++--
 t/t5521-pull-options.sh             |  4 ++--
 t/t5547-push-quarantine.sh          |  4 ++--
 t/t5548-push-porcelain.sh           |  2 +-
 t/t6500-gc.sh                       |  4 ++--
 t/test-lib-functions.sh             | 26 ++++++++++++++++++++++++++
 11 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 6c941027a81..4532c90274b 100755
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -18,7 +18,7 @@ test_expect_success setup '
 test_expect_success 'hook allows updating ref if successful' '
 	test_when_finished "rm .git/hooks/reference-transaction" &&
 	git reset --hard PRE &&
-	write_script .git/hooks/reference-transaction <<-\EOF &&
+	write_hook reference-transaction <<-\EOF &&
 		echo "$*" >>actual
 	EOF
 	cat >expect <<-EOF &&
@@ -32,7 +32,7 @@ test_expect_success 'hook allows updating ref if successful' '
 test_expect_success 'hook aborts updating ref in prepared state' '
 	test_when_finished "rm .git/hooks/reference-transaction" &&
 	git reset --hard PRE &&
-	write_script .git/hooks/reference-transaction <<-\EOF &&
+	write_hook reference-transaction <<-\EOF &&
 		if test "$1" = prepared
 		then
 			exit 1
@@ -45,7 +45,7 @@ test_expect_success 'hook aborts updating ref in prepared state' '
 test_expect_success 'hook gets all queued updates in prepared state' '
 	test_when_finished "rm .git/hooks/reference-transaction actual" &&
 	git reset --hard PRE &&
-	write_script .git/hooks/reference-transaction <<-\EOF &&
+	write_hook reference-transaction <<-\EOF &&
 		if test "$1" = prepared
 		then
 			while read -r line
@@ -68,7 +68,7 @@ test_expect_success 'hook gets all queued updates in prepared state' '
 test_expect_success 'hook gets all queued updates in committed state' '
 	test_when_finished "rm .git/hooks/reference-transaction actual" &&
 	git reset --hard PRE &&
-	write_script .git/hooks/reference-transaction <<-\EOF &&
+	write_hook reference-transaction <<-\EOF &&
 		if test "$1" = committed
 		then
 			while read -r line
@@ -88,7 +88,7 @@ test_expect_success 'hook gets all queued updates in committed state' '
 test_expect_success 'hook gets all queued updates in aborted state' '
 	test_when_finished "rm .git/hooks/reference-transaction actual" &&
 	git reset --hard PRE &&
-	write_script .git/hooks/reference-transaction <<-\EOF &&
+	write_hook reference-transaction <<-\EOF &&
 		if test "$1" = aborted
 		then
 			while read -r line
@@ -115,11 +115,11 @@ test_expect_success 'interleaving hook calls succeed' '
 
 	git init --bare target-repo.git &&
 
-	write_script target-repo.git/hooks/reference-transaction <<-\EOF &&
+	write_hook -C target-repo.git reference-transaction <<-\EOF &&
 		echo $0 "$@" >>actual
 	EOF
 
-	write_script target-repo.git/hooks/update <<-\EOF &&
+	write_hook -C target-repo.git update <<-\EOF &&
 		echo $0 "$@" >>actual
 	EOF
 
diff --git a/t/t5406-remote-rejects.sh b/t/t5406-remote-rejects.sh
index 5c509db6fc3..c49aeea272c 100755
--- a/t/t5406-remote-rejects.sh
+++ b/t/t5406-remote-rejects.sh
@@ -5,7 +5,7 @@ test_description='remote push rejects are reported by client'
 . ./test-lib.sh
 
 test_expect_success 'setup' '
-	write_script .git/hooks/update <<-\EOF &&
+	write_hook update <<-\EOF &&
 	exit 1
 	EOF
 	echo 1 >file &&
diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh
index 9f1a483f426..1f7cad31442 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -5,7 +5,7 @@ test_description='remote messages are colorized on the client'
 . ./test-lib.sh
 
 test_expect_success 'setup' '
-	write_script .git/hooks/update <<-\EOF &&
+	write_hook update <<-\EOF &&
 	echo error: error
 	echo ERROR: also highlighted
 	echo hint: hint
diff --git a/t/t5411-proc-receive-hook.sh b/t/t5411-proc-receive-hook.sh
index 98b0e812082..054f4b2efb3 100755
--- a/t/t5411-proc-receive-hook.sh
+++ b/t/t5411-proc-receive-hook.sh
@@ -36,7 +36,7 @@ setup_upstream_and_workbench () {
 		TAG=$(git -C workbench rev-parse v123) &&
 
 		# setup pre-receive hook
-		write_script upstream.git/hooks/pre-receive <<-\EOF &&
+		write_hook -C upstream.git pre-receive <<-\EOF &&
 		exec >&2
 		echo "# pre-receive hook"
 		while read old new ref
@@ -46,7 +46,7 @@ setup_upstream_and_workbench () {
 		EOF
 
 		# setup post-receive hook
-		write_script upstream.git/hooks/post-receive <<-\EOF &&
+		write_hook -C upstream.git post-receive <<-\EOF &&
 		exec >&2
 		echo "# post-receive hook"
 		while read old new ref
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index a0faf0dd949..cdce39e6c76 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -265,7 +265,7 @@ test_expect_success 'fetch --atomic executes a single reference transaction only
 	EOF
 
 	rm -f atomic/actual &&
-	write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
+	write_hook -C atomic reference-transaction <<-\EOF &&
 		( echo "$*" && cat ) >>actual
 	EOF
 
@@ -298,7 +298,7 @@ test_expect_success 'fetch --atomic aborts all reference updates if hook aborts'
 	EOF
 
 	rm -f atomic/actual &&
-	write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
+	write_hook -C atomic/.git reference-transaction <<-\EOF &&
 		( echo "$*" && cat ) >>actual
 		exit 1
 	EOF
@@ -326,7 +326,7 @@ test_expect_success 'fetch --atomic --append appends to FETCH_HEAD' '
 	test_line_count = 2 atomic/.git/FETCH_HEAD &&
 	cp atomic/.git/FETCH_HEAD expected &&
 
-	write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
+	write_hook -C atomic reference-transaction <<-\EOF &&
 		exit 1
 	EOF
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index eea191d042f..11458052cb4 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1680,7 +1680,7 @@ test_expect_success 'updateInstead with push-to-checkout hook' '
 		git reset --hard HEAD^^ &&
 		git tag initial &&
 		git config receive.denyCurrentBranch updateInstead &&
-		write_script .git/hooks/push-to-checkout <<-\EOF
+		write_hook push-to-checkout <<-\EOF
 		echo >&2 updating from $(git rev-parse HEAD)
 		echo >&2 updating to "$1"
 
@@ -1739,7 +1739,7 @@ test_expect_success 'updateInstead with push-to-checkout hook' '
 	(
 		cd void &&
 		git config receive.denyCurrentBranch updateInstead &&
-		write_script .git/hooks/push-to-checkout <<-\EOF
+		write_hook push-to-checkout <<-\EOF
 		if git rev-parse --quiet --verify HEAD
 		then
 			has_head=yes
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 66cfcb09c55..9b30e90383c 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -233,7 +233,7 @@ test_expect_success 'git pull --no-verify flag passed to merge' '
 	git init src &&
 	test_commit -C src one &&
 	git clone src dst &&
-	write_script dst/.git/hooks/commit-msg <<-\EOF &&
+	write_hook -C dst commit-msg <<-\EOF &&
 	false
 	EOF
 	test_commit -C src two &&
@@ -245,7 +245,7 @@ test_expect_success 'git pull --no-verify --verify passed to merge' '
 	git init src &&
 	test_commit -C src one &&
 	git clone src dst &&
-	write_script dst/.git/hooks/commit-msg <<-\EOF &&
+	write_hook -C dst commit-msg <<-\EOF &&
 	false
 	EOF
 	test_commit -C src two &&
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index faaa51ccc56..e0bd469b110 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -5,7 +5,7 @@ test_description='check quarantine of objects during push'
 
 test_expect_success 'create picky dest repo' '
 	git init --bare dest.git &&
-	write_script dest.git/hooks/pre-receive <<-\EOF
+	write_hook -C dest.git pre-receive <<-\EOF
 	while read old new ref; do
 		test "$(git log -1 --format=%s $new)" = reject && exit 1
 	done
@@ -60,7 +60,7 @@ test_expect_success 'push to repo path with path separator (colon)' '
 
 test_expect_success 'updating a ref from quarantine is forbidden' '
 	git init --bare update.git &&
-	write_script update.git/hooks/pre-receive <<-\EOF &&
+	write_hook -C update.git pre-receive <<-\EOF &&
 	read old new refname
 	git update-ref refs/heads/unrelated $new
 	exit 1
diff --git a/t/t5548-push-porcelain.sh b/t/t5548-push-porcelain.sh
index f11ff57e549..8997433a992 100755
--- a/t/t5548-push-porcelain.sh
+++ b/t/t5548-push-porcelain.sh
@@ -168,7 +168,7 @@ run_git_push_porcelain_output_test() {
 	'
 
 	test_expect_success "prepare pre-receive hook ($PROTOCOL)" '
-		write_script "$upstream/hooks/pre-receive" <<-EOF
+		write_hook -C "$upstream" pre-receive <<-EOF
 		exit 1
 		EOF
 	'
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index c2021267f2c..98cf395806b 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -103,7 +103,7 @@ test_expect_success 'pre-auto-gc hook can stop auto gc' '
 	git init pre-auto-gc-hook &&
 	(
 		cd pre-auto-gc-hook &&
-		write_script ".git/hooks/pre-auto-gc" <<-\EOF &&
+		write_hook pre-auto-gc <<-\EOF &&
 		echo >&2 no gc for you &&
 		exit 1
 		EOF
@@ -130,7 +130,7 @@ test_expect_success 'pre-auto-gc hook can stop auto gc' '
 
 	(
 		cd pre-auto-gc-hook &&
-		write_script ".git/hooks/pre-auto-gc" <<-\EOF &&
+		write_hook pre-auto-gc <<-\EOF &&
 		echo >&2 will gc for you &&
 		exit 0
 		EOF
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index eef2262a360..3786d39ccab 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -551,6 +551,32 @@ write_script () {
 	chmod +x "$1"
 }
 
+## Usage: write-hook pre-receive
+## Usage: write-hook -C some-dir pre-receive
+write_hook () {
+	indir= &&
+	while test $# != 0
+	do
+		case "$1" in
+		-C)
+			indir="$2"
+			shift
+			;;
+		-*)
+			BUG "invalid write_hook: $1"
+			;;
+		*)
+			break
+			;;
+		esac &&
+		shift
+	done &&
+	git_dir=$(git -C "$indir" rev-parse --absolute-git-dir) &&
+	hook_dir="$git_dir/hooks" &&
+	hook_file="$hook_dir/$1"
+	write_script "$hook_file"
+}
+
 # Use test_set_prereq to tell that a particular prerequisite is available.
 # The prerequisite can later be checked for in two ways:
 #
-- 
2.34.1.1020.gb1392dd1877

[PATCH 11/13] tests: change "cat && chmod +x" to use "write_hook"

[Ævar Arnfjörð Bjarmason]

Refactor various test code to use the "write_hook()" helper. Let's
indent this code and add it to "test_expect_success" while we're at
it.

As in a preceding commit some of this code drops the explicit "mkdir
-p", but as noted we'll be having the "write_hook" wrapper handle that
soon anyway.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3412-rebase-root.sh       | 18 ++++-------
 t/t3413-rebase-hook.sh       | 18 ++++-------
 t/t5401-update-hooks.sh      | 62 ++++++++++++++++--------------------
 t/t5402-post-merge-hook.sh   | 16 ++++++----
 t/t5407-post-rewrite-hook.sh | 14 ++++----
 t/t5516-fetch-push.sh        | 15 +++------
 t/t5541-http-push-smart.sh   |  4 +--
 t/t5601-clone.sh             |  4 +--
 8 files changed, 60 insertions(+), 91 deletions(-)

diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
index 19c6f4acbf6..c2dfb562961 100755
--- a/t/t3412-rebase-root.sh
+++ b/t/t3412-rebase-root.sh
@@ -31,12 +31,9 @@ test_expect_success 'rebase --root fails with too many args' '
 '
 
 test_expect_success 'setup pre-rebase hook' '
-	mkdir -p .git/hooks &&
-	cat >.git/hooks/pre-rebase <<EOF &&
-#!$SHELL_PATH
-echo "\$1,\$2" >.git/PRE-REBASE-INPUT
-EOF
-	chmod +x .git/hooks/pre-rebase
+	write_hook pre-rebase <<-\EOF
+	echo "$1,$2" >.git/PRE-REBASE-INPUT
+	EOF
 '
 cat > expect <<EOF
 4
@@ -141,12 +138,9 @@ commit work7~5
 EOF
 
 test_expect_success 'setup pre-rebase hook that fails' '
-	mkdir -p .git/hooks &&
-	cat >.git/hooks/pre-rebase <<EOF &&
-#!$SHELL_PATH
-false
-EOF
-	chmod +x .git/hooks/pre-rebase
+	write_hook pre-rebase <<-\EOF
+	false
+	EOF
 '
 
 test_expect_success 'pre-rebase hook stops rebase' '
diff --git a/t/t3413-rebase-hook.sh b/t/t3413-rebase-hook.sh
index b4acb3be5cf..100157aa0ab 100755
--- a/t/t3413-rebase-hook.sh
+++ b/t/t3413-rebase-hook.sh
@@ -41,12 +41,9 @@ test_expect_success 'rebase -i' '
 '
 
 test_expect_success 'setup pre-rebase hook' '
-	mkdir -p .git/hooks &&
-	cat >.git/hooks/pre-rebase <<EOF &&
-#!$SHELL_PATH
-echo "\$1,\$2" >.git/PRE-REBASE-INPUT
-EOF
-	chmod +x .git/hooks/pre-rebase
+	write_hook pre-rebase <<-\EOF
+	echo "$1,$2" >.git/PRE-REBASE-INPUT
+	EOF
 '
 
 test_expect_success 'pre-rebase hook gets correct input (1)' '
@@ -102,12 +99,9 @@ test_expect_success 'pre-rebase hook gets correct input (6)' '
 '
 
 test_expect_success 'setup pre-rebase hook that fails' '
-	mkdir -p .git/hooks &&
-	cat >.git/hooks/pre-rebase <<EOF &&
-#!$SHELL_PATH
-false
-EOF
-	chmod +x .git/hooks/pre-rebase
+	write_hook pre-rebase <<-\EOF
+	false
+	EOF
 '
 
 test_expect_success 'pre-rebase hook stops rebase (1)' '
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 6012cc8172a..1bd656d35ce 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -20,45 +20,37 @@ test_expect_success setup '
 	git clone --bare ./. victim.git &&
 	GIT_DIR=victim.git git update-ref refs/heads/tofail $commit1 &&
 	git update-ref refs/heads/main $commit1 &&
-	git update-ref refs/heads/tofail $commit0
-'
+	git update-ref refs/heads/tofail $commit0 &&
 
-cat >victim.git/hooks/pre-receive <<'EOF'
-#!/bin/sh
-printf %s "$@" >>$GIT_DIR/pre-receive.args
-cat - >$GIT_DIR/pre-receive.stdin
-echo STDOUT pre-receive
-echo STDERR pre-receive >&2
-EOF
-chmod u+x victim.git/hooks/pre-receive
+	write_hook -C victim.git pre-receive <<-\EOF &&
+	printf %s "$@" >>$GIT_DIR/pre-receive.args
+	cat - >$GIT_DIR/pre-receive.stdin
+	echo STDOUT pre-receive
+	echo STDERR pre-receive >&2
+	EOF
 
-cat >victim.git/hooks/update <<'EOF'
-#!/bin/sh
-echo "$@" >>$GIT_DIR/update.args
-read x; printf %s "$x" >$GIT_DIR/update.stdin
-echo STDOUT update $1
-echo STDERR update $1 >&2
-test "$1" = refs/heads/main || exit
-EOF
-chmod u+x victim.git/hooks/update
+	write_hook -C victim.git update <<-\EOF &&
+	echo "$@" >>$GIT_DIR/update.args
+	read x; printf %s "$x" >$GIT_DIR/update.stdin
+	echo STDOUT update $1
+	echo STDERR update $1 >&2
+	test "$1" = refs/heads/main || exit
+	EOF
 
-cat >victim.git/hooks/post-receive <<'EOF'
-#!/bin/sh
-printf %s "$@" >>$GIT_DIR/post-receive.args
-cat - >$GIT_DIR/post-receive.stdin
-echo STDOUT post-receive
-echo STDERR post-receive >&2
-EOF
-chmod u+x victim.git/hooks/post-receive
+	write_hook -C victim.git post-receive <<-\EOF &&
+	printf %s "$@" >>$GIT_DIR/post-receive.args
+	cat - >$GIT_DIR/post-receive.stdin
+	echo STDOUT post-receive
+	echo STDERR post-receive >&2
+	EOF
 
-cat >victim.git/hooks/post-update <<'EOF'
-#!/bin/sh
-echo "$@" >>$GIT_DIR/post-update.args
-read x; printf %s "$x" >$GIT_DIR/post-update.stdin
-echo STDOUT post-update
-echo STDERR post-update >&2
-EOF
-chmod u+x victim.git/hooks/post-update
+	write_hook -C victim.git post-update <<-\EOF
+	echo "$@" >>$GIT_DIR/post-update.args
+	read x; printf %s "$x" >$GIT_DIR/post-update.stdin
+	echo STDOUT post-update
+	echo STDERR post-update >&2
+	EOF
+'
 
 test_expect_success push '
 	test_must_fail git send-pack --force ./victim.git \
diff --git a/t/t5402-post-merge-hook.sh b/t/t5402-post-merge-hook.sh
index 3e5e19c7191..c425a807efe 100755
--- a/t/t5402-post-merge-hook.sh
+++ b/t/t5402-post-merge-hook.sh
@@ -25,13 +25,15 @@ test_expect_success setup '
 	GIT_DIR=clone2/.git git update-index --add a
 '
 
-for clone in 1 2; do
-	cat >clone${clone}/.git/hooks/post-merge <<'EOF'
-#!/bin/sh
-echo $@ >> $GIT_DIR/post-merge.args
-EOF
-	chmod u+x clone${clone}/.git/hooks/post-merge
-done
+test_expect_success 'setup clone hooks' '
+	test_when_finished "rm -f hook" &&
+	cat >hook <<-\EOF &&
+	echo $@ >> $GIT_DIR/post-merge.args
+	EOF
+
+	write_hook -C clone1 post-merge <hook &&
+	write_hook -C clone2 post-merge <hook
+'
 
 test_expect_success 'post-merge does not run for up-to-date ' '
 	GIT_DIR=clone1/.git git merge $commit0 &&
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index 6da8d760e28..7a2bf55d8e2 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -17,15 +17,13 @@ test_expect_success 'setup' '
 	git checkout A^0 &&
 	test_commit E bar E &&
 	test_commit F foo F &&
-	git checkout main
-'
+	git checkout main &&
 
-cat >.git/hooks/post-rewrite <<EOF
-#!/bin/sh
-echo \$@ > "$TRASH_DIRECTORY"/post-rewrite.args
-cat > "$TRASH_DIRECTORY"/post-rewrite.data
-EOF
-chmod u+x .git/hooks/post-rewrite
+	write_hook post-rewrite <<-EOF
+	echo \$@ > "$TRASH_DIRECTORY"/post-rewrite.args
+	cat > "$TRASH_DIRECTORY"/post-rewrite.data
+	EOF
+'
 
 clear_hook_input () {
 	rm -f post-rewrite.args post-rewrite.data
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 11458052cb4..fd355ae48c6 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -62,32 +62,25 @@ mk_test_with_hooks() {
 	(
 		cd "$repo_name" &&
 		mkdir .git/hooks &&
-		cd .git/hooks &&
 
-		cat >pre-receive <<-'EOF' &&
-		#!/bin/sh
+		write_hook pre-receive <<-'EOF' &&
 		cat - >>pre-receive.actual
 		EOF
 
-		cat >update <<-'EOF' &&
-		#!/bin/sh
+		write_hook update <<-'EOF' &&
 		printf "%s %s %s\n" "$@" >>update.actual
 		EOF
 
-		cat >post-receive <<-'EOF' &&
-		#!/bin/sh
+		write_hook post-receive <<-'EOF' &&
 		cat - >>post-receive.actual
 		EOF
 
-		cat >post-update <<-'EOF' &&
-		#!/bin/sh
+		write_hook post-update <<-'EOF'
 		for ref in "$@"
 		do
 			printf "%s\n" "$ref" >>post-update.actual
 		done
 		EOF
-
-		chmod +x pre-receive update post-receive post-update
 	)
 }
 
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 8ca50f8b18c..11da2325c6d 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -96,11 +96,9 @@ test_expect_success 'create and delete remote branch' '
 	test_must_fail git show-ref --verify refs/remotes/origin/dev
 '
 
-cat >"$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update" <<EOF
-#!/bin/sh
+write_hook -C "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" update <<\EOF
 exit 1
 EOF
-chmod a+x "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update"
 
 cat >exp <<EOF
 remote: error: hook declined to update refs/heads/dev2
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 83c24fc97a7..6fd041e7d3e 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -79,12 +79,10 @@ test_expect_success 'clone from hooks' '
 	cd .. &&
 	git init r1 &&
 	cd r1 &&
-	cat >.git/hooks/pre-commit <<-\EOF &&
-	#!/bin/sh
+	write_hook pre-commit <<-\EOF &&
 	git clone ../r0 ../r2
 	exit 1
 	EOF
-	chmod u+x .git/hooks/pre-commit &&
 	: >file &&
 	git add file &&
 	test_must_fail git commit -m invoke-hook &&
-- 
2.34.1.1020.gb1392dd1877

[PATCH 12/13] tests: migrate miscellaneous "write_script" to "write_hooks"

[Ævar Arnfjörð Bjarmason]

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3430-rebase-merges.sh    | 5 +++--
 t/t5540-http-push-webdav.sh | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 43c82d9a33b..8003e41427c 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -292,9 +292,10 @@ test_expect_success 'post-rewrite hook and fixups work for merges' '
 	git commit --fixup HEAD same2.t &&
 	fixup="$(git rev-parse HEAD)" &&
 
-	mkdir -p .git/hooks &&
 	test_when_finished "rm .git/hooks/post-rewrite" &&
-	echo "cat >actual" | write_script .git/hooks/post-rewrite &&
+	write_hook post-rewrite <<-\EOF &&
+	cat >actual
+	EOF
 
 	test_tick &&
 	git rebase -i --autosquash -r HEAD^^^ &&
diff --git a/t/t5540-http-push-webdav.sh b/t/t5540-http-push-webdav.sh
index 8b1f76fb3b8..31d13547b26 100755
--- a/t/t5540-http-push-webdav.sh
+++ b/t/t5540-http-push-webdav.sh
@@ -36,7 +36,7 @@ test_expect_success 'setup remote repository' '
 	git clone --bare test_repo test_repo.git &&
 	cd test_repo.git &&
 	git --bare update-server-info &&
-	write_script "hooks/post-update" <<-\EOF &&
+	write_hook post-update <<-\EOF &&
 	exec git update-server-info
 	EOF
 	ORIG_HEAD=$(git rev-parse --verify HEAD) &&
-- 
2.34.1.1020.gb1392dd1877

[PATCH 13/13] tests: don't depend on template-created .git/hooks

[Ævar Arnfjörð Bjarmason]

Change the "write_hook" wrapper to implicitly "mkdir -p
.git/hooks" (or equivalent), and stop having copy_templates() make the
"hooks" directory. One test in "t5516-fetch-push.sh" won't need to
move our hooks out of the way anymore.

As with a preceding change to drop the dependency on the
template-created "branches" we can now stop depending on the template
having created the "hooks" directory for us.

Since this was the last special-case handled by the
"lazy_mkdir_strbuf_or_die_setlen()" function added earlier in this
series we can remove that special-case and the
"GIT_TEST_BARE_TEMPLATE" handling.

The choice to not use "mkdir -p" in "write_hook" is deliberate. We're
being a bit stricter in not potentially creating N leading
directories, but also not failing on the second "write_hook"
invocation in a repository as a simple "mkdir" without "-p" would.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/init-db.c           | 39 +------------------------------------
 t/t5516-fetch-push.sh       |  4 +---
 t/t7450-bad-git-dotfiles.sh |  1 +
 t/test-lib-functions.sh     |  4 ++++
 t/test-lib.sh               |  3 +--
 5 files changed, 8 insertions(+), 43 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 3700a6b854e..0301b8f613e 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -93,36 +93,10 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 	}
 }
 
-static void lazy_mkdir_strbuf_or_die_setlen(struct strbuf *path, size_t oldlen,
-					    const char *dir)
-{
-	strbuf_addstr(path, dir);
-	if (mkdir(path->buf, 0777) < 0) {
-		int saved_errno = errno;
-		struct stat st;
-
-		/*
-		 * Unfortunately there's no EEXIST_{DIR,FILE}, and
-		 * we'd like to pass these only if the path is already
-		 * what we want it to be, not if it's a normal.
-		 */
-		if (lstat(path->buf, &st))
-			die_errno(_("cannot stat '%s'"), path->buf);
-		else if (S_ISDIR(st.st_mode))
-			goto cleanup;
-
-		errno = saved_errno;
-		die_errno(_("cannot mkdir '%s'"), path->buf);
-	}
-cleanup:
-	strbuf_setlen(path, oldlen);
-}
-
 static void copy_templates(int no_template, const char *template_dir,
 			   const char *init_template_dir)
 {
 	struct strbuf path = STRBUF_INIT;
-	size_t len;
 	struct strbuf template_path = STRBUF_INIT;
 	size_t template_len;
 	struct repository_format template_format = REPOSITORY_FORMAT_INIT;
@@ -134,7 +108,7 @@ static void copy_templates(int no_template, const char *template_dir,
 		return;
 	if (!template_dir && !init_template_dir &&
 	    git_env_bool(GIT_NO_TEMPLATE_DIR_ENVIRONMENT, 0))
-		goto no_template;
+		return;
 	if (!template_dir)
 		template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
 	if (!template_dir)
@@ -184,17 +158,6 @@ static void copy_templates(int no_template, const char *template_dir,
 	strbuf_release(&template_path);
 	clear_repository_format(&template_format);
 	return;
-no_template:
-	if (!git_env_bool("GIT_TEST_BARE_TEMPLATE", 0))
-		return;
-
-	strbuf_addstr(&path, get_git_common_dir());
-	strbuf_complete(&path, '/');
-	len = path.len;
-
-	lazy_mkdir_strbuf_or_die_setlen(&path, len, "hooks");
-
-	strbuf_release(&path);
 }
 
 /*
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index fd355ae48c6..20677c84117 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -28,8 +28,7 @@ mk_empty () {
 	(
 		cd "$repo_name" &&
 		git init &&
-		git config receive.denyCurrentBranch warn &&
-		mv .git/hooks .git/hooks-disabled
+		git config receive.denyCurrentBranch warn
 	)
 }
 
@@ -61,7 +60,6 @@ mk_test_with_hooks() {
 	mk_test "$@" &&
 	(
 		cd "$repo_name" &&
-		mkdir .git/hooks &&
 
 		write_hook pre-receive <<-'EOF' &&
 		cat - >>pre-receive.actual
diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
index 41706c1c9ff..425440d40b7 100755
--- a/t/t7450-bad-git-dotfiles.sh
+++ b/t/t7450-bad-git-dotfiles.sh
@@ -54,6 +54,7 @@ test_expect_success 'add evil submodule' '
 
 	mkdir modules &&
 	cp -r .git/modules/evil modules &&
+	mkdir modules/evil/hooks &&
 	write_script modules/evil/hooks/post-checkout <<-\EOF &&
 	echo >&2 "RUNNING POST CHECKOUT"
 	EOF
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 3786d39ccab..75fa312f3e9 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -573,6 +573,10 @@ write_hook () {
 	done &&
 	git_dir=$(git -C "$indir" rev-parse --absolute-git-dir) &&
 	hook_dir="$git_dir/hooks" &&
+	if ! test -d "$hook_dir"
+	then
+		mkdir "$hook_dir"
+	fi &&
 	hook_file="$hook_dir/$1"
 	write_script "$hook_file"
 }
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bd09d691da3..3abd51464e6 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1356,8 +1356,7 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 	fi
 fi
 GIT_NO_TEMPLATE_DIR=true
-GIT_TEST_BARE_TEMPLATE=true
-export GIT_NO_TEMPLATE_DIR GIT_TEST_BARE_TEMPLATE
+export GIT_NO_TEMPLATE_DIR
 GIT_CONFIG_NOSYSTEM=1
 GIT_ATTR_NOSYSTEM=1
 GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/.."
-- 
2.34.1.1020.gb1392dd1877

Re: [PATCH 10/13] test-lib-functions: add and use a "write_hook" wrapper

[Eric Sunshine]

On Sun, Dec 12, 2021 at 4:24 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Add a "write_hook" wrapper for the common case of "write_script
> .git/hooks/<NAME>". This also accepts a "-C" option like
> "test_commit". Let's convert various trivial cases of "write_script"
> over to it.
> [...]
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -551,6 +551,32 @@ write_script () {
> +## Usage: write-hook pre-receive
> +## Usage: write-hook -C some-dir pre-receive
> +write_hook () {
> +       indir= &&
> +       while test $# != 0
> +       do

It's not clear whether the intention is to maintain the &&-chain in
this function...

> +               case "$1" in
> +               -C)
> +                       indir="$2"
> +                       shift
> +                       ;;

... or not care about it since it's broken here before `shift`...

> +               -*)
> +                       BUG "invalid write_hook: $1"
> +                       ;;
> +               *)
> +                       break
> +                       ;;
> +               esac &&
> +               shift
> +       done &&
> +       git_dir=$(git -C "$indir" rev-parse --absolute-git-dir) &&
> +       hook_dir="$git_dir/hooks" &&
> +       hook_file="$hook_dir/$1"
> +       write_script "$hook_file"

... and here before `write_script`.

> +}

Re: [PATCH 10/13] test-lib-functions: add and use a "write_hook" wrapper

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 13 2021, Eric Sunshine wrote:

> On Sun, Dec 12, 2021 at 4:24 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Add a "write_hook" wrapper for the common case of "write_script
>> .git/hooks/<NAME>". This also accepts a "-C" option like
>> "test_commit". Let's convert various trivial cases of "write_script"
>> over to it.
>> [...]
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
>> @@ -551,6 +551,32 @@ write_script () {
>> +## Usage: write-hook pre-receive
>> +## Usage: write-hook -C some-dir pre-receive
>> +write_hook () {
>> +       indir= &&
>> +       while test $# != 0
>> +       do
>
> It's not clear whether the intention is to maintain the &&-chain in
> this function...
>
>> +               case "$1" in
>> +               -C)
>> +                       indir="$2"
>> +                       shift
>> +                       ;;
>
> ... or not care about it since it's broken here before `shift`...
>
>> +               -*)
>> +                       BUG "invalid write_hook: $1"
>> +                       ;;
>> +               *)
>> +                       break
>> +                       ;;
>> +               esac &&
>> +               shift
>> +       done &&
>> +       git_dir=$(git -C "$indir" rev-parse --absolute-git-dir) &&
>> +       hook_dir="$git_dir/hooks" &&
>> +       hook_file="$hook_dir/$1"
>> +       write_script "$hook_file"
>
> ... and here before `write_script`.

Thanks, those should all use &&-chaining. Will fix.

Re: [PATCH 10/13] test-lib-functions: add and use a "write_hook" wrapper

[Eric Sunshine]

On Mon, Dec 13, 2021 at 11:29 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, Dec 13 2021, Eric Sunshine wrote:
> > It's not clear whether the intention is to maintain the &&-chain in
> > this function...
> > ... or not care about it since it's broken here before `shift`...
>
> Thanks, those should all use &&-chaining. Will fix.

By the way, the new chainlint could be made to catch broken &&-chains
(and missing `|| return 1`) in test script functions, as well; it
doesn't have to limit its checks only to tests. The reason I haven't
done so yet is that it's not clear how much we care about &&-chains in
functions, especially since we have _so many_ functions which don't
maintain the &&-chain. In the long run, I think it might be beneficial
to extend chainlint to check shell functions too, but fixing the
&&-chains in functions probably have to be done incrementally, thus
would likely require some sort of whitelisting or blacklisting
mechanism until all functions have been fixed. Anyhow, it's food for
thought.

Re: [PATCH 10/13] test-lib-functions: add and use a "write_hook" wrapper

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 13 2021, Eric Sunshine wrote:

> On Mon, Dec 13, 2021 at 11:29 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> On Mon, Dec 13 2021, Eric Sunshine wrote:
>> > It's not clear whether the intention is to maintain the &&-chain in
>> > this function...
>> > ... or not care about it since it's broken here before `shift`...
>>
>> Thanks, those should all use &&-chaining. Will fix.
>
> By the way, the new chainlint could be made to catch broken &&-chains
> (and missing `|| return 1`) in test script functions, as well; it
> doesn't have to limit its checks only to tests. The reason I haven't
> done so yet is that it's not clear how much we care about &&-chains in
> functions, especially since we have _so many_ functions which don't
> maintain the &&-chain. In the long run, I think it might be beneficial
> to extend chainlint to check shell functions too, but fixing the
> &&-chains in functions probably have to be done incrementally, thus
> would likely require some sort of whitelisting or blacklisting
> mechanism until all functions have been fixed. Anyhow, it's food for
> thought.

I think doing that & phasing it in would be very useful.

E.g. if you run the tests with SANITIZE=leak and
GIT_TEST_PASSING_SANITIZE_LEAK=true in "next" now you'll find it passes,
but we'll still (especially if you log them or --verbose-log) have
memory leaks in various places still.

Those aren't new issues in anything I've done in the leak testing mode,
although I'm hoping to eventually getting around to fixing them. They're
just cases where tests pass because some function is lacking a &&.

Although to be fair the SANITIZE=leak default is to die at the very end,
so if the program had something useful to do it probably got around to
doing it, but that doesn't apply when we invoke ourselves via
run-command.c (as that invocation will fail, and we'll usually abort).

But it would have been nice to have had those failures cascade up from
the functions up to the top-level.

We've also said we shouldn't use things like this, i.e. a pipe with git
on the LHS:

    git <cmd> | ... &&

But I've run into a few cases where a test succeeds, even if both
commands here die:

    test "$(git <cmd>)" = "$(git <cmd2>)"

Which, if we're adding more lints is maybe something to consider
too. I.e. it falls under the general umbrella of cases where we'd hide
failures in "git".

Re: [PATCH 10/13] test-lib-functions: add and use a "write_hook" wrapper

[Eric Sunshine]

On Mon, Dec 13, 2021 at 2:42 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, Dec 13 2021, Eric Sunshine wrote:
> > By the way, the new chainlint could be made to catch broken &&-chains
> > (and missing `|| return 1`) in test script functions, as well; it
> > doesn't have to limit its checks only to tests. The reason I haven't
> > done so yet is that it's not clear how much we care about &&-chains in
> > functions, especially since we have _so many_ functions which don't
> > maintain the &&-chain. In the long run, I think it might be beneficial
> > to extend chainlint to check shell functions too, but fixing the
> > &&-chains in functions probably have to be done incrementally, thus
> > would likely require some sort of whitelisting or blacklisting
> > mechanism until all functions have been fixed. Anyhow, it's food for
> > thought.
>
> I think doing that & phasing it in would be very useful.

I forgot to mention another reason that I haven't really thought yet
about tackling the linting of shell functions, which is that some
shell functions drive tests:

    test_it () {
        foo=$1
        bar=$2
        test_expect_sucess "something $foo" '
            do_it "$bar"
        '
    }

in which an &&-chain within the function body isn't meaningful,
whereas other functions are called by tests:

    cmp_it () {
        a=$1 &&
        b=$2 &&
        test_cmp "$a" "$b"
    }

    test_expect_success 'something' '
        echo foo >foo &&
        echo bar >bar &&
        cmp_it foo bar
    '

in which the &&-chain within the function body is important.

It _may_ be possible to figure out automatically into which category a
function falls, but would probably be easier to have some sort of
annotation mechanism to distinguish one category of function from the
other, and only validate a function which falls into the latter
category.

> We've also said we shouldn't use things like this, i.e. a pipe with git
> on the LHS:
>
>     git <cmd> | ... &&
>
> But I've run into a few cases where a test succeeds, even if both
> commands here die:
>
>     test "$(git <cmd>)" = "$(git <cmd2>)"
>
> Which, if we're adding more lints is maybe something to consider
> too. I.e. it falls under the general umbrella of cases where we'd hide
> failures in "git".

Ya, this is a nice example, among others, of questionable code which a
linter might be able to detect. In fact, while working on the new
chainlint, I noted that it would be possible to replace
t/check-non-portable-shell.pl by adding a few more rules to the new
linter. A primary benefit of doing so is that
check-non-portable-shell.pl takes about 2.5 seconds to run on my
(admittedly 10+ year old) machine. However, I'm also hesitant to do so
since there is value in having those checks reside in a standalone
script like that; it's such a simple script that anyone can add new
checks without having to spend a lot of time studying how to do so.

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

[Derrick Stolee]

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?

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

Thanks,
-Stolee

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

[Ævar Arnfjörð Bjarmason]

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.

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

[Derrick Stolee]

On 12/20/2021 11:13 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Dec 20 2021, Derrick Stolee wrote:
> 
>> On 12/12/2021 3:13 PM, Ævar Arnfjörð Bjarmason wrote:
>>> 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.

You continue after this with more motivations for adding 'init' 
unconditionally, which I am not fighting.

What I _am_ saying is important is that if we are trying to write
a file to a known location and its parent directory doesn't exist,
then we should create it. Not doing so is a bug and should be
fixed, no matter how rare such a thing is to occur. As you've
shown, it is not required to have an info directory until we need
one (e.g. for sparse-checkout or an excludes file).

If you're not planning to add that to this series, then I'll add it
to my list. I do think it would fit well into this one, though.

Thanks,
-Stolee

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

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 20 2021, Derrick Stolee wrote:

> On 12/20/2021 11:13 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Mon, Dec 20 2021, Derrick Stolee wrote:
>> 
>>> On 12/12/2021 3:13 PM, Ævar Arnfjörð Bjarmason wrote:
>>>> 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.
>
> You continue after this with more motivations for adding 'init' 
> unconditionally, which I am not fighting.
>
> What I _am_ saying is important is that if we are trying to write
> a file to a known location and its parent directory doesn't exist,
> then we should create it. Not doing so is a bug and should be
> fixed, no matter how rare such a thing is to occur. As you've
> shown, it is not required to have an info directory until we need
> one (e.g. for sparse-checkout or an excludes file).
>
> If you're not planning to add that to this series, then I'll add it
> to my list. I do think it would fit well into this one, though.

Ah, you mean for the case where "git sparse-checkout" will fail because
.git/info/ doesn't exist now (e.g. because it's an existing repo with
--template=).

Yes that bug will not be fixed by this series. I'd welcome a patch to
fix it & could even integrate it with this series.

But I just found it rather "meh" and not that worth fixing. I.e. that it
hasn't been noticed or reported until now shows that this is a very rare
mode of operation. It seems most people just "git init" or "git clone",
or maybe almost nobody uses "sparse-checkout". I don't know.

I did try to fix it in an earlier version of this.

Maybe I went overboard with that, but I didn't just sprinkle
safe_create_leading_directories() to every caller as you suggest, but
rather wanted to change the API so that cases like:

        sparse_filename = get_sparse_checkout_filename();
        res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
        free(sparse_filename);

Would just call some function saying "add to sparse excludes", and not
care about the filename. Since having the API at that levels means you
end up with a lot of accounting work of getting the filename, freeing it
etc.

E.g. this as another example from add_patterns_cone_mode() (the function
makes no other use of sparse_filename):

	char *sparse_filename = get_sparse_checkout_filename();
        [...]
        if (add_patterns_from_file_to_list(sparse_filename, "", 0,
                                           &existing, NULL, 0))
                die(_("unable to load existing sparse-checkout patterns"));
        free(sparse_filename);

But per the upthread rationale I figured "meh" on that and that just
changing "git init" would fix the problem going forward in practice, so
I didn't pursue it.

Won't unconditionally adding a safe_create_leading_directories() also
close the door to having a core.sparseCheckoutFile similar to
core.excludesFile (but maybe it makes no sense). I.e. we'd be free to
"mkdir -p" the .git/info, but if the user is pointing it to some other
path then blindly creating that possibly nested path might be confusing,
as opposed to just immediately erroring out.

So...

All of which is to say that if you'd like to untangle it I'll review it,
and would be happy to include it in this series. But for the
--no-template change I thought I'd just try to sidestep that particular
aspect.

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

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

> What I _am_ saying is important is that if we are trying to write
> a file to a known location and its parent directory doesn't exist,
> then we should create it. Not doing so is a bug and should be
> fixed, no matter how rare such a thing is to occur. As you've
> shown, it is not required to have an info directory until we need
> one (e.g. for sparse-checkout or an excludes file).
>
> If you're not planning to add that to this series, then I'll add it
> to my list. I do think it would fit well into this one, though.

Historically, "git init" relied on the templates to create necessary
directories, and the subcommands in turn learned to depend on the
presence of these directories.

At the same time we allowed that the templates can be customized by
the end users.  It was a bug, exactly for the reason you said above.

Before we talk about creating 'info' directory directly in "git
init" or anything done in this topic, we should fix the existing
bug, and the right fix is to use safe-create-leading-directories.

With that, it may become unnecessary to have this "create 'info' in
'init'".

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

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 20 2021, Junio C Hamano wrote:

> Derrick Stolee <stolee@gmail.com> writes:
>
>> What I _am_ saying is important is that if we are trying to write
>> a file to a known location and its parent directory doesn't exist,
>> then we should create it. Not doing so is a bug and should be
>> fixed, no matter how rare such a thing is to occur. As you've
>> shown, it is not required to have an info directory until we need
>> one (e.g. for sparse-checkout or an excludes file).
>>
>> If you're not planning to add that to this series, then I'll add it
>> to my list. I do think it would fit well into this one, though.
>
> Historically, "git init" relied on the templates to create necessary
> directories, and the subcommands in turn learned to depend on the
> presence of these directories.
>
> At the same time we allowed that the templates can be customized by
> the end users.  It was a bug, exactly for the reason you said above.
>
> Before we talk about creating 'info' directory directly in "git
> init" or anything done in this topic, we should fix the existing
> bug, and the right fix is to use safe-create-leading-directories.
>
> With that, it may become unnecessary to have this "create 'info' in
> 'init'".

I don't see why we'd consider that as a worthwhile direction to go
in. The "git-init" documentation states:
    
    This command creates an empty Git repository - basically a `.git`
    directory with subdirectories for `objects`, `refs/heads`,
    `refs/tags`, and template files. 

I.e. we promise to create "objects", but not "objects/{info,pack}", even
though we've done so since f49fb35d0d5 (git-init-db: create "pack"
subdirectory under objects, 2005-06-27) and d57306c7945 (Create
objects/info/ directory in init-db., 2005-08-20).

Our test suite reveals our own assumptions, but it's also indicative of
assumptions others have made.

It's cheap to create .git/info unconditionally, and we create similar
empty subdirectories, so why not do it for .git/info? Why would we
needlessly break widely documented out-of-tree recipies like:

    some-user-excludes >.git/info/excludes

Which yes, rely on something you can't strictly rely on, but is true
enough most of the time that people do.

So I don't see what finding and fixing every instance of assuming
.git/info in the test suite buys us.

After doing that we'd be back to square one of having to decide if
exposing a mode that effectively did the same could be overly pedantic
in that case, or cheaply cater to out-of-tree code that made the same
assumption.

That's the question this topic is mainly trying to address. It's also
worthwhile to fix the in-tree dependency in sparse-checkout, but I don't
see why we'd insist that one can't be done without the other.

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> I don't see why we'd consider that as a worthwhile direction to go
> in. The "git-init" documentation states:
>     
>     This command creates an empty Git repository - basically a `.git`
>     directory with subdirectories for `objects`, `refs/heads`,
>     `refs/tags`, and template files. 
>
> I.e. we promise to create "objects", but not "objects/{info,pack}", even
> though we've done so since f49fb35d0d5 (git-init-db: create "pack"
> subdirectory under objects, 2005-06-27) and d57306c7945 (Create
> objects/info/ directory in init-db., 2005-08-20).

I view it as a documentation bug, though.

The only thing we need to promise is that (1) the directory prepared
by "git init" will be recognised as a repository by the current and
future versions of Git (and hopefully the existing ones, too), and
(2) versions of Git will not barf due to missing directory and stuff
due to end-user mischief around custom templates.  I do not think we
even need refs/heads/ directory and I do not see that, especially
with a presence with "basically" there, the sentence promises that
we will always create refs/tags/ directory.  For (1), only objects/,
refs/ and HEAD is necessary, as long as (2) is satisfied by lazy
creation of leading paths.

We would want to be able to cope with a repository that lost
.git/info directory due to loss of it in custom templates anyway.
Just like we create leading missing directories lazily for any
hierarchy under .git/, we should create .git/info on-demand.

Pre-creating .git/info might be nice, but becomes much less
important after it happens, and that is why I view it as a much
lower priority than what Derrick suggested.

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

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 20 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> I don't see why we'd consider that as a worthwhile direction to go
>> in. The "git-init" documentation states:
>>     
>>     This command creates an empty Git repository - basically a `.git`
>>     directory with subdirectories for `objects`, `refs/heads`,
>>     `refs/tags`, and template files. 
>>
>> I.e. we promise to create "objects", but not "objects/{info,pack}", even
>> though we've done so since f49fb35d0d5 (git-init-db: create "pack"
>> subdirectory under objects, 2005-06-27) and d57306c7945 (Create
>> objects/info/ directory in init-db., 2005-08-20).
>
> I view it as a documentation bug, though.
>
> The only thing we need to promise is that (1) the directory prepared
> by "git init" will be recognised as a repository by the current and
> future versions of Git (and hopefully the existing ones, too), and
> (2) versions of Git will not barf due to missing directory and stuff
> due to end-user mischief around custom templates.  I do not think we
> even need refs/heads/ directory and I do not see that, especially
> with a presence with "basically" there, the sentence promises that
> we will always create refs/tags/ directory.  For (1), only objects/,
> refs/ and HEAD is necessary, as long as (2) is satisfied by lazy
> creation of leading paths.

Do you think that we should stop creating objects/{pack,info} then,
provided that the few test failures that assume them are cleand up
(which for at least one of them is much easier than the top-level
"info")?

> We would want to be able to cope with a repository that lost
> .git/info directory due to loss of it in custom templates anyway.
> Just like we create leading missing directories lazily for any
> hierarchy under .git/, we should create .git/info on-demand.
>
> Pre-creating .git/info might be nice, but becomes much less
> important after it happens, and that is why I view it as a much
> lower priority than what Derrick suggested.

Yes, we agree that we should deal with it not being created, that we
don't is a bug in sparse-checkout.

That's still a separate question from whether it's worthwhile investment
of both our and user time to skip creating them and deal with the
resulting churn in the tests and needlessly break code in the wild.

It seems as though you're saying that any fixes or changes in this area
would be incomplete without moving us towards the most pedantic and
minimalist interpretation possible when doing a "git init", is that
right?

I.e. to end up with this .git from "git init --template=" (the same as
now plus rmdir .git/{objects,refs}/*):
    
    $ tree -a -p
    .
    └── [drwxr-xr-x]  .git
        ├── [-rw-r--r--]  config
        ├── [-rw-r--r--]  HEAD
        ├── [drwxr-xr-x]  objects
        └── [drwxr-xr-x]  refs
    
    3 directories, 2 files

I can work towards that with these patches, it just seems to me to be
needlessly creating potential issues for little or no benefit.

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> It seems as though you're saying that any fixes or changes in this area
> would be incomplete without moving us towards the most pedantic and
> minimalist interpretation possible when doing a "git init", is that
> right?

Not at all.  Belt and suspenders is a good way forward.  I am just
saying that on-demand creation is more important than the other one.

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

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 20 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> It seems as though you're saying that any fixes or changes in this area
>> would be incomplete without moving us towards the most pedantic and
>> minimalist interpretation possible when doing a "git init", is that
>> right?
>
> Not at all.  Belt and suspenders is a good way forward.  I am just
> saying that on-demand creation is more important than the other one.

So you do agree that we should create .git/info whatever the template
says (as this series does), or not?

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Mon, Dec 20 2021, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> It seems as though you're saying that any fixes or changes in this area
>>> would be incomplete without moving us towards the most pedantic and
>>> minimalist interpretation possible when doing a "git init", is that
>>> right?
>>
>> Not at all.  Belt and suspenders is a good way forward.  I am just
>> saying that on-demand creation is more important than the other one.
>
> So you do agree that we should create .git/info whatever the template
> says (as this series does), or not?

Probably "should" is too strong a word.  

I do not mind "mkdir -p" existing, but I think it is more important
to spend our engineering effort to make sure there are necessary
on-demand creation of leading paths.

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

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 20 2021, Derrick Stolee wrote:

> On 12/20/2021 11:13 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Mon, Dec 20 2021, Derrick Stolee wrote:
>> 
>>> On 12/12/2021 3:13 PM, Ævar Arnfjörð Bjarmason wrote:
>>>> 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.
>
> You continue after this with more motivations for adding 'init' 
> unconditionally, which I am not fighting.
>
> What I _am_ saying is important is that if we are trying to write
> a file to a known location and its parent directory doesn't exist,
> then we should create it. Not doing so is a bug and should be
> fixed, no matter how rare such a thing is to occur. As you've
> shown, it is not required to have an info directory until we need
> one (e.g. for sparse-checkout or an excludes file).
>
> If you're not planning to add that to this series, then I'll add it
> to my list. I do think it would fit well into this one, though.

Just so we'll avoid stepping on each other's toes, what's the status of
your plan/non-plan to work on that more isolated fix, perhaps you have
one that's unsubmitted?

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

[Derrick Stolee]

On 1/12/2022 7:42 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Dec 20 2021, Derrick Stolee wrote:
> 
>> On 12/20/2021 11:13 AM, Ævar Arnfjörð Bjarmason wrote:
>>>
>>> On Mon, Dec 20 2021, Derrick Stolee wrote:
>>>
>>>> On 12/12/2021 3:13 PM, Ævar Arnfjörð Bjarmason wrote:
>>>>> 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.
>>
>> You continue after this with more motivations for adding 'init' 
>> unconditionally, which I am not fighting.
>>
>> What I _am_ saying is important is that if we are trying to write
>> a file to a known location and its parent directory doesn't exist,
>> then we should create it. Not doing so is a bug and should be
>> fixed, no matter how rare such a thing is to occur. As you've
>> shown, it is not required to have an info directory until we need
>> one (e.g. for sparse-checkout or an excludes file).
>>
>> If you're not planning to add that to this series, then I'll add it
>> to my list. I do think it would fit well into this one, though.
> 
> Just so we'll avoid stepping on each other's toes, what's the status of
> your plan/non-plan to work on that more isolated fix, perhaps you have
> one that's unsubmitted?

I do not have one that is unsubmitted. I was hoping that you would
include it in a v2 to this series. I might have been quicker to
volunteer to create one had I not been sidelined for two weeks, but
right now I have a lot to catch up on so don't have the time.

Thanks,
-Stolee

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

[Ævar Arnfjörð Bjarmason]

On Tue, Jan 18 2022, Derrick Stolee wrote:

> On 1/12/2022 7:42 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Mon, Dec 20 2021, Derrick Stolee wrote:
>> 
>>> On 12/20/2021 11:13 AM, Ævar Arnfjörð Bjarmason wrote:
>>>>
>>>> On Mon, Dec 20 2021, Derrick Stolee wrote:
>>>>
>>>>> On 12/12/2021 3:13 PM, Ævar Arnfjörð Bjarmason wrote:
>>>>>> 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.
>>>
>>> You continue after this with more motivations for adding 'init' 
>>> unconditionally, which I am not fighting.
>>>
>>> What I _am_ saying is important is that if we are trying to write
>>> a file to a known location and its parent directory doesn't exist,
>>> then we should create it. Not doing so is a bug and should be
>>> fixed, no matter how rare such a thing is to occur. As you've
>>> shown, it is not required to have an info directory until we need
>>> one (e.g. for sparse-checkout or an excludes file).
>>>
>>> If you're not planning to add that to this series, then I'll add it
>>> to my list. I do think it would fit well into this one, though.
>> 
>> Just so we'll avoid stepping on each other's toes, what's the status of
>> your plan/non-plan to work on that more isolated fix, perhaps you have
>> one that's unsubmitted?
>
> I do not have one that is unsubmitted. I was hoping that you would
> include it in a v2 to this series. I might have been quicker to
> volunteer to create one had I not been sidelined for two weeks, but
> right now I have a lot to catch up on so don't have the time.

Good to hear that you're better (or back?, not 100% sure what
"sidelined" here means).

I'll try to get to re-rolling it with a fix for that sparse-checkout
issue, hopefully sooner than later. Just wanted to avoid potential
duplicate work.

Thanks.

[PATCH v2 0/7] tests: don't depend on "git init" using the template

[Ævar Arnfjörð Bjarmason]

This reduced-in-scope re-roll of [1] changes those tests that
implicitly relied on the existence of the default "git init" template
to no longer do so. This makes the intent of the tests more explicit,
e.g. in the case of the .git/branch directory.

Parts of the v1 were already split up, re-rolled and merged as
a5bf611cc4f (Merge branch 'ab/hook-tests-updates', 2022-03-30).

Future goals: I then have a local topic on top of this to add the
"--no-template" option to "git init", and to have the test suite run
in that mode by default, that's currently one of the blockers for
making the test run without the bin-wrappers[2], which as noted in [3]
leaves us with some blind spots in our tests.

But for now this is all relatively straightforward conversion of
existing tests to make them not rely on "git init" giving them a
template, which they opt-in to with a new TEST_CREATE_REPO_NO_TEMPLATE
variable.

Junio: There's a trivial conflict in one of the tests with "next",
i.e.:

	diff --cc t/t6101-rev-parse-parents.sh
	index 5f55ab98d35,a3a41c7a3e4..00000000000
	--- a/t/t6101-rev-parse-parents.sh
	+++ b/t/t6101-rev-parse-parents.sh
	@@@ -8,7 -8,7 +8,11 @@@ test_description='Test git rev-parse wi
	  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
	  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

	++<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< HEAD
	 +TEST_CREATE_REPO_NO_TEMPLATE=1
	++================================
	+ TEST_PASSES_SANITIZE_LEAK=true
	++>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> origin/seen

The resolution is to keep both sides, i.e. to end up with:

	TEST_CREATE_REPO_NO_TEMPLATE=1
	TEST_PASSES_SANITIZE_LEAK=true

1. https://lore.kernel.org/git/cover-00.13-00000000000-20211212T201308Z-avarab@gmail.com/
2. https://lore.kernel.org/git/220405.86k0c3lt2l.gmgdl@evledraar.gmail.com/
3. https://lore.kernel.org/git/211124.86sfvld4cl.gmgdl@evledraar.gmail.com/

Ævar Arnfjörð Bjarmason (7):
  t0008: don't rely on default ".git/info/exclude"
  tests: don't depend on template-created .git/branches
  tests: don't assume a .git/info for .git/info/grafts
  tests: don't assume a .git/info for .git/info/attributes
  tests: don't assume a .git/info for .git/info/refs
  tests: don't assume a .git/info for .git/info/exclude
  tests: don't assume a .git/info for .git/info/sparse-checkout

 t/annotate-tests.sh                    |  3 ++-
 t/lib-submodule-update.sh              |  7 +++++--
 t/t0003-attributes.sh                  |  5 ++++-
 t/t0008-ignores.sh                     | 12 +++++++-----
 t/t0028-working-tree-encoding.sh       |  2 ++
 t/t1011-read-tree-sparse-checkout.sh   |  2 ++
 t/t1090-sparse-checkout-scope.sh       |  5 ++++-
 t/t1301-shared-repo.sh                 |  3 ++-
 t/t2018-checkout-branch.sh             |  4 +++-
 t/t2400-worktree-add.sh                |  4 ++++
 t/t3426-rebase-submodule.sh            |  1 +
 t/t3507-cherry-pick-conflict.sh        |  2 ++
 t/t5000-tar-tree.sh                    |  5 ++++-
 t/t5001-archive-attr.sh                |  5 ++++-
 t/t5002-archive-attr-pattern.sh        |  5 ++++-
 t/t5003-archive-zip.sh                 |  5 ++++-
 t/t5318-commit-graph.sh                |  3 ++-
 t/t5505-remote.sh                      |  6 ++++--
 t/t5516-fetch-push.sh                  | 16 ++++++++++++++--
 t/t5550-http-fetch-dumb.sh             |  3 ++-
 t/t6001-rev-list-graft.sh              |  1 +
 t/t6101-rev-parse-parents.sh           |  2 ++
 t/t6435-merge-sparse.sh                |  2 ++
 t/t7063-status-untracked-cache.sh      |  3 ++-
 t/t7418-submodule-sparse-gitmodules.sh |  3 ++-
 t/t7814-grep-recurse-submodules.sh     |  8 ++++++--
 t/t8001-annotate.sh                    |  1 +
 t/t8002-blame.sh                       |  1 +
 t/t8012-blame-colors.sh                |  1 +
 t/test-lib.sh                          |  4 +++-
 30 files changed, 97 insertions(+), 27 deletions(-)

Range-diff against v1:
 1:  5526d3dc838 <  -:  ----------- t0001: fix gaps in "TEMPLATE DIRECTORY" coverage
 2:  ef2b67768cf <  -:  ----------- init: split out template population from create_default_files()
 3:  784b7947512 <  -:  ----------- init: unconditionally create the "info" directory
 4:  3d4ea5c5d30 =  1:  21927e21832 t0008: don't rely on default ".git/info/exclude"
 5:  6bbb39f13fc <  -:  ----------- init & clone: add a --no-template option
 6:  2f478f7ba4a <  -:  ----------- init & clone: add init.templateDir=[bool]
 7:  9402fb23b40 <  -:  ----------- test-lib: create test data with "git init --no-template" (almost)
 8:  0c9b953dd43 <  -:  ----------- tests: don't depend on template-created .git/branches
 9:  d97122f5fd8 <  -:  ----------- t5540: don't rely on "hook/post-update.sample"
10:  ca55471d134 <  -:  ----------- test-lib-functions: add and use a "write_hook" wrapper
11:  6f805f7ebac <  -:  ----------- tests: change "cat && chmod +x" to use "write_hook"
12:  2acbaa77f8d <  -:  ----------- tests: migrate miscellaneous "write_script" to "write_hooks"
13:  d021a5981a1 <  -:  ----------- tests: don't depend on template-created .git/hooks
 -:  ----------- >  2:  c4d0e6c684b tests: don't depend on template-created .git/branches
 -:  ----------- >  3:  967762b4670 tests: don't assume a .git/info for .git/info/grafts
 -:  ----------- >  4:  64292c20c84 tests: don't assume a .git/info for .git/info/attributes
 -:  ----------- >  5:  b9f667bf292 tests: don't assume a .git/info for .git/info/refs
 -:  ----------- >  6:  29da5a3d431 tests: don't assume a .git/info for .git/info/exclude
 -:  ----------- >  7:  0149c80f09c tests: don't assume a .git/info for .git/info/sparse-checkout
-- 
2.36.1.1119.g5a713b43bd4

[PATCH v2 1/7] t0008: don't rely on default ".git/info/exclude"

[Ævar Arnfjörð Bjarmason]

Change a test added in 368aa52952d (add git-check-ignore sub-command,
2013-01-06) to clobber .git/info/exclude rather than append to
it.

These tests would break if the "templates/info--exclude" file added in
d3af621b147 (Redo the templates generation and installation.,
2005-08-06) wasn't exactly 6 lines (of only comments).

Let's instead clobber the default .git/info/excludes file, and test
only our own expected content. This is not strictly needed for
anything in this series, but is a good cleanup while we're at it.

As discussed in the preceding commit a lot of things depend on the
"info" directory being created, but this was the only test that relied
on the specific content in the "templates/info--exclude" file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0008-ignores.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 5575dade8ee..c3655a0c53e 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -225,7 +225,7 @@ test_expect_success 'setup' '
 		!globaltwo
 		globalthree
 	EOF
-	cat <<-\EOF >>.git/info/exclude
+	cat <<-\EOF >.git/info/exclude
 		per-repo
 	EOF
 '
@@ -543,9 +543,9 @@ test_expect_success_multi 'submodule from subdirectory' '' '
 
 test_expect_success 'global ignore not yet enabled' '
 	expect_from_stdin <<-\EOF &&
-		.git/info/exclude:7:per-repo	per-repo
+		.git/info/exclude:1:per-repo	per-repo
 		a/.gitignore:2:*three	a/globalthree
-		.git/info/exclude:7:per-repo	a/per-repo
+		.git/info/exclude:1:per-repo	a/per-repo
 	EOF
 	test_check_ignore "-v globalone per-repo a/globalthree a/per-repo not-ignored a/globaltwo"
 '
@@ -566,10 +566,10 @@ test_expect_success 'global ignore with -v' '
 	enable_global_excludes &&
 	expect_from_stdin <<-EOF &&
 		$global_excludes:1:globalone	globalone
-		.git/info/exclude:7:per-repo	per-repo
+		.git/info/exclude:1:per-repo	per-repo
 		$global_excludes:3:globalthree	globalthree
 		a/.gitignore:2:*three	a/globalthree
-		.git/info/exclude:7:per-repo	a/per-repo
+		.git/info/exclude:1:per-repo	a/per-repo
 		$global_excludes:2:!globaltwo	globaltwo
 	EOF
 	test_check_ignore "-v globalone per-repo globalthree a/globalthree a/per-repo not-ignored globaltwo"
-- 
2.36.1.1119.g5a713b43bd4

[PATCH v2 2/7] tests: don't depend on template-created .git/branches

[Ævar Arnfjörð Bjarmason]

As noted in c8a58ac5a52 (Revert "Don't create the $GIT_DIR/branches
directory on init", 2009-10-31) there was an attempt long ago in
0cc5691a8b0 (Don't create the $GIT_DIR/branches directory on init,
2009-10-30) to get rid of the legacy "branches" directory.

We should probably get rid of its creation by removing the
"templates/branches--" file. But whatever our default behavior, our
tests should be tightened up to explicitly create the .git/branches
directory if they rely on our default templates, to make the
dependency on those templates clear.

So let's amend the two tests that would fail if .git/branches wasn't
created. To do this introduce a new "TEST_CREATE_REPO_NO_TEMPLATE"
variable, which we'll set before sourcing test-lib.sh, and change the
"git clone" and "git init" commands in the tests themselves to
explicitly pass "--template=".

This way they won't get a .git/branches in either their top-level
.git, or in the ones they create. We can then amend the tests that
rely on the ".git/branches" directory existing to create it
explicitly, and to remove it after its creation.

This new "TEST_CREATE_REPO_NO_TEMPLATE" variable is a less
heavy-handed version of the "NO_SET_GIT_TEMPLATE_DIR" variable. See
a94d305bf80 (t/t0001-init.sh: add test for 'init with init.templatedir
set', 2010-02-26) for its implementation.

Unlike "TEST_CREATE_REPO_NO_TEMPLATE", this new
"TEST_CREATE_REPO_NO_TEMPLATE" variable is narrowly scoped to what the
"git init" in test-lib.sh does, as opposed to the global effect of
"NO_SET_GIT_TEMPLATE_DIR" and the setting of "GIT_TEMPLATE_DIR" in
wrap-for-bin.sh.

I experimented with adding a new "GIT_WRAP_FOR_BIN_VIA_TEST_LIB"
variable set in test-lib.sh, which would cause wrap-for-bin.sh to not
set GIT_TEMPLATE_DIR, GITPERLLIB etc, as we set those in
test-lib.sh. I think that's a viable approach, but it would interact
e.g. with the appending feature of GITPERLLIB added in
8bade1e12e2 (wrap-for-bin: make bin-wrappers chainable, 2013-07-04).

Doing so would allow us to convert the tests in t0001-init.sh that now
use "NO_SET_GIT_TEMPLATE_DIR" to simply unset "GIT_TEMPLATE_DIR" in a
sub-shell before invoking "git init" or "git clone". I think that
approach is worth pursuing, but let's table it for now. Some future
wrap-for-bin.sh refactoring can try to address it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t5505-remote.sh     |  6 ++++--
 t/t5516-fetch-push.sh | 16 ++++++++++++++--
 t/test-lib.sh         |  4 +++-
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index fff14e13ed4..cfb47509e3f 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -957,11 +957,12 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
 '
 
 test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
-	git clone one six &&
+	git clone --template= one six &&
 	origin_url=$(pwd)/one &&
 	(
 		cd six &&
 		git remote rm origin &&
+		mkdir .git/branches &&
 		echo "$origin_url#main" >.git/branches/origin &&
 		git remote rename origin origin &&
 		test_path_is_missing .git/branches/origin &&
@@ -972,10 +973,11 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 '
 
 test_expect_success 'migrate a remote from named file in $GIT_DIR/branches (2)' '
-	git clone one seven &&
+	git clone --template= one seven &&
 	(
 		cd seven &&
 		git remote rm origin &&
+		mkdir .git/branches &&
 		echo "quux#foom" > .git/branches/origin &&
 		git remote rename origin origin &&
 		test_path_is_missing .git/branches/origin &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 4dfb080433e..5d3054eeb89 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -17,6 +17,7 @@ This test checks the following functionality:
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 D=$(pwd)
@@ -25,7 +26,8 @@ mk_empty () {
 	repo_name="$1"
 	test_when_finished "rm -rf \"$repo_name\"" &&
 	test_path_is_missing "$repo_name" &&
-	git init "$repo_name" &&
+	git init --template= "$repo_name" &&
+	mkdir "$repo_name"/.git/hooks &&
 	git -C "$repo_name" config receive.denyCurrentBranch warn
 }
 
@@ -77,7 +79,7 @@ mk_test_with_hooks() {
 
 mk_child() {
 	test_when_finished "rm -rf \"$2\"" &&
-	git clone "$1" "$2"
+	git clone --template= "$1" "$2"
 }
 
 check_push_result () {
@@ -916,6 +918,7 @@ test_expect_success 'fetch with branches' '
 	mk_empty testrepo &&
 	git branch second $the_first_commit &&
 	git checkout second &&
+	mkdir testrepo/.git/branches &&
 	echo ".." > testrepo/.git/branches/branch1 &&
 	(
 		cd testrepo &&
@@ -929,6 +932,7 @@ test_expect_success 'fetch with branches' '
 
 test_expect_success 'fetch with branches containing #' '
 	mk_empty testrepo &&
+	mkdir testrepo/.git/branches &&
 	echo "..#second" > testrepo/.git/branches/branch2 &&
 	(
 		cd testrepo &&
@@ -943,7 +947,11 @@ test_expect_success 'fetch with branches containing #' '
 test_expect_success 'push with branches' '
 	mk_empty testrepo &&
 	git checkout second &&
+
+	test_when_finished "rm -rf .git/branches" &&
+	mkdir .git/branches &&
 	echo "testrepo" > .git/branches/branch1 &&
+
 	git push branch1 &&
 	(
 		cd testrepo &&
@@ -955,7 +963,11 @@ test_expect_success 'push with branches' '
 
 test_expect_success 'push with branches containing #' '
 	mk_empty testrepo &&
+
+	test_when_finished "rm -rf .git/branches" &&
+	mkdir .git/branches &&
 	echo "testrepo#branch3" > .git/branches/branch2 &&
+
 	git push branch2 &&
 	(
 		cd testrepo &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8ba5ca15345..144f14cc142 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1529,7 +1529,9 @@ remove_trash_directory "$TRASH_DIRECTORY" || {
 remove_trash=t
 if test -z "$TEST_NO_CREATE_REPO"
 then
-	git init "$TRASH_DIRECTORY" >&3 2>&4 ||
+	git init \
+	    ${TEST_CREATE_REPO_NO_TEMPLATE:+--template=} \
+	    "$TRASH_DIRECTORY" >&3 2>&4 ||
 	error "cannot run git init"
 else
 	mkdir -p "$TRASH_DIRECTORY"
-- 
2.36.1.1119.g5a713b43bd4

[PATCH v2 3/7] tests: don't assume a .git/info for .git/info/grafts

[Ævar Arnfjörð Bjarmason]

Change those tests that assumed that a .git/info directory would be
created for them when writing .git/info/grafts to explicitly create
the directory.

Do this using the new "TEST_CREATE_REPO_NO_TEMPLATE" facility, and use
"mkdir" instead of "mkdir -p" to assert that we don't have the
.git/info already. An exception to this is the "with grafts" test in
"t6001-rev-list-graft.sh". There we're modifying our ".git" state in a
for-loop, in lieu of refactoring that more extensively let's use
"mkdir -p" there.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/annotate-tests.sh          | 3 ++-
 t/t2400-worktree-add.sh      | 4 ++++
 t/t5318-commit-graph.sh      | 3 ++-
 t/t6001-rev-list-graft.sh    | 1 +
 t/t6101-rev-parse-parents.sh | 2 ++
 t/t8001-annotate.sh          | 1 +
 t/t8002-blame.sh             | 1 +
 t/t8012-blame-colors.sh      | 1 +
 8 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index cc01d891504..f1b9a6ce4da 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -153,7 +153,7 @@ test_expect_success 'blame evil merge' '
 
 test_expect_success 'blame huge graft' '
 	test_when_finished "git checkout branch2" &&
-	test_when_finished "rm -f .git/info/grafts" &&
+	test_when_finished "rm -rf .git/info" &&
 	graft= &&
 	for i in 0 1 2
 	do
@@ -168,6 +168,7 @@ test_expect_success 'blame huge graft' '
 			graft="$graft$commit " || return 1
 		done
 	done &&
+	mkdir .git/info &&
 	printf "%s " $graft >.git/info/grafts &&
 	check_count -h 00 01 1 10 1
 '
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 2f564d533d0..f3242fef6b6 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -5,6 +5,7 @@ test_description='test git worktree add'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
@@ -229,6 +230,7 @@ test_expect_success 'checkout with grafts' '
 	SHA1=$(git rev-parse HEAD) &&
 	test_commit def &&
 	test_commit xyz &&
+	mkdir .git/info &&
 	echo "$(git rev-parse HEAD) $SHA1" >.git/info/grafts &&
 	cat >expected <<-\EOF &&
 	xyz
@@ -559,6 +561,8 @@ test_expect_success 'git worktree --no-guess-remote option overrides config' '
 '
 
 post_checkout_hook () {
+	test_when_finished "rm -rf .git/hooks" &&
+	mkdir .git/hooks &&
 	test_hook -C "$1" post-checkout <<-\EOF
 	{
 		echo $*
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index fbf0d64578c..be0b5641ffc 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -361,13 +361,14 @@ test_expect_success 'replace-objects invalidates commit-graph' '
 test_expect_success 'commit grafts invalidate commit-graph' '
 	cd "$TRASH_DIRECTORY" &&
 	test_when_finished rm -rf graft &&
-	git clone full graft &&
+	git clone --template= full graft &&
 	(
 		cd graft &&
 		git commit-graph write --reachable &&
 		test_path_is_file .git/objects/info/commit-graph &&
 		H1=$(git rev-parse --verify HEAD~1) &&
 		H3=$(git rev-parse --verify HEAD~3) &&
+		mkdir .git/info &&
 		echo "$H1 $H3" >.git/info/grafts &&
 		git -c core.commitGraph=false log >expect &&
 		git -c core.commitGraph=true log >actual &&
diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 7294147334a..16635ecc33e 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -99,6 +99,7 @@ do
 	"
 
 	test_expect_success 'with grafts' "
+		mkdir -p .git/info &&
 		echo '$B0 $A2' >.git/info/grafts &&
 		check $type $B2 -- $B2 $B1 $B0 $A2 $A1 $A0
 	"
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index c571fa51797..5f55ab98d35 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -8,6 +8,7 @@ test_description='Test git rev-parse with different parent options'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 test_cmp_rev_output () {
@@ -25,6 +26,7 @@ test_expect_success 'setup' '
 	git merge -m next --allow-unrelated-histories start2 &&
 	test_commit final &&
 
+	mkdir .git/info &&
 	test_seq 40 |
 	while read i
 	do
diff --git a/t/t8001-annotate.sh b/t/t8001-annotate.sh
index a536a621b24..d7167f55397 100755
--- a/t/t8001-annotate.sh
+++ b/t/t8001-annotate.sh
@@ -4,6 +4,7 @@ test_description='git annotate'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 PROG='git annotate'
diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
index ee4fdd8f18d..0147de304b4 100755
--- a/t/t8002-blame.sh
+++ b/t/t8002-blame.sh
@@ -4,6 +4,7 @@ test_description='git blame'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 PROG='git blame -c'
diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh
index 90c75dbb283..c3a5f6d01ff 100755
--- a/t/t8012-blame-colors.sh
+++ b/t/t8012-blame-colors.sh
@@ -4,6 +4,7 @@ test_description='colored git blame'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 PROG='git blame -c'
-- 
2.36.1.1119.g5a713b43bd4

[PATCH v2 4/7] tests: don't assume a .git/info for .git/info/attributes

[Ævar Arnfjörð Bjarmason]

Change those tests that assumed that a .git/info directory would be
created for them when writing .git/info/attributes to explicitly
create the directory by setting "TEST_CREATE_REPO_NO_TEMPLATE=1"
before sourcing test-lib.sh, and using the "--template=" argument to
"git clone".

The change here in here in t7814-grep-recurse-submodules.sh would
continue "succeeding" with only the "TEST_CREATE_REPO_NO_TEMPLATE=1"
part of this change. That's because those tests use
"test_expect_failure", so they'd "pass" without this change, as
"test_expect_failure" by design isn't discerning about what failure
conditions it'll accept.

But as we're fixing these sorts of issues across the test suite let's
fix this one too. This issue was spotted with a local merge with
another topic of mine[1], which introduces a stricter alternative to
"test_expect_failure".

1. https://lore.kernel.org/git/cover-0.7-00000000000-20220318T002951Z-avarab@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0003-attributes.sh              | 5 ++++-
 t/t0028-working-tree-encoding.sh   | 2 ++
 t/t5000-tar-tree.sh                | 5 ++++-
 t/t5001-archive-attr.sh            | 5 ++++-
 t/t5002-archive-attr-pattern.sh    | 5 ++++-
 t/t5003-archive-zip.sh             | 5 ++++-
 t/t7814-grep-recurse-submodules.sh | 8 ++++++--
 7 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 143f1005179..f7ee2f2ff0e 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -3,6 +3,7 @@
 test_description=gitattributes
 
 TEST_PASSES_SANITIZE_LEAK=true
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 attr_check_basic () {
@@ -284,7 +285,7 @@ test_expect_success 'using --git-dir and --work-tree' '
 '
 
 test_expect_success 'setup bare' '
-	git clone --bare . bare.git
+	git clone --template= --bare . bare.git
 '
 
 test_expect_success 'bare repository: check that .gitattribute is ignored' '
@@ -315,6 +316,7 @@ test_expect_success 'bare repository: check that --cached honors index' '
 test_expect_success 'bare repository: test info/attributes' '
 	(
 		cd bare.git &&
+		mkdir info &&
 		(
 			echo "f	test=f" &&
 			echo "a/i test=a/i"
@@ -360,6 +362,7 @@ test_expect_success SYMLINKS 'symlinks respected in core.attributesFile' '
 
 test_expect_success SYMLINKS 'symlinks respected in info/attributes' '
 	test_when_finished "rm .git/info/attributes" &&
+	mkdir .git/info &&
 	ln -s ../../attr .git/info/attributes &&
 	attr_check file set
 '
diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh
index 82905a2156f..7140960e1a3 100755
--- a/t/t0028-working-tree-encoding.sh
+++ b/t/t0028-working-tree-encoding.sh
@@ -5,6 +5,7 @@ test_description='working-tree-encoding conversion via gitattributes'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-encoding.sh"
 
@@ -69,6 +70,7 @@ test_expect_success 'check $GIT_DIR/info/attributes support' '
 	test_when_finished "rm -f test.utf32.git" &&
 	test_when_finished "git reset --hard HEAD" &&
 
+	mkdir .git/info &&
 	echo "*.utf32 text working-tree-encoding=utf-32" >.git/info/attributes &&
 	git add test.utf32 &&
 
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 7f8d2ab0a72..de192b52c47 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -24,6 +24,7 @@ commit id embedding:
 
 '
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 SUBSTFORMAT=%H%n
@@ -143,6 +144,7 @@ test_expect_success 'populate workdir' '
 test_expect_success \
     'add ignored file' \
     'echo ignore me >a/ignored &&
+     mkdir .git/info &&
      echo ignored export-ignore >.git/info/attributes'
 
 test_expect_success 'add files to repository' '
@@ -157,7 +159,8 @@ test_expect_success 'setup export-subst' '
 '
 
 test_expect_success 'create bare clone' '
-	git clone --bare . bare.git &&
+	git clone --template= --bare . bare.git &&
+	mkdir bare.git/info &&
 	cp .git/info/attributes bare.git/info/attributes
 '
 
diff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh
index 712ae522994..2f6eef5e372 100755
--- a/t/t5001-archive-attr.sh
+++ b/t/t5001-archive-attr.sh
@@ -2,6 +2,7 @@
 
 test_description='git archive attribute tests'
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 SUBSTFORMAT='%H (%h)%n'
@@ -20,6 +21,7 @@ extract_tar_to_dir () {
 
 test_expect_success 'setup' '
 	echo ignored >ignored &&
+	mkdir .git/info &&
 	echo ignored export-ignore >>.git/info/attributes &&
 	git add ignored &&
 
@@ -46,7 +48,8 @@ test_expect_success 'setup' '
 
 	git commit -m. &&
 
-	git clone --bare . bare &&
+	git clone --template= --bare . bare &&
+	mkdir bare/info &&
 	cp .git/info/attributes bare/info/attributes
 '
 
diff --git a/t/t5002-archive-attr-pattern.sh b/t/t5002-archive-attr-pattern.sh
index a66b5ba27e8..78ab75f1bc2 100755
--- a/t/t5002-archive-attr-pattern.sh
+++ b/t/t5002-archive-attr-pattern.sh
@@ -3,6 +3,7 @@
 test_description='git archive attribute pattern tests'
 
 TEST_PASSES_SANITIZE_LEAK=true
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 test_expect_exists() {
@@ -15,6 +16,7 @@ test_expect_missing() {
 
 test_expect_success 'setup' '
 	echo ignored >ignored &&
+	mkdir .git/info &&
 	echo ignored export-ignore >>.git/info/attributes &&
 	git add ignored &&
 
@@ -54,7 +56,8 @@ test_expect_success 'setup' '
 
 	git commit -m. &&
 
-	git clone --bare . bare &&
+	git clone --template= --bare . bare &&
+	mkdir bare/info &&
 	cp .git/info/attributes bare/info/attributes
 '
 
diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
index d726964307c..a7bbfcb7f75 100755
--- a/t/t5003-archive-zip.sh
+++ b/t/t5003-archive-zip.sh
@@ -2,6 +2,7 @@
 
 test_description='git archive --format=zip test'
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 SUBSTFORMAT=%H%n
@@ -121,6 +122,7 @@ test_expect_success 'prepare file list' '
 test_expect_success \
     'add ignored file' \
     'echo ignore me >a/ignored &&
+     mkdir .git/info &&
      echo ignored export-ignore >.git/info/attributes'
 
 test_expect_success 'add files to repository' '
@@ -139,7 +141,8 @@ test_expect_success 'setup export-subst and diff attributes' '
 '
 
 test_expect_success 'create bare clone' '
-	git clone --bare . bare.git &&
+	git clone --template= --bare . bare.git &&
+	mkdir bare.git/info &&
 	cp .git/info/attributes bare.git/info/attributes &&
 	# Recreate our changes to .git/config rather than just copying it, as
 	# we do not want to clobber core.bare or other settings.
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index a4476dc4922..3ad80526c4c 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -6,6 +6,7 @@ This test verifies the recurse-submodules feature correctly greps across
 submodules.
 '
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1
@@ -471,8 +472,10 @@ test_expect_failure 'grep --textconv: superproject .gitattributes (from index) d
 test_expect_failure 'grep --textconv: superproject .git/info/attributes does not affect submodules' '
 	reset_and_clean &&
 	test_config_global diff.d2x.textconv "sed -e \"s/d/x/\"" &&
-	super_attr="$(git rev-parse --git-path info/attributes)" &&
+	super_info="$(git rev-parse --git-path info)" &&
+	super_attr="$super_info/attributes" &&
 	test_when_finished "rm -f \"$super_attr\"" &&
+	mkdir "$super_info" &&
 	echo "a diff=d2x" >"$super_attr" &&
 
 	cat >expect <<-\EOF &&
@@ -516,7 +519,8 @@ test_expect_failure 'grep --textconv correctly reads submodule .git/info/attribu
 	reset_and_clean &&
 	test_config_global diff.d2x.textconv "sed -e \"s/d/x/\"" &&
 
-	submodule_attr="$(git -C submodule rev-parse --path-format=absolute --git-path info/attributes)" &&
+	submodule_info="$(git -C submodule rev-parse --path-format=absolute --git-path info)" &&
+	submodule_attr="$submodule_info/attributes" &&
 	test_when_finished "rm -f \"$submodule_attr\"" &&
 	echo "a diff=d2x" >"$submodule_attr" &&
 
-- 
2.36.1.1119.g5a713b43bd4

[PATCH v2 5/7] tests: don't assume a .git/info for .git/info/refs

[Ævar Arnfjörð Bjarmason]

Change those tests that assumed that a .git/info directory would be
created for them when writing .git/info/refs to explicitly create the
directory by using the "--template=" argument to "git init".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t1301-shared-repo.sh     | 3 ++-
 t/t5550-http-fetch-dumb.sh | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 84bf1970d8b..93a2f91f8a5 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -48,7 +48,7 @@ done
 test_expect_success 'shared=all' '
 	mkdir sub &&
 	cd sub &&
-	git init --shared=all &&
+	git init --template= --shared=all &&
 	test 2 = $(git config core.sharedrepository)
 '
 
@@ -57,6 +57,7 @@ test_expect_success POSIXPERM 'update-server-info honors core.sharedRepository'
 	git add a1 &&
 	test_tick &&
 	git commit -m a1 &&
+	mkdir .git/info &&
 	umask 0277 &&
 	git update-server-info &&
 	actual="$(ls -l .git/info/refs)" &&
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index f0d9cd584d3..3ca621f33b5 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -422,7 +422,8 @@ test_expect_success 'set up evil alternates scheme' '
 	sha1=$(git -C "$victim" rev-parse HEAD) &&
 
 	evil=$HTTPD_DOCUMENT_ROOT_PATH/evil.git &&
-	git init --bare "$evil" &&
+	git init --template= --bare "$evil" &&
+	mkdir "$evil/info" &&
 	# do this by hand to avoid object existence check
 	printf "%s\\t%s\\n" $sha1 refs/heads/main >"$evil/info/refs"
 '
-- 
2.36.1.1119.g5a713b43bd4

[PATCH v2 6/7] tests: don't assume a .git/info for .git/info/exclude

[Ævar Arnfjörð Bjarmason]

Change those tests that assumed that a .git/info directory would be
created for them when writing .git/info/exclude to explicitly create
the directory by setting "TEST_CREATE_REPO_NO_TEMPLATE=1" before
sourcing test-lib.sh, and using the "--template=" argument to "git
clone" and "git init".

In the case of ".git/modules/sub1/info" we deviate from the
established pattern in this and preceding commits of passing a
"--template=" and doing a "mkdir .git/info".

In that case "git checkout" will run the "submodule--helper clone",
and both e.g. "git submodule update --init" and "git checkout" do not
have a way to pass down options to the eventual "git init" or "git
clone". Let's instead assume that the submodule was populated with our
default templates, remove them, and then run the "mkdir".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/lib-submodule-update.sh         | 7 +++++--
 t/t0008-ignores.sh                | 2 ++
 t/t3426-rebase-submodule.sh       | 1 +
 t/t7063-status-untracked-cache.sh | 3 ++-
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index f7c7df0ca42..03e0abbdb83 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -207,7 +207,7 @@ prolog () {
 # should be updated to an existing commit.
 reset_work_tree_to () {
 	rm -rf submodule_update &&
-	git clone submodule_update_repo submodule_update &&
+	git clone --template= submodule_update_repo submodule_update &&
 	(
 		cd submodule_update &&
 		rm -rf sub1 &&
@@ -902,13 +902,14 @@ test_submodule_switch_recursing_with_args () {
 	'
 	# ... but an ignored file is fine.
 	test_expect_$RESULTOI "$command: added submodule removes an untracked ignored file" '
-		test_when_finished "rm submodule_update/.git/info/exclude" &&
+		test_when_finished "rm -rf submodule_update/.git/info" &&
 		prolog &&
 		reset_work_tree_to_interested no_submodule &&
 		(
 			cd submodule_update &&
 			git branch -t add_sub1 origin/add_sub1 &&
 			: >sub1 &&
+			mkdir .git/info &&
 			echo sub1 >.git/info/exclude &&
 			$command add_sub1 &&
 			test_superproject_content origin/add_sub1 &&
@@ -951,7 +952,9 @@ test_submodule_switch_recursing_with_args () {
 		reset_work_tree_to_interested add_sub1 &&
 		(
 			cd submodule_update &&
+			rm -rf .git/modules/sub1/info &&
 			git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+			mkdir .git/modules/sub1/info &&
 			echo ignored >.git/modules/sub1/info/exclude &&
 			: >sub1/ignored &&
 			$command replace_sub1_with_file &&
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index c3655a0c53e..c70d11bc914 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -3,6 +3,7 @@
 test_description=check-ignore
 
 TEST_PASSES_SANITIZE_LEAK=true
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 init_vars () {
@@ -225,6 +226,7 @@ test_expect_success 'setup' '
 		!globaltwo
 		globalthree
 	EOF
+	mkdir .git/info &&
 	cat <<-\EOF >.git/info/exclude
 		per-repo
 	EOF
diff --git a/t/t3426-rebase-submodule.sh b/t/t3426-rebase-submodule.sh
index 0ad3a07bf47..7a9f1127a4b 100755
--- a/t/t3426-rebase-submodule.sh
+++ b/t/t3426-rebase-submodule.sh
@@ -35,6 +35,7 @@ git_rebase_interactive () {
 	ls -1pR * >>actual &&
 	test_cmp expect actual &&
 	set_fake_editor &&
+	mkdir .git/info &&
 	echo "fake-editor.sh" >.git/info/exclude &&
 	may_only_be_test_must_fail "$2" &&
 	$2 git rebase -i "$1"
diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index 9936cc329ec..f5050b75b29 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -86,7 +86,7 @@ test_expect_success 'core.untrackedCache is unset' '
 '
 
 test_expect_success 'setup' '
-	git init worktree &&
+	git init --template= worktree &&
 	cd worktree &&
 	mkdir done dtwo dthree &&
 	touch one two three done/one dtwo/two dthree/three &&
@@ -94,6 +94,7 @@ test_expect_success 'setup' '
 	test-tool chmtime =-300 done dtwo dthree &&
 	test-tool chmtime =-300 . &&
 	git add one two done/one &&
+	mkdir .git/info &&
 	: >.git/info/exclude &&
 	git update-index --untracked-cache &&
 	test_oid_cache <<-EOF
-- 
2.36.1.1119.g5a713b43bd4

[PATCH v2 7/7] tests: don't assume a .git/info for .git/info/sparse-checkout

[Ævar Arnfjörð Bjarmason]

Change those tests that assumed that a .git/info directory would be
created for them when writing .git/info/sparse-checkout to explicitly
create the directory by setting "TEST_CREATE_REPO_NO_TEMPLATE=1"
before sourcing test-lib.sh, and using the "--template=" argument to
"git clone".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t1011-read-tree-sparse-checkout.sh   | 2 ++
 t/t1090-sparse-checkout-scope.sh       | 5 ++++-
 t/t2018-checkout-branch.sh             | 4 +++-
 t/t3507-cherry-pick-conflict.sh        | 2 ++
 t/t6435-merge-sparse.sh                | 2 ++
 t/t7418-submodule-sparse-gitmodules.sh | 3 ++-
 6 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index 63a553d7b32..742f0fa909f 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -11,6 +11,7 @@ test_description='sparse checkout tests
   A	init.t
 '
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-read-tree.sh
 
@@ -53,6 +54,7 @@ test_expect_success 'read-tree without .git/info/sparse-checkout' '
 '
 
 test_expect_success 'read-tree with .git/info/sparse-checkout but disabled' '
+	mkdir .git/info &&
 	echo >.git/info/sparse-checkout &&
 	read_tree_u_must_succeed -m -u HEAD &&
 	git ls-files -t >result &&
diff --git a/t/t1090-sparse-checkout-scope.sh b/t/t1090-sparse-checkout-scope.sh
index d1833c0f31b..3a14218b245 100755
--- a/t/t1090-sparse-checkout-scope.sh
+++ b/t/t1090-sparse-checkout-scope.sh
@@ -5,6 +5,7 @@ test_description='sparse checkout scope tests'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 test_expect_success 'setup' '
@@ -25,6 +26,7 @@ test_expect_success 'create feature branch' '
 
 test_expect_success 'perform sparse checkout of main' '
 	git config --local --bool core.sparsecheckout true &&
+	mkdir .git/info &&
 	echo "!/*" >.git/info/sparse-checkout &&
 	echo "/a" >>.git/info/sparse-checkout &&
 	echo "/c" >>.git/info/sparse-checkout &&
@@ -73,7 +75,7 @@ test_expect_success 'skip-worktree on files outside sparse patterns' '
 
 test_expect_success 'in partial clone, sparse checkout only fetches needed blobs' '
 	test_create_repo server &&
-	git clone "file://$(pwd)/server" client &&
+	git clone --template= "file://$(pwd)/server" client &&
 
 	test_config -C server uploadpack.allowfilter 1 &&
 	test_config -C server uploadpack.allowanysha1inwant 1 &&
@@ -85,6 +87,7 @@ test_expect_success 'in partial clone, sparse checkout only fetches needed blobs
 	git -C server commit -m message &&
 
 	test_config -C client core.sparsecheckout 1 &&
+	mkdir client/.git/info &&
 	echo "!/*" >client/.git/info/sparse-checkout &&
 	echo "/a" >>client/.git/info/sparse-checkout &&
 	git -C client fetch --filter=blob:none origin &&
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 52e51b0726f..771c3c3c50e 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -2,6 +2,7 @@
 
 test_description='checkout'
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 # Arguments: [!] <branch> <oid> [<checkout options>]
@@ -257,11 +258,12 @@ test_expect_success 'checkout -b to a new branch preserves mergeable changes des
 		git checkout branch1-scratch &&
 		test_might_fail git branch -D branch3 &&
 		git config core.sparseCheckout false &&
-		rm .git/info/sparse-checkout" &&
+		rm -rf .git/info" &&
 
 	test_commit file2 &&
 
 	echo stuff >>file1 &&
+	mkdir .git/info &&
 	echo file2 >.git/info/sparse-checkout &&
 	git config core.sparseCheckout true &&
 
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 979e843c65a..f32799e0463 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -12,6 +12,7 @@ test_description='test cherry-pick and revert with conflicts
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 pristine_detach () {
@@ -558,6 +559,7 @@ test_expect_success 'cherry-pick preserves sparse-checkout' '
 		echo \"/*\" >.git/info/sparse-checkout
 		git read-tree --reset -u HEAD
 		rm .git/info/sparse-checkout" &&
+	mkdir .git/info &&
 	echo /unrelated >.git/info/sparse-checkout &&
 	git read-tree --reset -u HEAD &&
 	test_must_fail git cherry-pick -Xours picked>actual &&
diff --git a/t/t6435-merge-sparse.sh b/t/t6435-merge-sparse.sh
index 74562e12356..fde4aa3cd1a 100755
--- a/t/t6435-merge-sparse.sh
+++ b/t/t6435-merge-sparse.sh
@@ -2,6 +2,7 @@
 
 test_description='merge with sparse files'
 
+TEST_CREATE_REPO_NO_TEMPLATE=1
 . ./test-lib.sh
 
 # test_file $filename $content
@@ -26,6 +27,7 @@ test_expect_success 'setup' '
 	git rm modify_delete &&
 	test_commit_this ours &&
 	git config core.sparseCheckout true &&
+	mkdir .git/info &&
 	echo "/checked-out" >.git/info/sparse-checkout &&
 	git reset --hard &&
 	test_must_fail git merge theirs
diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh
index f87e524d6d4..57d7ab3eced 100755
--- a/t/t7418-submodule-sparse-gitmodules.sh
+++ b/t/t7418-submodule-sparse-gitmodules.sh
@@ -31,8 +31,9 @@ test_expect_success 'sparse checkout setup which hides .gitmodules' '
 		test_tick &&
 		git commit -m "Add submodule"
 	) &&
-	git clone upstream super &&
+	git clone --template= upstream super &&
 	(cd super &&
+		mkdir .git/info &&
 		cat >.git/info/sparse-checkout <<-\EOF &&
 		/*
 		!/.gitmodules
-- 
2.36.1.1119.g5a713b43bd4

Re: [PATCH v2 0/7] tests: don't depend on "git init" using the template

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> This reduced-in-scope re-roll of [1] changes those tests that
> implicitly relied on the existence of the default "git init" template
> to no longer do so. This makes the intent of the tests more explicit,
> e.g. in the case of the .git/branch directory.
>
> Parts of the v1 were already split up, re-rolled and merged as
> a5bf611cc4f (Merge branch 'ab/hook-tests-updates', 2022-03-30).
>
> Future goals: I then have a local topic on top of this to add the
> "--no-template" option to "git init", and to have the test suite run
> in that mode by default, that's currently one of the blockers for
> making the test run without the bin-wrappers[2], which as noted in [3]
> leaves us with some blind spots in our tests.
>
> But for now this is all relatively straightforward conversion of
> existing tests to make them not rely on "git init" giving them a
> template, which they opt-in to with a new TEST_CREATE_REPO_NO_TEMPLATE
> variable.
>
> Junio: There's a trivial conflict in one of the tests with "next",
> i.e.:

Heh.  That is good to know, but I am not sure why we need to run
without template to begin with, so "why this much churn?" is my
first reaction.  Without the churn in the first place, we do not
have to worry about unnecessary conflicts ;-).

Thanks.

Re: [PATCH v2 0/7] tests: don't depend on "git init" using the template

[Ævar Arnfjörð Bjarmason]

On Fri, Jun 03 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> This reduced-in-scope re-roll of [1] changes those tests that
>> implicitly relied on the existence of the default "git init" template
>> to no longer do so. This makes the intent of the tests more explicit,
>> e.g. in the case of the .git/branch directory.
>>
>> Parts of the v1 were already split up, re-rolled and merged as
>> a5bf611cc4f (Merge branch 'ab/hook-tests-updates', 2022-03-30).
>>
>> Future goals: I then have a local topic on top of this to add the
>> "--no-template" option to "git init", and to have the test suite run
>> in that mode by default, that's currently one of the blockers for
>> making the test run without the bin-wrappers[2], which as noted in [3]
>> leaves us with some blind spots in our tests.
>>
>> But for now this is all relatively straightforward conversion of
>> existing tests to make them not rely on "git init" giving them a
>> template, which they opt-in to with a new TEST_CREATE_REPO_NO_TEMPLATE
>> variable.
>>
>> Junio: There's a trivial conflict in one of the tests with "next",
>> i.e.:
>
> Heh.  That is good to know, but I am not sure why we need to run
> without template to begin with, so "why this much churn?" is my
> first reaction.  Without the churn in the first place, we do not
> have to worry about unnecessary conflicts ;-).

A few reasons:

 * We currently lack test coverage due to the bin-wrappers, as noted in
   the "remove cwd" topic, i.e. we couldn't easily test our own $PWD
   behavior because we run tests through a shell.

 * It's also much slower, as detailed in the linked thread(s).

 * When I do a full test run I get ~790MB of accumulated trash (if I
   have the tests not remove "trash directory"*), out of that ~168MB or
   more than 20% is just 'hooks' directories.

   Particularly on boxes with slower I/O (including CI) that really adds
   up.

 * Since the original submission we've fixed the bug I found in the
   sparse-checkout code where it had a hard reliance on our default
   templates, which "init" and "clone" allow you to customize.

   So having our tests provide you these by default leaves the door open
   to other such bugs, better to not rely on that at all.

   This series doesn't get us there, i.e. the default is still to
   provide them, but with these changes it'll be trivial to flip that
   switch in the follow-up series.

But yeah, this doesn't give us much more than churn *right now*, but I
think it's worth having these because the end goals above make sense,
and if I'm going to include the eventual benefits in one series that'll
be a much larger topic that'll be harder to review. I'd much prefer to
split up some trivial "prep" work first.

Thanks.

Re: [PATCH v2 0/7] tests: don't depend on "git init" using the template

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>  * Since the original submission we've fixed the bug I found in the
>    sparse-checkout code where it had a hard reliance on our default
>    templates, which "init" and "clone" allow you to customize.

I actually would not call that a bug.  At least, it is not an
interesting "bug".

If your custom template lacks .git/info directory, some things may
not be created correctly, and we could work it around by "mkdir -p".

But if your template creates a file there, we cannot get you unstuck
without removing it before "mkdir -p", but we probably do not want
to lose information that we do not even understand.

If your customization of your template breaks your repository in
certain ways, you can keep both halves.  I do not think we should
really cater to such "customize to break" use case.

>    This series doesn't get us there, i.e. the default is still to
>    provide them, but with these changes it'll be trivial to flip that
>    switch in the follow-up series.

So, I do not buy that goal in the first place.  Why not stop at
saying "don't do stupid things to your templates to break your
repository" and move on?  Don't we have bigger fish to fry?

Thanks.