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

[PATCHv3 00/35] Revamp the attr subsystem!

From
Stefan Beller <sbeller@google.com>
Date
Nov 10, 2016, 20:33 UTC
Message-ID
<20161110203428.30512-1-sbeller@google.com>
previous discussion: https://public-inbox.org/git/20161028185502.8789-1-sbeller@google.com/
Changes since v2:
* dropped one patch (that exposes git_attr_counted, nobody uses
  it throughout the series )
* added pathspec guarding and added a test with git-add to see if it works.
* squashed "SQUASH" commits.
* changed test for multithreaded tests as git-status makes use of pathspec
  code that doesn't support this new magic yet. 
interdiff to currently queued at the bottom of this email.

v2 was: previous discussion at https://public-inbox.org/git/20161022233225.8883-1-sbeller@google.com

This implements the discarded series': jc/attr jc/attr-more sb/pathspec-label sb/submodule-default-paths

This includes
* The fixes for windows
* Junios latest suggestion to use git_attr_check_initv instead of
  alloc/append.
* I implemented the thread safe attr API in patch 27 (attr: convert to new threadsafe API)
* patch 28 (attr: keep attr stack for each check) makes it actually possible
  to run in a multithreaded environment.
* I added a test for the multithreaded when it is introduced in patch 32
  (pathspec: allow querying for attributes) as well as a test to disallow
  multiple "attr"s in a pathspec.

Thanks, Stefan

Junio C Hamano (23):
  commit.c: use strchrnul() to scan for one line
  attr.c: use strchrnul() to scan for one line
  attr.c: update a stale comment on "struct match_attr"
  attr.c: explain the lack of attr-name syntax check in parse_attr()
  attr.c: complete a sentence in a comment
  attr.c: mark where #if DEBUG ends more clearly
  attr.c: simplify macroexpand_one()
  attr.c: tighten constness around "git_attr" structure
  attr.c: plug small leak in parse_attr_line()
  attr: rename function and struct related to checking attributes
  attr: (re)introduce git_check_attr() and struct git_attr_check
  attr: convert git_all_attrs() to use "struct git_attr_check"
  attr: convert git_check_attrs() callers to use the new API
  attr: retire git_check_attrs() API
  attr: add counted string version of git_check_attr()
  attr: expose validity check for attribute names
  attr.c: add push_stack() helper
  attr.c: pass struct git_attr_check down the callchain
  attr.c: rename a local variable check
  attr.c: correct ugly hack for git_all_attrs()
  attr.c: introduce empty_attr_check_elems()
  attr.c: always pass check[] to collect_some_attrs()
  attr.c: outline the future plans by heavily commenting
Nguyễn Thái Ngọc Duy (1):
  attr: support quoting pathname patterns in C style
Stefan Beller (11):
  attr: make git_check_attr_counted static
  attr: convert to new threadsafe API
  attr: keep attr stack for each check
  Documentation: fix a typo
  pathspec: move long magic parsing out of prefix_pathspec
  pathspec: move prefix check out of the inner loop
  pathspec: allow querying for attributes
  pathspec: allow escaped query values
  submodule update: add `--init-default-path` switch
  clone: add --init-submodule=<pathspec> switch
  completion: clone can initialize specific submodules
 Documentation/config.txt                      |   5 +
 Documentation/git-clone.txt                   |  23 +-
 Documentation/git-submodule.txt               |  17 +-
 Documentation/gitattributes.txt               |  10 +-
 Documentation/glossary-content.txt            |  20 +
 Documentation/technical/api-gitattributes.txt | 140 ++++---
 archive.c                                     |  25 +-
 attr.c                                        | 517 ++++++++++++++++++--------
 attr.h                                        |  71 ++--
 builtin/check-attr.c                          |  74 ++--
 builtin/clone.c                               |  36 +-
 builtin/pack-objects.c                        |  27 +-
 commit.c                                      |   3 +-
 common-main.c                                 |   2 +
 contrib/completion/git-completion.bash        |   1 +
 convert.c                                     |  46 ++-
 dir.c                                         |  41 +-
 git-submodule.sh                              |  21 +-
 hashmap.h                                     |   2 +
 ll-merge.c                                    |  35 +-
 pathspec.c                                    | 228 ++++++++++--
 pathspec.h                                    |  19 +-
 t/t0003-attributes.sh                         |  26 ++
 t/t6134-pathspec-with-labels.sh               | 201 ++++++++++
 t/t7400-submodule-basic.sh                    | 134 +++++++
 userdiff.c                                    |  21 +-
 ws.c                                          |  19 +-
 27 files changed, 1354 insertions(+), 410 deletions(-)
 create mode 100755 t/t6134-pathspec-with-labels.sh
-- interdiff to current origin/sb/attr:
diff --git a/attr.c b/attr.c
index cf15e76..979bd3c 100644
--- a/attr.c
+++ b/attr.c
@@ -135,7 +135,7 @@ static void report_invalid_attr(const char *name, size_t len,
 	strbuf_release(&err);
 }
 
