git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 11/35] attr: (re)introduce git_check_attr() and struct git_attr_check

From
Stefan Beller <sbeller@google.com>
Date
Nov 10, 2016, 20:34 UTC
Message-ID
<20161110203428.30512-12-sbeller@google.com>
In-Reply-To
<20161110203428.30512-1-sbeller@google.com>
From: Junio C Hamano <gitster@pobox.com>
A common pattern to check N attributes for many paths is to
 (1) prepare an array A of N git_attr_check_elem items;
 (2) call git_attr() to intern the N attribute names and fill A;
 (3) repeatedly call git_check_attrs() for path with N and A;

A look-up for these N attributes for a single path P scans the entire attr_stack, starting from the .git/info/attributes file and then .gitattributes file in the directory the path P is in, going upwards to find .gitattributes file found in parent directories.

An earlier commit 06a604e6 (attr: avoid heavy work when we know the specified attr is not defined, 2014-12-28) tried to optimize out this scanning for one trivial special case: when the attribute being sought is known not to exist, we do not have to scan for it. While this may be a cheap and effective heuristic, it would not work well when N is (much) more than 1.

What we would want is a more customized way to skip irrelevant entries in the attribute stack, and the definition of irrelevance is tied to the set of attributes passed to git_check_attrs() call, i.e. the set of attributes being sought. The data necessary for this optimization needs to live alongside the set of attributes, but a simple array of git_attr_check_elem simply does not have any place for that.

Introduce "struct git_attr_check" that contains N, the number of attributes being sought, and A, the array that holds N git_attr_check_elem items, and a function git_check_attr() that takes a path P and this structure as its parameters. This structure can later be extended to hold extra data necessary for optimization.

Also, to make it easier to write the first two steps in common cases, introduce git_attr_check_initl() helper function, which takes a NULL-terminated list of attribute names and initialize this structure.

As an illustration of this new API, convert archive.c that asks for export-subst and export-ignore attributes for each paths.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 archive.c | 24 ++++++------------------
 attr.c    | 34 ++++++++++++++++++++++++++++++++++
 attr.h    |  9 +++++++++
 3 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/archive.c b/archive.c
index 2dc8d6c..11e3951 100644
--- a/archive.c
+++ b/archive.c
@@ -87,19 +87,6 @@ void *sha1_file_to_archive(const struct archiver_args *args,
 	return buffer;
 }
 
