threads / patch / 28018

v2, 6 partsgit-check-attr should work for relative paths

Subject: [PATCH v2 0/6] git-check-attr should work for relative paths

## tl;dr

7 messages between Aug 4, 2011 and Aug 4, 2011. Diffs are folded; open one to read it.

replies: 6people: 1as markdown or json

Michael Haggerty· Aug 4, 2011, 04:47 UTC · lore

This is a reroll of the earlier RFC. It must be applied on top of "Add --all option to git-check-attr" v3.

This re-roll differs in the following ways from the RFC:
* Adjusted for changes between the "--all" patch series v2 -> v3.
* Fixed some C problems found by Junio.

Currently, git-check-attr gets confused if it is passed certain types of unnormalized paths or is invoked from a non-top-level directory. For example, using the test repo created by t0003-attributes.sh, inquiring about a path that starts with "./" causes git-check-attr to emit a spurious warning:

    $ git check-attr test ./f
    [attr]notest !test
     not allowed: ./.gitattributes:1
    ./f: test: f
    $ git check-attr test ./a/i
    [attr]notest !test
     not allowed: ./.gitattributes:1
    ./a/i: test: a/i

In these cases it seems to find the right values, even including the application of macros from the top-level .gitattributes file. (My guess is that it is loading the top-level .gitattributes file twice--once as a top-level file, and once believing that it is in a subdirectory.)

Invoking git-check-attr from a subdirectory using a relative path gives incorrect answers; for example:

    $ (cd a; git check-attr test i )
    i: test: unspecified

(On the other hand, if invoked using the full path of a file, it "works":

    $ (cd a; git check-attr test a/i )
    a/i: test: a/i

.) I think that this behavior is confusing and inconsistent with other git commands. It is also inconsistent with the behavior of .gitignore, which works from subdirectories.

The patches in this series add tests demonstrating the problem, propose a possible solution, and add a couple of subcommands to test-path-utils.

The changes to test-path-utils helped me figure out how the corresponding functions work (comments? we don't need no stinkin' comments) but they are not used anywhere. Take 'em or leave 'em.

Michael Haggerty (6):
  git-check-attr: test that no output is written to stderr
  git-check-attr: Demonstrate problems with unnormalized paths
  git-check-attr: Demonstrate problems with relative paths
  git-check-attr: Normalize paths
  test-path-utils: Add subcommand "absolute_path"
  test-path-utils: Add subcommand "prefix_path"
 builtin/check-attr.c  |   20 ++++++++++++--------
 t/t0003-attributes.sh |   29 ++++++++++++++++++++++++++---
 test-path-utils.c     |   22 ++++++++++++++++++++++
 3 files changed, 60 insertions(+), 11 deletions(-)
-- 
1.7.6.8.gd2879
Michael Haggerty· Aug 4, 2011, 04:47 UTC · re: Michael Haggerty · lore

[PATCH v2 1/6] git-check-attr: test that no output is written to stderr

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 t/t0003-attributes.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
Show changes to t/t0003-attributes.sh +3 −2
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 5acb2d5..4f2fbc0 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -9,9 +9,10 @@ attr_check () {
 	path="$1"
 	expect="$2"
 
-	git check-attr test -- "$path" >actual &&
+	git check-attr test -- "$path" >actual 2>err &&
 	echo "$path: test: $2" >expect &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	test_line_count = 0 err
 
 }
 

-- 
1.7.6.8.gd2879
Michael Haggerty· Aug 4, 2011, 04:47 UTC · re: Michael Haggerty · lore

[PATCH v2 2/6] git-check-attr: Demonstrate problems with unnormalized paths

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 t/t0003-attributes.sh |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
Show changes to t/t0003-attributes.sh +9 −0
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 4f2fbc0..43957ea 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -93,6 +93,15 @@ test_expect_success 'attribute test' '
 
 '
 
+test_expect_failure 'unnormalized paths' '
+
+	attr_check ./f f &&
+	attr_check ./a/g a/g &&
+	attr_check a/./g a/g &&
+	attr_check a/c/../b/g a/b/g
+
+'
+
 test_expect_success 'core.attributesfile' '
 	attr_check global unspecified &&
 	git config core.attributesfile "$HOME/global-gitattributes" &&

-- 
1.7.6.8.gd2879
Michael Haggerty· Aug 4, 2011, 04:47 UTC · re: Michael Haggerty · lore