-struct git_attr *git_attr_counted(const char *name, size_t len)
+static struct git_attr *git_attr_internal(const char *name, int len)
 {
 	unsigned hval = hash_name(name, len);
 	unsigned pos = hval % HASHSIZE;
@@ -171,7 +171,7 @@ struct git_attr *git_attr_counted(const char *name, size_t len)
 
 struct git_attr *git_attr(const char *name)
 {
-	return git_attr_counted(name, strlen(name));
+	return git_attr_internal(name, strlen(name));
 }
 
 /* What does a matched pattern decide? */
@@ -259,7 +259,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
 		else {
 			e->setto = xmemdupz(equals + 1, ep - equals - 1);
 		}
-		e->attr = git_attr_counted(cp, len);
+		e->attr = git_attr_internal(cp, len);
 	}
 	return ep + strspn(ep, blank);
 }
@@ -320,7 +320,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
 		      sizeof(struct attr_state) * num_attr +
 		      (is_macro ? 0 : namelen + 1));
 	if (is_macro) {
-		res->u.attr = git_attr_counted(name, namelen);
+		res->u.attr = git_attr_internal(name, namelen);
 		res->u.attr->maybe_macro = 1;
 	} else {
 		char *p = (char *)&(res->state[num_attr]);
diff --git a/attr.h b/attr.h
index e7534da..ab381ae 100644
--- a/attr.h
+++ b/attr.h
@@ -2,7 +2,7 @@
 #define ATTR_H
 
 /*
- * Must be called on platforms that do not support static initialisation
+ * Must be called on platforms that do not support static initialization
  * of mutexes.
  */
 extern void attr_start(void);
diff --git a/dir.c b/dir.c
index 3ec9117..c56d2ee 100644
--- a/dir.c
+++ b/dir.c
@@ -140,7 +140,8 @@ static size_t common_prefix_len(const struct pathspec *pathspec)
 		       PATHSPEC_LITERAL |
 		       PATHSPEC_GLOB |
 		       PATHSPEC_ICASE |
-		       PATHSPEC_EXCLUDE);
+		       PATHSPEC_EXCLUDE |
+		       PATHSPEC_ATTR);
 
 	for (n = 0; n < pathspec->nr; n++) {
 		size_t i = 0, len = 0, item_len;
@@ -349,7 +350,8 @@ static int do_match_pathspec(const struct pathspec *ps,
 		       PATHSPEC_LITERAL |
 		       PATHSPEC_GLOB |
 		       PATHSPEC_ICASE |
-		       PATHSPEC_EXCLUDE);
+		       PATHSPEC_EXCLUDE |
+		       PATHSPEC_ATTR);
 
 	if (!ps->nr) {
 		if (!ps->recursive ||
diff --git a/pathspec.c b/pathspec.c
index 8ea5dcb..d702803 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -232,6 +232,7 @@ static void eat_long_magic(struct pathspec_item *item, const char *elt,
 			char *attr_body = xmemdupz(body, len - strlen("attr:"));
 			parse_pathspec_attr_match(item, attr_body);
 			free(attr_body);
+			*magic |= PATHSPEC_ATTR;
 			continue;
 		}
 
diff --git a/pathspec.h b/pathspec.h
index ebef4bf..c2d046d 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -8,13 +8,15 @@
 #define PATHSPEC_GLOB		(1<<3)
 #define PATHSPEC_ICASE		(1<<4)
 #define PATHSPEC_EXCLUDE	(1<<5)
+#define PATHSPEC_ATTR		(1<<6)
 #define PATHSPEC_ALL_MAGIC	  \
 	(PATHSPEC_FROMTOP	| \
 	 PATHSPEC_MAXDEPTH	| \
 	 PATHSPEC_LITERAL	| \
 	 PATHSPEC_GLOB		| \
 	 PATHSPEC_ICASE		| \
-	 PATHSPEC_EXCLUDE)
+	 PATHSPEC_EXCLUDE	| \
+	 PATHSPEC_ATTR)
 
 #define PATHSPEC_ONESTAR 1	/* the pathspec pattern satisfies GFNM_ONESTAR */
 
diff --git a/t/t6134-pathspec-with-labels.sh b/t/t6134-pathspec-with-labels.sh
index 3d03e0a..5aafc0c 100755
--- a/t/t6134-pathspec-with-labels.sh
+++ b/t/t6134-pathspec-with-labels.sh
@@ -160,7 +160,23 @@ test_expect_success 'fail on multiple attr specifiers in one pathspec item' '
 '
 
 test_expect_success 'checking attributes in a multithreaded process' '
-	git status ":(attr:labelB)"
+	cat <<-EOF >expect &&
+	fileA:A
+	EOF
+	echo A >fileA &&
+	echo A >fileB &&
+	git grep A -- ":(attr:labelA)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'fail if attr magic is used places not implemented' '
+	# The main purpose of this test is to check that we actually fail
+	# when you attempt to use attr magic in commands that do not implement
+	# attr magic. This test does not advocate git-add to stay that way,
+	# though, but git-add is convenient as it has its own internal pathspec
+	# parsing.
+	test_must_fail git add ":(attr:labelB)" 2>actual &&
+	test_i18ngrep "unsupported magic" actual
 '
 
 test_expect_success 'abort on giving invalid label on the command line' '

-- 
2.10.1.469.g00a8914
Next: Stefan Beller
Message 1 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.