Merge pull request #4296 from pks-t/pks/pattern-based-gitignore Fix negative ignore rules with patterns
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224
diff --git a/src/ignore.c b/src/ignore.c
index 4c0d26d..f089dbe 100644
--- a/src/ignore.c
+++ b/src/ignore.c
@@ -48,38 +48,42 @@
*/
static int does_negate_pattern(git_attr_fnmatch *rule, git_attr_fnmatch *neg)
{
+ int (*cmp)(const char *, const char *, size_t);
git_attr_fnmatch *longer, *shorter;
char *p;
- if ((rule->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0
- && (neg->flags & GIT_ATTR_FNMATCH_NEGATIVE) != 0) {
-
- /* If lengths match we need to have an exact match */
- if (rule->length == neg->length) {
- return strcmp(rule->pattern, neg->pattern) == 0;
- } else if (rule->length < neg->length) {
- shorter = rule;
- longer = neg;
- } else {
- shorter = neg;
- longer = rule;
- }
+ if ((rule->flags & GIT_ATTR_FNMATCH_NEGATIVE) != 0
+ || (neg->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0)
+ return false;
+
+ if (neg->flags & GIT_ATTR_FNMATCH_ICASE)
+ cmp = git__strncasecmp;
+ else
+ cmp = strncmp;
+
+ /* If lengths match we need to have an exact match */
+ if (rule->length == neg->length) {
+ return cmp(rule->pattern, neg->pattern, rule->length) == 0;
+ } else if (rule->length < neg->length) {
+ shorter = rule;
+ longer = neg;
+ } else {
+ shorter = neg;
+ longer = rule;
+ }
- /* Otherwise, we need to check if the shorter
- * rule is a basename only (that is, it contains
- * no path separator) and, if so, if it
- * matches the tail of the longer rule */
- p = longer->pattern + longer->length - shorter->length;
+ /* Otherwise, we need to check if the shorter
+ * rule is a basename only (that is, it contains
+ * no path separator) and, if so, if it
+ * matches the tail of the longer rule */
+ p = longer->pattern + longer->length - shorter->length;
- if (p[-1] != '/')
- return false;
- if (memchr(shorter->pattern, '/', shorter->length) != NULL)
- return false;
+ if (p[-1] != '/')
+ return false;
+ if (memchr(shorter->pattern, '/', shorter->length) != NULL)
+ return false;
- return memcmp(p, shorter->pattern, shorter->length) == 0;
- }
-
- return false;
+ return cmp(p, shorter->pattern, shorter->length) == 0;
}
/**
@@ -97,7 +101,7 @@ static int does_negate_pattern(git_attr_fnmatch *rule, git_attr_fnmatch *neg)
*/
static int does_negate_rule(int *out, git_vector *rules, git_attr_fnmatch *match)
{
- int error = 0;
+ int error = 0, fnflags;
size_t i;
git_attr_fnmatch *rule;
char *path;
@@ -105,6 +109,10 @@ static int does_negate_rule(int *out, git_vector *rules, git_attr_fnmatch *match
*out = 0;
+ fnflags = FNM_PATHNAME;
+ if (match->flags & GIT_ATTR_FNMATCH_ICASE)
+ fnflags |= FNM_IGNORECASE;
+
/* path of the file relative to the workdir, so we match the rules in subdirs */
if (match->containing_dir) {
git_buf_puts(&buf, match->containing_dir);
@@ -125,12 +133,12 @@ static int does_negate_rule(int *out, git_vector *rules, git_attr_fnmatch *match
continue;
}
- /*
- * When dealing with a directory, we add '/<star>' so
- * p_fnmatch() honours FNM_PATHNAME. Checking for LEADINGDIR
- * alone isn't enough as that's also set for nagations, so we
- * need to check that NEGATIVE is off.
- */
+ /*
+ * When dealing with a directory, we add '/<star>' so
+ * p_fnmatch() honours FNM_PATHNAME. Checking for LEADINGDIR
+ * alone isn't enough as that's also set for nagations, so we
+ * need to check that NEGATIVE is off.
+ */
git_buf_clear(&buf);
if (rule->containing_dir) {
git_buf_puts(&buf, rule->containing_dir);
@@ -144,7 +152,7 @@ static int does_negate_rule(int *out, git_vector *rules, git_attr_fnmatch *match
if (error < 0)
goto out;
- if ((error = p_fnmatch(git_buf_cstr(&buf), path, FNM_PATHNAME)) < 0) {
+ if ((error = p_fnmatch(git_buf_cstr(&buf), path, fnflags)) < 0) {
giterr_set(GITERR_INVALID, "error matching pattern");
goto out;
}
@@ -207,8 +215,14 @@ static int parse_ignore_file(
scan = git__next_line(scan);
- /* if a negative match doesn't actually do anything, throw it away */
- if (match->flags & GIT_ATTR_FNMATCH_NEGATIVE)
+ /*
+ * If a negative match doesn't actually do anything,
+ * throw it away. As we cannot always verify whether a
+ * rule containing wildcards negates another rule, we
+ * do not optimize away these rules, though.
+ * */
+ if (match->flags & GIT_ATTR_FNMATCH_NEGATIVE
+ && !(match->flags & GIT_ATTR_FNMATCH_HASWILD))
error = does_negate_rule(&valid_rule, &attrs->rules, match);
if (!error && valid_rule)
diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c
index a089ee4..856e61f 100644
--- a/tests/attr/ignore.c
+++ b/tests/attr/ignore.c
@@ -303,3 +303,46 @@ void test_attr_ignore__test(void)
assert_is_ignored(true, "dist/foo.o");
assert_is_ignored(true, "bin/foo");
}
+
+void test_attr_ignore__unignore_dir_succeeds(void)
+{
+ cl_git_rewritefile("attr/.gitignore",
+ "*.c\n"
+ "!src/*.c\n");
+ assert_is_ignored(false, "src/foo.c");
+ assert_is_ignored(true, "src/foo/foo.c");
+}
+
+void test_attr_ignore__case_insensitive_unignores_previous_rule(void)
+{
+ git_config *cfg;
+
+ cl_git_rewritefile("attr/.gitignore",
+ "/case\n"
+ "!/Case/\n");
+
+ cl_git_pass(git_repository_config(&cfg, g_repo));
+ cl_git_pass(git_config_set_bool(cfg, "core.ignorecase", true));
+
+ cl_must_pass(p_mkdir("attr/case", 0755));
+ cl_git_mkfile("attr/case/file", "content");
+
+ assert_is_ignored(false, "case/file");
+}
+
+void test_attr_ignore__case_sensitive_unignore_does_nothing(void)
+{
+ git_config *cfg;
+
+ cl_git_rewritefile("attr/.gitignore",
+ "/case\n"
+ "!/Case/\n");
+
+ cl_git_pass(git_repository_config(&cfg, g_repo));
+ cl_git_pass(git_config_set_bool(cfg, "core.ignorecase", false));
+
+ cl_must_pass(p_mkdir("attr/case", 0755));
+ cl_git_mkfile("attr/case/file", "content");
+
+ assert_is_ignored(true, "case/file");
+}
diff --git a/tests/status/ignore.c b/tests/status/ignore.c
index 23384fb..dc58e8b 100644
--- a/tests/status/ignore.c
+++ b/tests/status/ignore.c
@@ -1155,3 +1155,30 @@ void test_status_ignore__subdir_ignore_everything_except_certain_files(void)
refute_is_ignored("project/src/foo.c");
refute_is_ignored("project/src/foo/foo.c");
}
+
+void test_status_ignore__deeper(void)
+{
+ int ignored;
+
+ g_repo = cl_git_sandbox_init("empty_standard_repo");
+
+ cl_git_mkfile("empty_standard_repo/.gitignore",
+ "*.data\n"
+ "!dont_ignore/*.data\n");
+
+ cl_git_pass(p_mkdir("empty_standard_repo/dont_ignore", 0777));
+ cl_git_mkfile("empty_standard_repo/foo.data", "");
+ cl_git_mkfile("empty_standard_repo/bar.data", "");
+ cl_git_mkfile("empty_standard_repo/dont_ignore/foo.data", "");
+ cl_git_mkfile("empty_standard_repo/dont_ignore/bar.data", "");
+
+ cl_git_pass(git_ignore_path_is_ignored(&ignored, g_repo, "foo.data"));
+ cl_assert_equal_i(1, ignored);
+ cl_git_pass(git_ignore_path_is_ignored(&ignored, g_repo, "bar.data"));
+ cl_assert_equal_i(1, ignored);
+
+ cl_git_pass(git_ignore_path_is_ignored(&ignored, g_repo, "dont_ignore/foo.data"));
+ cl_assert_equal_i(0, ignored);
+ cl_git_pass(git_ignore_path_is_ignored(&ignored, g_repo, "dont_ignore/bar.data"));
+ cl_assert_equal_i(0, ignored);
+}