All of lore.kernel.org
 help / color / mirror / Atom feed
From: "John Cai via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: John Cai <johncai86@gmail.com>, John Cai <johncai86@gmail.com>
Subject: [PATCH] attr: attr.allowInvalidSource config to allow invalid revision
Date: Wed, 20 Sep 2023 14:00:30 +0000	[thread overview]
Message-ID: <pull.1577.git.git.1695218431033.gitgitgadget@gmail.com> (raw)

From: John Cai <johncai86@gmail.com>

44451a2e5e (attr: teach "--attr-source=<tree>" global option to "git",
2023-05-06) provided the ability to pass in a treeish as the attr
source. When a revision does not resolve to a valid tree is passed, Git
will die. GitLab keeps bare repositories and always reads attributes
from the default branch, so we pass in HEAD to --attr-source.

With empty repositories however, HEAD does not point to a valid treeish,
causing Git to die. This means we would need to check for a valid
treeish each time. To avoid this, let's add a configuration that allows
Git to simply ignore --attr-source if it does not resolve to a valid
tree.

Signed-off-by: John Cai <johncai86@gmail.com>
---
    attr: attr.allowInvalidSource config to allow empty revision
    
    44451a2e5e (attr: teach "--attr-source=" global option to "git",
    2023-05-06) provided the ability to pass in a treeish as the attr
    source. When a revision does not resolve to a valid tree is passed, Git
    will die. GitLab keeps bare repositories and always reads attributes
    from the default branch, so we pass in HEAD to --attr-source.
    
    With empty repositories however, HEAD does not point to a valid treeish,
    causing Git to die. This means we would need to check for a valid
    treeish each time. To avoid this, let's add a configuration that allows
    Git to simply ignore --attr-source if it does not resolve to a valid
    tree.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1577%2Fjohn-cai%2Fjc%2Fconfig-attr-invalid-source-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1577/john-cai/jc/config-attr-invalid-source-v1
Pull-Request: https://github.com/git/git/pull/1577

 Documentation/config.txt      |  2 ++
 Documentation/config/attr.txt |  6 ++++++
 attr.c                        | 11 +++++++++--
 t/t0003-attributes.sh         | 36 +++++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/config/attr.txt

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 229b63a454c..b1891c2b5af 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -371,6 +371,8 @@ other popular tools, and describe them in your documentation.
 
 include::config/advice.txt[]
 
+include::config/attr.txt[]
+
 include::config/core.txt[]
 
 include::config/add.txt[]
diff --git a/Documentation/config/attr.txt b/Documentation/config/attr.txt
new file mode 100644
index 00000000000..2218f0c982a
--- /dev/null
+++ b/Documentation/config/attr.txt
@@ -0,0 +1,6 @@
+attr.allowInvalidSource::
+	If `--attr-source` cannot resolve to a valid tree object, ignore
+	`--attr-source` instead of erroring out, and fall back to looking for
+	attributes in the default locations. Useful when passing `HEAD` into
+	`attr-source` since it allows `HEAD` to point to an unborn branch in
+	cases like an empty repository.
diff --git a/attr.c b/attr.c
index 71c84fbcf86..854a3720e3f 100644
--- a/attr.c
+++ b/attr.c
@@ -1208,8 +1208,15 @@ static void compute_default_attr_source(struct object_id *attr_source)
 	if (!default_attr_source_tree_object_name || !is_null_oid(attr_source))
 		return;
 
-	if (repo_get_oid_treeish(the_repository, default_attr_source_tree_object_name, attr_source))
-		die(_("bad --attr-source or GIT_ATTR_SOURCE"));
+
+	if (repo_get_oid_treeish(the_repository, default_attr_source_tree_object_name, attr_source)) {
+		int allow_invalid_attr_source = 0;
+
+		git_config_get_bool("attr.allowinvalidsource", &allow_invalid_attr_source);
+
+		if (!allow_invalid_attr_source)
+			die(_("bad --attr-source or GIT_ATTR_SOURCE"));
+	}
 }
 
 static struct object_id *default_attr_source(void)
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 26e082f05b4..3272237ee2b 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -342,6 +342,42 @@ test_expect_success 'bare repository: check that .gitattribute is ignored' '
 	)
 '
 