[PATCH v2 3/6] git-check-attr: Demonstrate problems with relative paths

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 t/t0003-attributes.sh |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)
Show changes to t/t0003-attributes.sh +14 −1
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 43957ea..f6cf77d 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -19,7 +19,7 @@ attr_check () {
 
 test_expect_success 'setup' '
 
-	mkdir -p a/b/d a/c &&
+	mkdir -p a/b/d a/c b &&
 	(
 		echo "[attr]notest !test"
 		echo "f	test=f"
@@ -102,6 +102,19 @@ test_expect_failure 'unnormalized paths' '
 
 '
 
+test_expect_failure 'relative paths' '
+
+	(cd a && attr_check ../f f) &&
+	(cd a && attr_check f f) &&
+	(cd a && attr_check i a/i) &&
+	(cd a && attr_check g a/g) &&
+	(cd a && attr_check b/g a/b/g) &&
+	(cd b && attr_check ../a/f f) &&
+	(cd b && attr_check ../a/g a/g) &&
+	(cd b && attr_check ../a/b/g a/b/g)
+
+'
+
 test_expect_success 'core.attributesfile' '
 	attr_check global unspecified &&
 	git config core.attributesfile "$HOME/global-gitattributes" &&

-- 
1.7.6.8.gd2879
Michael Haggerty· Aug 4, 2011, 04:47 UTC · re: Michael Haggerty · lore

[PATCH v2 4/6] git-check-attr: Normalize paths

Normalize the path arguments (relative to the working tree root, if applicable) before looking up their attributes. This requires passing the prefix down the call chain.

This fixes two test cases for different reasons:
* "unnormalized paths" is fixed because the .gitattribute-file-seeking
  code is not confused into reading the top-level file twice.
* "relative paths" is fixed because the canonical pathnames are passed
  to get_check_attr() or get_all_attrs(), allowing them to match the
  pathname patterns as expected.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 builtin/check-attr.c  |   20 ++++++++++++--------
 t/t0003-attributes.sh |    4 ++--
 2 files changed, 14 insertions(+), 10 deletions(-)
Show changes to 2 files +14 −10

builtin/check-attr.c, t/t0003-attributes.sh

diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 6b16368..708988a 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -41,22 +41,26 @@ static void output_attr(int cnt, struct git_attr_check *check,
 	}
 }
 
-static void check_attr(int cnt, struct git_attr_check *check,
-	const char *file)
+static void check_attr(const char *prefix, int cnt,
+	struct git_attr_check *check, const char *file)
 {
+	char *full_path =
+		prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
 	if (check != NULL) {
-		if (git_check_attr(file, cnt, check))
+		if (git_check_attr(full_path, cnt, check))
 			die("git_check_attr died");
 		output_attr(cnt, check, file);
 	} else {
-		if (git_all_attrs(file, &cnt, &check))
+		if (git_all_attrs(full_path, &cnt, &check))
 			die("git_all_attrs died");
 		output_attr(cnt, check, file);
 		free(check);
 	}
+	free(full_path);
 }
 
-static void check_attr_stdin_paths(int cnt, struct git_attr_check *check)
+static void check_attr_stdin_paths(const char *prefix, int cnt,
+	struct git_attr_check *check)
 {
 	struct strbuf buf, nbuf;
 	int line_termination = null_term_line ? 0 : '\n';
@@ -70,7 +74,7 @@ static void check_attr_stdin_paths(int cnt, struct git_attr_check *check)
 				die("line is badly quoted");
 			strbuf_swap(&buf, &nbuf);
 		}
-		check_attr(cnt, check, buf.buf);
+		check_attr(prefix, cnt, check, buf.buf);
 		maybe_flush_or_die(stdout, "attribute to stdout");
 	}
 	strbuf_release(&buf);
@@ -154,10 +158,10 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
 	}
 
 	if (stdin_paths)
-		check_attr_stdin_paths(cnt, check);
+		check_attr_stdin_paths(prefix, cnt, check);
 	else {
 		for (i = filei; i < argc; i++)
-			check_attr(cnt, check, argv[i]);
+			check_attr(prefix, cnt, check, argv[i]);
 		maybe_flush_or_die(stdout, "attribute to stdout");
 	}
 	return 0;
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index f6cf77d..ae2f1da 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -93,7 +93,7 @@ test_expect_success 'attribute test' '
 
 '
 
-test_expect_failure 'unnormalized paths' '
+test_expect_success 'unnormalized paths' '
 
 	attr_check ./f f &&
 	attr_check ./a/g a/g &&
@@ -102,7 +102,7 @@ test_expect_failure 'unnormalized paths' '
 
 '
 
-test_expect_failure 'relative paths' '
+test_expect_success 'relative paths' '
 
 	(cd a && attr_check ../f f) &&
 	(cd a && attr_check f f) &&

-- 
1.7.6.8.gd2879
Michael Haggerty· Aug 4, 2011, 04:47 UTC · re: Michael Haggerty · lore

[PATCH v2 5/6] test-path-utils: Add subcommand "absolute_path"

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 test-path-utils.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
Show changes to test-path-utils.c +9 −0
diff --git a/test-path-utils.c b/test-path-utils.c
index e767159..a410e30 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -20,6 +20,15 @@ int main(int argc, char **argv)
 		return 0;
 	}
 
+	if (argc >= 2 && !strcmp(argv[1], "absolute_path")) {
+		while (argc > 2) {
+			puts(absolute_path(argv[2]));
+			argc--;
+			argv++;
+		}
+		return 0;
+	}
+
 	if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
 		int len = longest_ancestor_length(argv[2], argv[3]);
 		printf("%d\n", len);

-- 
1.7.6.8.gd2879
Michael Haggerty· Aug 4, 2011, 04:47 UTC · re: Michael Haggerty · lore

[PATCH v2 6/6] test-path-utils: Add subcommand "prefix_path"

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 test-path-utils.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
Show changes to test-path-utils.c +13 −0
diff --git a/test-path-utils.c b/test-path-utils.c
index a410e30..3bc20e9 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -35,6 +35,19 @@ int main(int argc, char **argv)
 		return 0;
 	}
 
+	if (argc >= 4 && !strcmp(argv[1], "prefix_path")) {
+		char *prefix = argv[2];
+		int prefix_len = strlen(prefix);
+		int nongit_ok;
+		setup_git_directory_gently(&nongit_ok);
+		while (argc > 3) {
+			puts(prefix_path(prefix, prefix_len, argv[3]));
+			argc--;
+			argv++;
+		}
+		return 0;
+	}
+
 	if (argc == 4 && !strcmp(argv[1], "strip_path_suffix")) {
 		char *prefix = strip_path_suffix(argv[2], argv[3]);
 		printf("%s\n", prefix ? prefix : "(null)");

-- 
1.7.6.8.gd2879

← back to recent threads