[PATCH] add: fail "git add ignored-dir/file" without -f

Subsystems: the rest

DORMANTno replies

2 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH] add: fail "git add ignored-dir/file" without -f

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:22

"git add <pathspec>" usually fails the request and gives an advice message
to use the -f option when <pathspec> exactly names an existing path in the
work tree.  However, we didn't issue the warning nor fail the request when
the <pathspec> matches an existing path but it is ignored because a higher
level directory is ignored as a whole.  In such a case, we do not even
descend into it (and we shouldn't).

This catches such a case and issues some warning.

Signed-off-by: Junio C Hamano <redacted>
---

 * This is still provisional and I am not fully happy with it; it seems to
   pass the tests.  The error message is based on the full directory name
   we are culling, not on the actual pathspec the user gave us, as we do
   not have access to it.

 dir.c                  |   15 +++++-----
 t/t2204-add-ignored.sh |   70 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 7 deletions(-)
 create mode 100755 t/t2204-add-ignored.sh
diff --git a/dir.c b/dir.c
index 00d698d..af4ba92 100644
--- a/dir.c
+++ b/dir.c
@@ -413,13 +413,10 @@ static struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna
 	return dir->entries[dir->nr++] = dir_entry_new(pathname, len);
 }
 
-static struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)
+static void dir_add_ignored(struct dir_struct *dir, const char *pathname, int len)
 {
-	if (!cache_name_is_other(pathname, len))
-		return NULL;
-
 	ALLOC_GROW(dir->ignored, dir->ignored_nr+1, dir->ignored_alloc);
-	return dir->ignored[dir->ignored_nr++] = dir_entry_new(pathname, len);
+	dir->ignored[dir->ignored_nr++] = dir_entry_new(pathname, len);
 }
 
 enum exist_status {
@@ -638,7 +635,8 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 {
 	int exclude = excluded(dir, path, &dtype);
 	if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
-	    && in_pathspec(path, *len, simplify))
+	    && in_pathspec(path, *len, simplify)
+	    && cache_name_is_other(path, *len))
 		dir_add_ignored(dir, path, *len);
 
 	/*
@@ -841,8 +839,11 @@ static int treat_leading_path(struct dir_struct *dir,
 			return 0;
 		blen = baselen;
 		if (treat_one_path(dir, pathbuf, &blen, simplify,
-				   DT_DIR, NULL) == path_ignored)
+				   DT_DIR, NULL) == path_ignored) {
+			if (dir->flags & DIR_COLLECT_IGNORED)
+				dir_add_ignored(dir, pathbuf, baselen);
 			return 0; /* do not recurse into it */
+		}
 		if (len <= baselen)
 			return 1; /* finished checking */
 	}
diff --git a/t/t2204-add-ignored.sh b/t/t2204-add-ignored.sh
new file mode 100755
index 0000000..c1ce12b
--- /dev/null
+++ b/t/t2204-add-ignored.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+test_description='giving ignored paths to git add'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	mkdir sub dir dir/sub &&
+	echo sub >.gitignore &&
+	echo ign >>.gitignore &&
+	for p in . sub dir dir/sub
+	do
+		>"$p/ign" &&
+		>"$p/file" || exit 1
+	done
+'
+
+for i in file dir/file dir 'd*'
+do
+	test_expect_success "no complaints for unignored $i" '
+		rm -f .git/index &&
+		git add "$i" &&
+		git ls-files "$i" >out &&
+		test -s out
+	'
+done
+
+for i in ign dir/ign dir/sub dir/sub/*ign sub/file sub sub/*
+do
+	test_expect_success "complaints for ignored $i" '
+		rm -f .git/index &&
+		test_must_fail git add "$i" 2>err &&
+		git ls-files "$i" >out &&
+		! test -s out &&
+		grep -e "Use -f if" err &&
+		cat err
+	'
+done
+
+for i in sub sub/*
+do
+	test_expect_success "complaints for ignored $i in dir" '
+		rm -f .git/index &&
+		(
+			cd dir &&
+			test_must_fail git add "$i" 2>err &&
+			git ls-files "$i" >out &&
+			! test -s out &&
+			grep -e "Use -f if" err &&
+			cat err
+		)
+	'
+done
+
+for i in ign file
+do
+	test_expect_success "complaints for ignored $i in sub" '
+		rm -f .git/index &&
+		(
+			cd sub &&
+			test_must_fail git add "$i" 2>err &&
+			git ls-files "$i" >out &&
+			! test -s out &&
+			grep -e "Use -f if" err &&
+			cat err
+		)
+	'
+done
+
+test_done
-- 
1.7.0.1.241.g6604f

[PATCH 1/3] t0050: mark non-working test as such

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:22

The test is to prepare an empty file "camelcase" in the index, remove
and replace it with another file "CamelCase" with "1" as its contents
in the working tree, and add it to the index, in a repository configured
to be case insensitive.

However, the test actually checked ls-files knows about a pathname that
matches "camelcase" case insensitively.  It didn't check if the added
contents actually was the updated one.

Mark the test as non-working.

Signed-off-by: Junio C Hamano <redacted>
---

 * This comes from 8a19aaa (t0050: Add test for case insensitive add,
   2008-05-11); back then the code did add both camelcase and CamelCase,
   and somewhere after that we once fixed it, but 1e5f764 (builtin-add.c:
   optimize -A option and "git add .", 2008-07-22) broke it in a different
   way (namely, it stopped adding the updated contents); I am not going to
   have time to debug it further anytime soon (hint hint)...

 t/t0050-filesystem.sh |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 89282cc..41df6bc 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -108,13 +108,17 @@ $test_case 'merge (case change)' '
 
 '
 
-$test_case 'add (with different case)' '
+
+
+test_expect_failure 'add (with different case)' '
 
 	git reset --hard initial &&
 	rm camelcase &&
 	echo 1 >CamelCase &&
 	git add CamelCase &&
-	test $(git ls-files | grep -i camelcase | wc -l) = 1
+	camel=$(git ls-files | grep -i camelcase) &&
+	test $(echo "$camel" | wc -l) = 1 &&
+	test "z$(git cat-file blob :$camel)" = z1
 
 '
 
-- 
1.7.0.1.241.g6604f
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help