+bad_attr_source_err="fatal: bad --attr-source or GIT_ATTR_SOURCE"
+
+test_expect_success 'attr.allowInvalidSource when HEAD is unborn' '
+	test_when_finished rm -rf empty &&
+	echo $bad_attr_source_err >expect_err &&
+	echo "f/path: test: unspecified" >expect &&
+	git init empty &&
+	test_must_fail git -C empty --attr-source=HEAD check-attr test -- f/path 2>err &&
+	test_cmp expect_err err &&
+	git -C empty -c attr.allowInvalidSource=true --attr-source=HEAD check-attr test -- f/path >actual 2>err &&
+	test_must_be_empty err &&
+	test_cmp expect actual
+'
+
+test_expect_success 'attr.allowInvalidSource when --attr-source points to non-existing ref' '
+	test_when_finished rm -rf empty &&
+	echo $bad_attr_source_err >expect_err &&
+	echo "f/path: test: unspecified" >expect &&
+	git init empty &&
+	test_must_fail git -C empty --attr-source=refs/does/not/exist check-attr test -- f/path 2>err &&
+	test_cmp expect_err err &&
+	git -C empty -c attr.allowInvalidSource=true --attr-source=refs/does/not/exist check-attr test -- f/path >actual 2>err &&
+	test_must_be_empty err &&
+	test_cmp expect actual
+'
+
+test_expect_success 'bad attr source defaults to reading .gitattributes file' '
+	test_when_finished rm -rf empty &&
+	git init empty &&
+	echo "f/path test=val" >empty/.gitattributes &&
+	echo "f/path: test: val" >expect &&
+	git -C empty -c attr.allowInvalidSource=true --attr-source=HEAD check-attr test -- f/path >actual 2>err &&
+	test_must_be_empty err &&
+	test_cmp expect actual
+'
+
 test_expect_success 'bare repository: with --source' '
 	(
 		cd bare.git &&

base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3
-- 
gitgitgadget

             reply	other threads:[~2023-09-20 14:00 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-20 14:00 John Cai via GitGitGadget [this message]
2023-09-20 16:06 ` [PATCH] attr: attr.allowInvalidSource config to allow invalid revision Junio C Hamano
2023-09-21  4:15   ` Jeff King
2023-09-21  8:52     ` Junio C Hamano
2023-09-21 21:40       ` Jeff King
2023-09-26 18:27         ` John Cai
2023-09-26 18:30       ` John Cai
2023-09-26 18:23     ` John Cai
2023-10-04 18:18 ` [PATCH v2 0/2] attr: add attr.tree and attr.allowInvalidSource configs John Cai via GitGitGadget
2023-10-04 18:18   ` [PATCH v2 1/2] attr: add attr.tree for setting the treeish to read attributes from John Cai via GitGitGadget
2023-10-04 19:58     ` Junio C Hamano
2023-10-05 17:07       ` Jeff King
2023-10-05 19:46         ` John Cai
2023-10-04 23:45     ` Junio C Hamano
2023-10-06 17:20       ` Jonathan Tan
2023-10-04 18:18   ` [PATCH v2 2/2] attr: add attr.allowInvalidSource config to allow invalid revision John Cai via GitGitGadget
2023-10-10 19:49   ` [PATCH v3 0/2] attr: add attr.tree config John Cai via GitGitGadget
2023-10-10 19:49     ` [PATCH v3 1/2] attr: read attributes from HEAD when bare repo John Cai via GitGitGadget
2023-10-10 19:58       ` Eric Sunshine
2023-10-10 19:49     ` [PATCH v3 2/2] attr: add attr.tree for setting the treeish to read attributes from John Cai via GitGitGadget
2023-10-10 22:14       ` Junio C Hamano
2023-10-11  2:19         ` John Cai
2023-10-11 17:13     ` [PATCH v4 0/2] attr: add attr.tree config John Cai via GitGitGadget
2023-10-11 17:13       ` [PATCH v4 1/2] attr: read attributes from HEAD when bare repo John Cai via GitGitGadget
2023-10-11 17:13       ` [PATCH v4 2/2] attr: add attr.tree for setting the treeish to read attributes from John Cai via GitGitGadget
2023-10-11 22:09       ` [PATCH v4 0/2] attr: add attr.tree config Junio C Hamano
2023-10-13 15:30         ` John Cai
2023-10-13 17:39       ` [PATCH v5 " John Cai via GitGitGadget
2023-10-13 17:39         ` [PATCH v5 1/2] attr: read attributes from HEAD when bare repo John Cai via GitGitGadget
2023-10-13 17:39         ` [PATCH v5 2/2] attr: add attr.tree for setting the treeish to read attributes from John Cai via GitGitGadget
2023-10-13 18:52         ` [PATCH v5 0/2] attr: add attr.tree config Junio C Hamano
2023-10-13 20:31           ` Junio C Hamano
2023-10-13 20:47             ` Junio C Hamano
2023-10-19 15:43               ` John Cai

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=pull.1577.git.git.1695218431033.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johncai86@gmail.com \
    /path/to/YOUR_REPLY

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

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