threads / patch / 23308

v2, 3 partsHandling of gitattribute macro expansion

Subject: [PATCH v2 0/3] Handling of gitattribute macro expansion

## tl;dr

4 messages between Apr 2, 2010 and Apr 2, 2010. Diffs are folded; open one to read it.

replies: 3people: 1as markdown or json

Henrik Grubbström (Grubba)· Apr 2, 2010, 19:33 UTC · lore

Second try at handling macro expansion in gitattribute files in a deterministic way.

Henrik Grubbström (Grubba) (3):
  Attr: Fixed debug output for macro expansion.
  Attr: Allow multiple changes to an attribute on the same line.
  Attr: Expand macros immediately when encountered.
 attr.c                |   38 ++++++++++++++++++++++++--------------
 t/t0003-attributes.sh |   15 +++++++++++++++
 2 files changed, 39 insertions(+), 14 deletions(-)
Henrik Grubbström (Grubba)· Apr 2, 2010, 19:33 UTC · re: Henrik Grubbström (Grubba) · lore

[PATCH 1/3] Attr: Fixed debug output for macro expansion.

When debug_set() was called during macro expansion, it received a pointer to a struct git_attr rather than a string.

Signed-off-by: Henrik Grubbström <grubba@grubba.org>
---
 attr.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Show changes to attr.c +3 −1
diff --git a/attr.c b/attr.c
index f5346ed..5c6464e 100644
--- a/attr.c
+++ b/attr.c
@@ -605,7 +605,9 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
 		const char *v = a->state[i].setto;
 
 		if (*n == ATTR__UNKNOWN) {
-			debug_set(what, a->u.pattern, attr, v);
+			debug_set(what,
+				  a->is_macro?a->u.attr->name:a->u.pattern,
+				  attr, v);
 			*n = v;
 			rem--;
 		}
-- 
1.7.0.3.316.g33b5e
Henrik Grubbström (Grubba)· Apr 2, 2010, 19:33 UTC · re: Henrik Grubbström (Grubba) · lore

[PATCH v2 2/3] Attr: Allow multiple changes to an attribute on the same line.

When using macros it isn't inconceivable to have an attribute being set by a macro, and then being reset explicitly. Always use the latest value.

Signed-off-by: Henrik Grubbström <grubba@grubba.org>
---
 attr.c                |    2 +-
 t/t0003-attributes.sh |    6 ++++++
 2 files changed, 7 insertions(+), 1 deletions(-)
Show changes to 2 files +7 −1

attr.c, t/t0003-attributes.sh

diff --git a/attr.c b/attr.c
index 5c6464e..968fb8b 100644
--- a/attr.c
+++ b/attr.c
@@ -599,7 +599,7 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
 	struct git_attr_check *check = check_all_attr;
 	int i;
 
-	for (i = 0; 0 < rem && i < a->num_attr; i++) {
+	for (i = a->num_attr - 1; 0 < rem && 0 <= i; i--) {
 		struct git_attr *attr = a->state[i].attr;
 		const char **n = &(check[attr->attr_nr].value);
 		const char *v = a->state[i].setto;
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 1c77192..bd9c8de 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -22,6 +22,8 @@ test_expect_success 'setup' '
 	(
 		echo "f	test=f"
 		echo "a/i test=a/i"
+		echo "onoff test -test"
+		echo "offon -test test"
 	) >.gitattributes &&
 	(
 		echo "g test=a/g" &&
@@ -44,6 +46,8 @@ test_expect_success 'attribute test' '
 	attr_check b/g unspecified &&
 	attr_check a/b/h a/b/h &&
 	attr_check a/b/d/g "a/b/d/*"
+	attr_check onoff unset
+	attr_check offon set
 
 '
 
@@ -58,6 +62,8 @@ a/b/g: test: a/b/g
 b/g: test: unspecified
 a/b/h: test: a/b/h
 a/b/d/g: test: a/b/d/*
+onoff: test: unset
+offon: test: set
 EOF
 
 	sed -e "s/:.*//" < expect | git check-attr --stdin test > actual &&
-- 
1.7.0.3.316.g33b5e
Henrik Grubbström (Grubba)· Apr 2, 2010, 19:33 UTC · re: Henrik Grubbström (Grubba) · lore

[PATCH v2 3/3] Attr: Expand macros immediately when encountered.

When using macros it is otherwise hard to know whether an attribute set by the macro should override an already set attribute. Consider the following .gitattributes file:

[attr]mybinary binary -ident * ident foo.bin mybinary bar.bin mybinary ident

Without this patch both foo.bin and bar.bin will have the ident attribute set, which is probably not what the user expects. With this patch foo.bin will have an unset ident attribute, while bar.bin will have it set.

Signed-off-by: Henrik Grubbström <grubba@grubba.org>
---
 attr.c                |   32 ++++++++++++++++++++------------
 t/t0003-attributes.sh |    9 +++++++++
 2 files changed, 29 insertions(+), 12 deletions(-)
Show changes to 2 files +29 −12

attr.c, t/t0003-attributes.sh

diff --git a/attr.c b/attr.c
index 968fb8b..f90bb8e 100644
--- a/attr.c
+++ b/attr.c
@@ -594,6 +594,8 @@ static int path_matches(const char *pathname, int pathlen,
 	return fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;
 }
 
+static int macroexpand_one(int attr_nr, int rem);
+
 static int fill_one(const char *what, struct match_attr *a, int rem)
 {
 	struct git_attr_check *check = check_all_attr;
@@ -610,6 +612,7 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
 				  attr, v);
 			*n = v;
 			rem--;
+			rem = macroexpand_one(attr->attr_nr, rem);
 		}
 	}
 	return rem;
@@ -631,19 +634,27 @@ static int fill(const char *path, int pathlen, struct attr_stack *stk, int rem)
 	return rem;
 }
 
