{"thread":{"id":"40393","subject":"[PATCH v3 0/2] gitignore, re-inclusion fix","startedAt":"2015-09-21T09:56:13Z","lastAt":"2015-09-21T18:03:17Z","messageCount":4,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"270386","messageId":"1442829375-1926-1-git-send-email-pclouds@gmail.com","threadId":"40393","inReplyTo":"1442106945-24080-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 0/2] gitignore, re-inclusion fix","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-09-21T09:56:13Z","receivedAt":"2015-09-21T09:56:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Changes since v2\n\n-- 8< --\ndiff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\nindex 889a72a..79a1948 100644\n--- a/Documentation/gitignore.txt\n+++ b/Documentation/gitignore.txt\n@@ -82,9 +82,12 @@ PATTERN FORMAT\n \n  - An optional prefix \"`!`\" which negates the pattern; any\n    matching file excluded by a previous pattern will become\n-   included again. It is possible to re-include a file if a parent\n-   directory of that file is excluded, with restrictions. See section\n-   NOTES for detail.\n+   included again.\n+   Put a backslash (\"`\\`\") in front of the first \"`!`\" for patterns\n+   that begin with a literal \"`!`\", for example, \"`\\!important!.txt`\".\n+   It is possible to re-include a file if a parent directory of that\n+   file is excluded if certain conditions are met. See section NOTES\n+   for detail.\n \n  - If the pattern ends with a slash, it is removed for the\n    purpose of the following description, but it would only find\n@@ -138,8 +141,11 @@ not tracked by Git remain untracked.\n To stop tracking a file that is currently tracked, use\n 'git rm --cached'.\n \n-To re-include a file when its parent directory is excluded, the\n-following conditions must be met:\n+To re-include files or directories when their parent directory is\n+excluded, the following conditions must be met:\n+\n+ - The rules to exclude a directory and re-include a subset back must\n+   be in the same .gitignore file.\n \n  - The directory part in the re-include rules must be literal (i.e. no\n    wildcards)\ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex 9de49a6..da257c0 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -310,9 +310,14 @@ test_expect_success 'negative patterns' '\n \t(\n \t\tcd reinclude &&\n \t\tcat >.gitignore <<-\\EOF &&\n+\t\t/fooo\n \t\t/foo\n \t\t!foo/bar/bar\n \t\tEOF\n+\t\tmkdir fooo &&\n+\t\tcat >fooo/.gitignore <<-\\EOF &&\n+\t\t!/*\n+\t\tEOF\n \t\tmkdir -p foo/bar &&\n \t\ttouch abc foo/def foo/bar/ghi foo/bar/bar &&\n \t\tgit ls-files -o --exclude-standard >../actual &&\n-- 8< --\n\nNguyễn Thái Ngọc Duy (2):\n  dir.c: make last_exclude_matching_from_list() run til the end\n  dir.c: don't exclude whole dir prematurely if neg pattern may match\n\n Documentation/gitignore.txt        | 23 ++++++++--\n dir.c                              | 89 +++++++++++++++++++++++++++++++++++---\n t/t3001-ls-files-others-exclude.sh | 25 +++++++++++\n 3 files changed, 127 insertions(+), 10 deletions(-)\n\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"270387","messageId":"1442829375-1926-2-git-send-email-pclouds@gmail.com","threadId":"40393","inReplyTo":"1442829375-1926-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 1/2] dir.c: make last_exclude_matching_from_list() run til the end","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-09-21T09:56:14Z","receivedAt":"2015-09-21T09:56:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The next patch adds some post processing to the result value before it's\nreturned to the caller. Keep all branches reach the end of the function,\nso we can do it all in one place.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n dir.c | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex c00c7e2..3a7630a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -901,6 +901,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\t\t\t\t       int *dtype,\n \t\t\t\t\t\t       struct exclude_list *el)\n {\n+\tstruct exclude *exc = NULL; /* undecided */\n \tint i;\n \n \tif (!el->nr)\n@@ -922,18 +923,22 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\tif (match_basename(basename,\n \t\t\t\t\t   pathlen - (basename - pathname),\n \t\t\t\t\t   exclude, prefix, x->patternlen,\n-\t\t\t\t\t   x->flags))\n-\t\t\t\treturn x;\n+\t\t\t\t\t   x->flags)) {\n+\t\t\t\texc = x;\n+\t\t\t\tbreak;\n+\t\t\t}\n \t\t\tcontinue;\n \t\t}\n \n \t\tassert(x->baselen == 0 || x->base[x->baselen - 1] == '/');\n \t\tif (match_pathname(pathname, pathlen,\n \t\t\t\t   x->base, x->baselen ? x->baselen - 1 : 0,\n-\t\t\t\t   exclude, prefix, x->patternlen, x->flags))\n-\t\t\treturn x;\n+\t\t\t\t   exclude, prefix, x->patternlen, x->flags)) {\n+\t\t\texc = x;\n+\t\t\tbreak;\n+\t\t}\n \t}\n-\treturn NULL; /* undecided */\n+\treturn exc;\n }\n \n /*\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"270388","messageId":"1442829375-1926-3-git-send-email-pclouds@gmail.com","threadId":"40393","inReplyTo":"1442829375-1926-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 2/2] dir.c: don't exclude whole dir prematurely if neg pattern may match","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-09-21T09:56:15Z","receivedAt":"2015-09-21T09:56:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"If there is a pattern \"!foo/bar\", this patch makes it not exclude \"foo\"\nright away. This gives us a chance to examine \"foo\" and re-include\n\"foo/bar\".\n\nIn order for it to detect that the directory under examination should\nnot be excluded right away, in other words it is a parent directory of a\nnegative pattern, the \"directory path\" of the negative pattern must be\nliteral. Patterns like \"!f?o/bar\" can't stop \"foo\" from being excluded.\n\nBasename matching (i.e. \"no slashes in the pattern\") or must-be-dir\nmatching (i.e. \"trailing slash in the pattern\") does not work well with\nthis. For example, if we descend in \"foo\" and are examining \"foo/abc\",\ncurrent code for \"foo/\" pattern will check if path \"foo/abc\", not \"foo\",\nis a directory. The same problem with basename matching. These may need\nbig code reorg to make it work.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/gitignore.txt        | 23 ++++++++++--\n dir.c                              | 76 +++++++++++++++++++++++++++++++++++++-\n t/t3001-ls-files-others-exclude.sh | 25 +++++++++++++\n 3 files changed, 118 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\nindex 473623d..79a1948 100644\n--- a/Documentation/gitignore.txt\n+++ b/Documentation/gitignore.txt\n@@ -82,12 +82,12 @@ PATTERN FORMAT\n \n  - An optional prefix \"`!`\" which negates the pattern; any\n    matching file excluded by a previous pattern will become\n-   included again. It is not possible to re-include a file if a parent\n-   directory of that file is excluded. Git doesn't list excluded\n-   directories for performance reasons, so any patterns on contained\n-   files have no effect, no matter where they are defined.\n+   included again.\n    Put a backslash (\"`\\`\") in front of the first \"`!`\" for patterns\n    that begin with a literal \"`!`\", for example, \"`\\!important!.txt`\".\n+   It is possible to re-include a file if a parent directory of that\n+   file is excluded if certain conditions are met. See section NOTES\n+   for detail.\n \n  - If the pattern ends with a slash, it is removed for the\n    purpose of the following description, but it would only find\n@@ -141,6 +141,21 @@ not tracked by Git remain untracked.\n To stop tracking a file that is currently tracked, use\n 'git rm --cached'.\n \n+To re-include files or directories when their parent directory is\n+excluded, the following conditions must be met:\n+\n+ - The rules to exclude a directory and re-include a subset back must\n+   be in the same .gitignore file.\n+\n+ - The directory part in the re-include rules must be literal (i.e. no\n+   wildcards)\n+\n+ - The rules to exclude the parent directory must not end with a\n+   trailing slash.\n+\n+ - The rules to exclude the parent directory must have at least one\n+   slash.\n+\n EXAMPLES\n --------\n \ndiff --git a/dir.c b/dir.c\nindex 3a7630a..a1f711c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -882,6 +882,25 @@ int match_pathname(const char *pathname, int pathlen,\n \t\t */\n \t\tif (!patternlen && !namelen)\n \t\t\treturn 1;\n+\t\t/*\n+\t\t * This can happen when we ignore some exclude rules\n+\t\t * on directories in other to see if negative rules\n+\t\t * may match. E.g.\n+\t\t *\n+\t\t * /abc\n+\t\t * !/abc/def/ghi\n+\t\t *\n+\t\t * The pattern of interest is \"/abc\". On the first\n+\t\t * try, we should match path \"abc\" with this pattern\n+\t\t * in the \"if\" statement right above, but the caller\n+\t\t * ignores it.\n+\t\t *\n+\t\t * On the second try with paths within \"abc\",\n+\t\t * e.g. \"abc/xyz\", we come here and try to match it\n+\t\t * with \"/abc\".\n+\t\t */\n+\t\tif (!patternlen && namelen && *name == '/')\n+\t\t\treturn 1;\n \t}\n \n \treturn fnmatch_icase_mem(pattern, patternlen,\n@@ -890,6 +909,48 @@ int match_pathname(const char *pathname, int pathlen,\n }\n \n /*\n+ * Return non-zero if pathname is a directory and an ancestor of the\n+ * literal path in a (negative) pattern. This is used to keep\n+ * descending in \"foo\" and \"foo/bar\" when the pattern is\n+ * \"!foo/bar/.gitignore\". \"foo/notbar\" will not be descended however.\n+ */\n+static int match_neg_path(const char *pathname, int pathlen, int *dtype,\n+\t\t\t  const char *base, int baselen,\n+\t\t\t  const char *pattern, int prefix, int patternlen,\n+\t\t\t  int flags)\n+{\n+\tassert((flags & EXC_FLAG_NEGATIVE) && !(flags & EXC_FLAG_NODIR));\n+\n+\tif (*dtype == DT_UNKNOWN)\n+\t\t*dtype = get_dtype(NULL, pathname, pathlen);\n+\tif (*dtype != DT_DIR)\n+\t\treturn 0;\n+\n+\tif (*pattern == '/') {\n+\t\tpattern++;\n+\t\tpatternlen--;\n+\t\tprefix--;\n+\t}\n+\n+\tif (baselen) {\n+\t\tif (((pathlen < baselen && base[pathlen] == '/') ||\n+\t\t     pathlen == baselen) &&\n+\t\t    !strncmp_icase(pathname, base, pathlen))\n+\t\t\treturn 1;\n+\t\tpathname += baselen + 1;\n+\t\tpathlen  -= baselen + 1;\n+\t}\n+\n+\n+\tif (prefix &&\n+\t    ((pathlen < prefix && pattern[pathlen] == '/') &&\n+\t     !strncmp_icase(pathname, pattern, pathlen)))\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+/*\n  * Scan the given exclude list in reverse to see whether pathname\n  * should be ignored.  The first match (i.e. the last on the list), if\n  * any, determines the fate.  Returns the exclude_list element which\n@@ -902,7 +963,7 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\t\t\t\t       struct exclude_list *el)\n {\n \tstruct exclude *exc = NULL; /* undecided */\n-\tint i;\n+\tint i, matched_negative_path = 0;\n \n \tif (!el->nr)\n \t\treturn NULL;\t/* undefined */\n@@ -937,7 +998,18 @@ static struct exclude *last_exclude_matching_from_list(const char *pathname,\n \t\t\texc = x;\n \t\t\tbreak;\n \t\t}\n-\t}\n+\n+\t\tif ((x->flags & EXC_FLAG_NEGATIVE) && !matched_negative_path &&\n+\t\t    match_neg_path(pathname, pathlen, dtype, x->base,\n+\t\t\t\t   x->baselen ? x->baselen - 1 : 0,\n+\t\t\t\t   exclude, prefix, x->patternlen, x->flags))\n+\t\t\tmatched_negative_path = 1;\n+\t}\n+\tif (exc &&\n+\t    !(exc->flags & EXC_FLAG_NEGATIVE) &&\n+\t    !(exc->flags & EXC_FLAG_NODIR) &&\n+\t    matched_negative_path)\n+\t\texc = NULL;\n \treturn exc;\n }\n \ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex 3fc484e..da257c0 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -305,4 +305,29 @@ test_expect_success 'ls-files with \"**\" patterns and no slashes' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'negative patterns' '\n+\tgit init reinclude &&\n+\t(\n+\t\tcd reinclude &&\n+\t\tcat >.gitignore <<-\\EOF &&\n+\t\t/fooo\n+\t\t/foo\n+\t\t!foo/bar/bar\n+\t\tEOF\n+\t\tmkdir fooo &&\n+\t\tcat >fooo/.gitignore <<-\\EOF &&\n+\t\t!/*\n+\t\tEOF\n+\t\tmkdir -p foo/bar &&\n+\t\ttouch abc foo/def foo/bar/ghi foo/bar/bar &&\n+\t\tgit ls-files -o --exclude-standard >../actual &&\n+\t\tcat >../expected <<-\\EOF &&\n+\t\t.gitignore\n+\t\tabc\n+\t\tfoo/bar/bar\n+\t\tEOF\n+\t\ttest_cmp ../expected ../actual\n+\t)\n+'\n+\n test_done\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"270414","messageId":"xmqqmvwfabve.fsf@gitster.mtv.corp.google.com","threadId":"40393","inReplyTo":"1442829375-1926-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 0/2] gitignore, re-inclusion fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-21T18:03:17Z","receivedAt":"2015-09-21T18:03:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This round is very clearly explained and I like it very much ;-)\n\nThanks for fixing this.\n"}]}