-static void setup_archive_check(struct git_attr_check_elem *check)
-{
-	static struct git_attr *attr_export_ignore;
-	static struct git_attr *attr_export_subst;
-
-	if (!attr_export_ignore) {
-		attr_export_ignore = git_attr("export-ignore");
-		attr_export_subst = git_attr("export-subst");
-	}
-	check[0].attr = attr_export_ignore;
-	check[1].attr = attr_export_subst;
-}
-
 struct directory {
 	struct directory *up;
 	struct object_id oid;
@@ -123,7 +110,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
 	struct archiver_context *c = context;
 	struct archiver_args *args = c->args;
 	write_archive_entry_fn_t write_entry = c->write_entry;
-	struct git_attr_check_elem check[2];
+	static struct git_attr_check *check;
 	const char *path_without_prefix;
 	int err;
 
@@ -137,11 +124,12 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
 		strbuf_addch(&path, '/');
 	path_without_prefix = path.buf + args->baselen;
 
-	setup_archive_check(check);
-	if (!git_check_attrs(path_without_prefix, ARRAY_SIZE(check), check)) {
-		if (ATTR_TRUE(check[0].value))
+	if (!check)
+		check = git_attr_check_initl("export-ignore", "export-subst", NULL);
+	if (!git_check_attr(path_without_prefix, check)) {
+		if (ATTR_TRUE(check->check[0].value))
 			return 0;
-		args->convert = ATTR_TRUE(check[1].value);
+		args->convert = ATTR_TRUE(check->check[1].value);
 	}
 
 	if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
diff --git a/attr.c b/attr.c
index c99e23a..861e1a2 100644
--- a/attr.c
+++ b/attr.c
@@ -829,3 +829,37 @@ void git_attr_set_direction(enum git_attr_direction new, struct index_state *ist
 		drop_attr_stack();
 	use_index = istate;
 }
+
+int git_check_attr(const char *path, struct git_attr_check *check)
+{
+	return git_check_attrs(path, check->check_nr, check->check);
+}
+
+struct git_attr_check *git_attr_check_initl(const char *one, ...)
+{
+	struct git_attr_check *check;
+	int cnt;
+	va_list params;
+	const char *param;
+
+	va_start(params, one);
+	for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
+		;
+	va_end(params);
+	check = xcalloc(1,
+			sizeof(*check) + cnt * sizeof(*(check->check)));
+	check->check_nr = cnt;
+	check->check = (struct git_attr_check_elem *)(check + 1);
+
+	check->check[0].attr = git_attr(one);
+	va_start(params, one);
+	for (cnt = 1; cnt < check->check_nr; cnt++) {
+		param = va_arg(params, const char *);
+		if (!param)
+			die("BUG: counted %d != ended at %d",
+			    check->check_nr, cnt);
+		check->check[cnt].attr = git_attr(param);
+	}
+	va_end(params);
+	return check;
+}
diff --git a/attr.h b/attr.h
index dd3c4a3..3fd8690 100644
--- a/attr.h
+++ b/attr.h
@@ -29,6 +29,15 @@ struct git_attr_check_elem {
 	const char *value;
 };
 
+struct git_attr_check {
+	int check_nr;
+	int check_alloc;
+	struct git_attr_check_elem *check;
+};
+
+extern struct git_attr_check *git_attr_check_initl(const char *, ...);
+extern int git_check_attr(const char *path, struct git_attr_check *);
+
 /*
  * Return the name of the attribute represented by the argument.  The
  * return value is a pointer to a null-delimited string that is part

-- 
2.10.1.469.g00a8914
Previous: Stefan BellerNext: Stefan Beller
Message 8 of 43 in “[PATCHv3 00/35] Revamp the attr subsystem!”
  1. Stefan BellerNov 10, 2016
  2. 02/35 attr.c: use strchrnul() to scan for one lineStefan Beller, Nov 10, 2016
  3. 06/35 attr.c: mark where #if DEBUG ends more clearlyStefan Beller, Nov 10, 2016
  4. 05/35 attr.c: complete a sentence in a commentStefan Beller, Nov 10, 2016
  5. 09/35 attr.c: plug small leak in parse_attr_line()Stefan Beller, Nov 10, 2016
  6. 10/35 attr: rename function and struct related to checking attributesStefan Beller, Nov 10, 2016
  7. 12/35 attr: convert git_all_attrs() to use "struct git_attr_check"Stefan Beller, Nov 10, 2016
  8. 11/35 attr: (re)introduce git_check_attr() and struct git_attr_checkStefan Beller, Nov 10, 2016
  9. 13/35 attr: convert git_check_attrs() callers to use the new APIStefan Beller, Nov 10, 2016
  10. 18/35 attr.c: add push_stack() helperStefan Beller, Nov 10, 2016
  11. 20/35 attr.c: rename a local variable checkStefan Beller, Nov 10, 2016
  12. 22/35 attr.c: introduce empty_attr_check_elems()Stefan Beller, Nov 10, 2016
  13. 19/35 attr.c: pass struct git_attr_check down the callchainStefan Beller, Nov 10, 2016
  14. 17/35 attr: support quoting pathname patterns in C styleStefan Beller, Nov 10, 2016
  15. 16/35 attr: expose validity check for attribute namesStefan Beller, Nov 10, 2016
  16. 34/35 clone: add --init-submodule=<pathspec> switchStefan Beller, Nov 10, 2016
  17. 26/35 attr: convert to new threadsafe APIStefan Beller, Nov 10, 2016
  18. 35/35 completion: clone can initialize specific submodulesStefan Beller, Nov 10, 2016
  19. 27/35 attr: keep attr stack for each checkStefan Beller, Nov 10, 2016
  20. 33/35 submodule update: add `--init-default-path` switchStefan Beller, Nov 10, 2016
  21. 28/35 Documentation: fix a typoStefan Beller, Nov 10, 2016
  22. 31/35 pathspec: allow querying for attributesStefan Beller, Nov 10, 2016
  23. Duy NguyenNov 22, 2016
  24. Stefan BellerNov 22, 2016
  25. Duy NguyenNov 23, 2016
  26. Brandon WilliamsNov 28, 2016
  27. Brandon WilliamsNov 28, 2016
  28. Brandon WilliamsNov 28, 2016
  29. Stefan BellerNov 29, 2016
  30. 32/35 pathspec: allow escaped query valuesStefan Beller, Nov 10, 2016
  31. 30/35 pathspec: move prefix check out of the inner loopStefan Beller, Nov 10, 2016
  32. 29/35 pathspec: move long magic parsing out of prefix_pathspecStefan Beller, Nov 10, 2016
  33. 23/35 attr.c: always pass check[] to collect_some_attrs()Stefan Beller, Nov 10, 2016
  34. 25/35 attr: make git_check_attr_counted staticStefan Beller, Nov 10, 2016
  35. 24/35 attr.c: outline the future plans by heavily commentingStefan Beller, Nov 10, 2016
  36. 21/35 attr.c: correct ugly hack for git_all_attrs()Stefan Beller, Nov 10, 2016
  37. 14/35 attr: retire git_check_attrs() APIStefan Beller, Nov 10, 2016
  38. 01/35 commit.c: use strchrnul() to scan for one lineStefan Beller, Nov 10, 2016
  39. 15/35 attr: add counted string version of git_check_attr()Stefan Beller, Nov 10, 2016
  40. 08/35 attr.c: tighten constness around "git_attr" structureStefan Beller, Nov 10, 2016
  41. 03/35 attr.c: update a stale comment on "struct match_attr"Stefan Beller, Nov 10, 2016
  42. 07/35 attr.c: simplify macroexpand_one()Stefan Beller, Nov 10, 2016
  43. 04/35 attr.c: explain the lack of attr-name syntax check in parse_attr()Stefan Beller, Nov 10, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.