-static int macroexpand(struct attr_stack *stk, int rem)
+static int macroexpand_one(int attr_nr, int rem)
 {
+	struct attr_stack *stk;
+	struct match_attr *a = NULL;
 	int i;
-	struct git_attr_check *check = check_all_attr;
 
-	for (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {
-		struct match_attr *a = stk->attrs[i];
-		if (!a->is_macro)
-			continue;
-		if (check[a->u.attr->attr_nr].value != ATTR__TRUE)
-			continue;
+	if (check_all_attr[attr_nr].value != ATTR__TRUE)
+		return rem;
+
+	for (stk = attr_stack; !a && stk; stk = stk->prev)
+		for (i = stk->num_matches - 1; !a && 0 <= i; i--) {
+			struct match_attr *ma = stk->attrs[i];
+			if (!ma->is_macro)
+				continue;
+			if (ma->u.attr->attr_nr == attr_nr)
+				a = ma;
+		}
+
+	if (a)
 		rem = fill_one("expand", a, rem);
-	}
+
 	return rem;
 }
 
@@ -668,9 +679,6 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)
 	for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
 		rem = fill(path, pathlen, stk, rem);
 
-	for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
-		rem = macroexpand(stk, rem);
-
 	for (i = 0; i < num; i++) {
 		const char *value = check_all_attr[check[i].attr->attr_nr].value;
 		if (value == ATTR__UNKNOWN)
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index bd9c8de..53bd7fc 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -20,10 +20,12 @@ test_expect_success 'setup' '
 
 	mkdir -p a/b/d a/c &&
 	(
+		echo "[attr]notest !test"
 		echo "f	test=f"
 		echo "a/i test=a/i"
 		echo "onoff test -test"
 		echo "offon -test test"
+		echo "no notest"
 	) >.gitattributes &&
 	(
 		echo "g test=a/g" &&
@@ -32,6 +34,7 @@ test_expect_success 'setup' '
 	(
 		echo "h test=a/b/h" &&
 		echo "d/* test=a/b/d/*"
+		echo "d/yes notest"
 	) >a/b/.gitattributes
 
 '
@@ -48,6 +51,9 @@ test_expect_success 'attribute test' '
 	attr_check a/b/d/g "a/b/d/*"
 	attr_check onoff unset
 	attr_check offon set
+	attr_check no unspecified
+	attr_check a/b/d/no "a/b/d/*"
+	attr_check a/b/d/yes unspecified
 
 '
 
@@ -64,6 +70,9 @@ a/b/h: test: a/b/h
 a/b/d/g: test: a/b/d/*
 onoff: test: unset
 offon: test: set
+no: test: unspecified
+a/b/d/no: test: a/b/d/*
+a/b/d/yes: test: unspecified
 EOF
 
 	sed -e "s/:.*//" < expect | git check-attr --stdin test > actual &&
-- 
1.7.0.3.316.g33b5e

← back to recent threads