{"thread":{"id":"27237","subject":"[PATCH 1/3] t3700: note a .gitignore matching fault","startedAt":"2011-05-02T12:47:42Z","lastAt":"2011-05-11T12:03:18Z","messageCount":18,"participants":["Nguyễn Thái Ngọc Duy","Johannes Sixt","Nguyen Thai Ngoc Duy","Thiago Farina","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"166856","messageId":"1304340464-14829-1-git-send-email-pclouds@gmail.com","threadId":"27237","inReplyTo":null,"subject":"[PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-02T12:47:42Z","receivedAt":"2011-05-02T12:47:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":".gitignore support both positive and negative patterns. One may negate\nthe other. Current code works well if both patterns target files in\nthe same directory.\n\nWhen a pattern targets a directory and an opposite pattern targets\nsome files/directories within that directory, we need to descend in\nthe directory until we're clear which ones are matched and which are\nnot.\n\nexcluded_from_list() (or its callers) fails to handle this case. It\ntoo eagerly decides the fate of the whole directory without looking\nfurther in.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Haven't figured out how to cleanly fix this yet, unfortunately.\n\n t/t3700-add.sh | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 7de42fa..6c3eed6 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -295,4 +295,20 @@ test_expect_success C_LOCALE_OUTPUT 'git add --dry-run --ignore-missing of non-e\n \ttest_cmp expect.err actual.err\n '\n \n+cat >expected <test/.gitignore <actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.7.4.74.g639db\n"},{"id":"166857","messageId":"1304340464-14829-2-git-send-email-pclouds@gmail.com","threadId":"27237","inReplyTo":"1304340464-14829-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/3] t1011: fix sparse-checkout initialization and add new file","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-02T12:47:43Z","receivedAt":"2011-05-02T12:47:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Do not append to $GIT_DIR/info/sparse-checkout at each test, overwrite\nit instead.\n\nAlso add sub/addedtoo for more complex tests later on\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t1011-read-tree-sparse-checkout.sh | 12 ++++++++----\n 1 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex de84e35..3f9d66f 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -17,19 +17,21 @@ test_expect_success 'setup' '\n \tcat >expected <<-\\EOF &&\n \t100644 77f0ba1734ed79d12881f81b36ee134de6a3327b 0\tinit.t\n \t100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tsub/added\n+\t100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tsub/addedtoo\n \t100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tsubsub/added\n \tEOF\n \tcat >expected.swt <<-\\EOF &&\n \tH init.t\n \tH sub/added\n+\tH sub/addedtoo\n \tH subsub/added\n \tEOF\n \n \ttest_commit init &&\n \techo modified >>init.t &&\n \tmkdir sub subsub &&\n-\ttouch sub/added subsub/added &&\n-\tgit add init.t sub/added subsub/added &&\n+\ttouch sub/added sub/addedtoo subsub/added &&\n+\tgit add init.t sub/added sub/addedtoo subsub/added &&\n \tgit commit -m \"modified and added\" &&\n \tgit tag top &&\n \tgit rm sub/added &&\n@@ -83,6 +85,7 @@ test_expect_success 'match directories with trailing slash' '\n \tcat >expected.swt-noinit <<-\\EOF &&\n \tS init.t\n \tH sub/added\n+\tH sub/addedtoo\n \tS subsub/added\n \tEOF\n \n@@ -95,7 +98,7 @@ test_expect_success 'match directories with trailing slash' '\n '\n \n test_expect_success 'match directories without trailing slash' '\n-\techo sub >>.git/info/sparse-checkout &&\n+\techo sub >.git/info/sparse-checkout &&\n \tgit read-tree -m -u HEAD &&\n \tgit ls-files -t >result &&\n \ttest_cmp expected.swt-noinit result &&\n@@ -104,7 +107,7 @@ test_expect_success 'match directories without trailing slash' '\n '\n \n test_expect_success 'match directory pattern' '\n-\techo \"s?b\" >>.git/info/sparse-checkout &&\n+\techo \"s?b\" >.git/info/sparse-checkout &&\n \tgit read-tree -m -u HEAD &&\n \tgit ls-files -t >result &&\n \ttest_cmp expected.swt-noinit result &&\n@@ -116,6 +119,7 @@ test_expect_success 'checkout area changes' '\n \tcat >expected.swt-nosub <<-\\EOF &&\n \tH init.t\n \tS sub/added\n+\tS sub/addedtoo\n \tS subsub/added\n \tEOF\n \n-- \n1.7.4.74.g639db\n"},{"id":"166858","messageId":"1304340464-14829-3-git-send-email-pclouds@gmail.com","threadId":"27237","inReplyTo":"1304340464-14829-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/3] sparse checkout: do not eagerly decide the fate for whole directory","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-02T12:47:44Z","receivedAt":"2011-05-02T12:47:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Sparse-setting code follows closely how files are excluded in\nread_directory(), every entry (including directories) are fed to\nexcluded_from_list() to decide if the entry is suitable. Directories\nare treated no different than files. If a directory is matched (or\nnot), the whole directory is considered matched (or not) and the\nprocess moves on.\n\nThis generally works as long as there are no patterns to exclude parts\nof the directory. In case of sparse checkout code, the following patterns\n\n t\n !t/t0000-basic.sh\n\nwill produce a worktree with full directory \"t\" even if t0000-basic.sh\nis requested to stay out.\n\nBy the same reasoning, if a directory is to be excluded, any rules to\nre-include certain files within that directory will be ignored.\n\nFix it by always checking files against patterns. If no pattern can\ndecide (ie. excluded_from_list() returns -1), those files will be\nincluded/excluded as same as their parent directory.\n\nNoticed-by: \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n The TODO better be solved once read_directory() fixes the same fault.\n I have a feeling that some code can be shared..\n\n t/t1011-read-tree-sparse-checkout.sh | 41 ++++++++++++++++++++++\n unpack-trees.c | 63 ++++++++++++++++++---------------\n 2 files changed, 75 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\nindex 3f9d66f..20a50eb 100755\n--- a/t/t1011-read-tree-sparse-checkout.sh\n+++ b/t/t1011-read-tree-sparse-checkout.sh\n@@ -106,6 +106,47 @@ test_expect_success 'match directories without trailing slash' '\n \ttest -f sub/added\n '\n \n+test_expect_success 'match directories with negated patterns' '\n+\tcat >expected.swt-negation <<\\EOF &&\n+S init.t\n+S sub/added\n+H sub/addedtoo\n+S subsub/added\n+EOF\n+\n+\tcat >.git/info/sparse-checkout <<\\EOF &&\n+sub\n+!sub/added\n+EOF\n+\tgit read-tree -m -u HEAD &&\n+\tgit ls-files -t >result &&\n+\ttest_cmp expected.swt-negation result &&\n+\ttest ! -f init.t &&\n+\ttest ! -f sub/added &&\n+\ttest -f sub/addedtoo\n+'\n+\n+test_expect_success 'match directories with negated patterns (2)' '\n+\tcat >expected.swt-negation2 <<\\EOF &&\n+H init.t\n+H sub/added\n+S sub/addedtoo\n+H subsub/added\n+EOF\n+\n+\tcat >.git/info/sparse-checkout <<\\EOF &&\n+/*\n+!sub\n+sub/added\n+EOF\n+\tgit read-tree -m -u HEAD &&\n+\tgit ls-files -t >result &&\n+\ttest_cmp expected.swt-negation2 result &&\n+\ttest -f init.t &&\n+\ttest -f sub/added &&\n+\ttest ! -f sub/addedtoo\n+'\n+\n test_expect_success 'match directory pattern' '\n \techo \"s?b\" >.git/info/sparse-checkout &&\n \tgit read-tree -m -u HEAD &&\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 500ebcf..5b8419c 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -814,43 +814,45 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n \treturn mask;\n }\n \n+static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n+\t\t\t char *prefix, int prefix_len,\n+\t\t\t int select_mask, int clear_mask,\n+\t\t\t struct exclude_list *el, int defval);\n+\n /* Whole directory matching */\n static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n \t\t\t char *prefix, int prefix_len,\n \t\t\t char *basename,\n \t\t\t int select_mask, int clear_mask,\n-\t\t\t struct exclude_list *el)\n+\t\t\t struct exclude_list *el, int defval)\n {\n-\tstruct cache_entry **cache_end = cache + nr;\n+\tstruct cache_entry **cache_end;\n \tint dtype = DT_DIR;\n \tint ret = excluded_from_list(prefix, prefix_len, basename, &dtype, el);\n \n \tprefix[prefix_len++] = '/';\n \n-\t/* included, no clearing for any entries under this directory */\n-\tif (!ret) {\n-\t\tfor (; cache != cache_end; cache++) {\n-\t\t\tstruct cache_entry *ce = *cache;\n-\t\t\tif (strncmp(ce->name, prefix, prefix_len))\n-\t\t\t\tbreak;\n-\t\t}\n-\t\treturn nr - (cache_end - cache);\n-\t}\n+\t/* If undecided, use parent directory's decision in defval */\n+\tif (ret < 0)\n+\t\tret = defval;\n \n-\t/* excluded, clear all selected entries under this directory. */\n-\tif (ret == 1) {\n-\t\tfor (; cache != cache_end; cache++) {\n-\t\t\tstruct cache_entry *ce = *cache;\n-\t\t\tif (select_mask && !(ce->ce_flags & select_mask))\n-\t\t\t\tcontinue;\n-\t\t\tif (strncmp(ce->name, prefix, prefix_len))\n-\t\t\t\tbreak;\n-\t\t\tce->ce_flags &= ~clear_mask;\n-\t\t}\n-\t\treturn nr - (cache_end - cache);\n+\tfor (cache_end = cache; cache_end != cache + nr; cache_end++) {\n+\t\tstruct cache_entry *ce = *cache_end;\n+\t\tif (strncmp(ce->name, prefix, prefix_len))\n+\t\t\tbreak;\n \t}\n \n-\treturn 0;\n+\t/*\n+\t * TODO: check el, if there are no patterns that may conflict\n+\t * with ret (iow, we know in advance the the incl/excl\n+\t * decision for the entire directory), clear flag here without\n+\t * calling clear_ce_flags_1(). That function will call\n+\t * the expensive excluded_from_list() on every entry.\n+\t */\n+\treturn clear_ce_flags_1(cache, cache_end - cache,\n+\t\t\t\tprefix, prefix_len,\n+\t\t\t\tselect_mask, clear_mask,\n+\t\t\t\tel, ret);\n }\n \n /*\n@@ -871,7 +873,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \t\t\t char *prefix, int prefix_len,\n \t\t\t int select_mask, int clear_mask,\n-\t\t\t struct exclude_list *el)\n+\t\t\t struct exclude_list *el, int defval)\n {\n \tstruct cache_entry **cache_end = cache + nr;\n \n@@ -882,7 +884,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \twhile(cache != cache_end) {\n \t\tstruct cache_entry *ce = *cache;\n \t\tconst char *name, *slash;\n-\t\tint len, dtype;\n+\t\tint len, dtype, ret;\n \n \t\tif (select_mask && !(ce->ce_flags & select_mask)) {\n \t\t\tcache++;\n@@ -911,7 +913,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \t\t\t\t\t\t prefix, prefix_len + len,\n \t\t\t\t\t\t prefix + prefix_len,\n \t\t\t\t\t\t select_mask, clear_mask,\n-\t\t\t\t\t\t el);\n+\t\t\t\t\t\t el, defval);\n \n \t\t\t/* clear_c_f_dir eats a whole dir already? */\n \t\t\tif (processed) {\n@@ -922,13 +924,16 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \t\t\tprefix[prefix_len + len++] = '/';\n \t\t\tcache += clear_ce_flags_1(cache, cache_end - cache,\n \t\t\t\t\t\t prefix, prefix_len + len,\n-\t\t\t\t\t\t select_mask, clear_mask, el);\n+\t\t\t\t\t\t select_mask, clear_mask, el, defval);\n \t\t\tcontinue;\n \t\t}\n \n \t\t/* Non-directory */\n \t\tdtype = ce_to_dtype(ce);\n-\t\tif (excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, el) > 0)\n+\t\tret = excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, el);\n+\t\tif (ret < 0)\n+\t\t\tret = defval;\n+\t\tif (ret > 0)\n \t\t\tce->ce_flags &= ~clear_mask;\n \t\tcache++;\n \t}\n@@ -943,7 +948,7 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,\n \treturn clear_ce_flags_1(cache, nr,\n \t\t\t\tprefix, 0,\n \t\t\t\tselect_mask, clear_mask,\n-\t\t\t\tel);\n+\t\t\t\tel, 0);\n }\n \n /*\n-- \n1.7.4.74.g639db\n"},{"id":"166859","messageId":"1304340923-15927-1-git-send-email-pclouds@gmail.com","threadId":"27237","inReplyTo":"1304340464-14829-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-02T12:55:23Z","receivedAt":"2011-05-02T12:55:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":".gitignore support both positive and negative patterns. One may negate\nthe other. Current code works well if both patterns target files in\nthe same directory.\n\nWhen a pattern targets a directory and an opposite pattern targets\nsome files/directories within that directory, we need to descend in\nthe directory until we're clear which ones are matched and which are\nnot.\n\nexcluded_from_list() fails to handle this case. It too eagerly decides\nthe fate of the whole directory without looking further in.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Sorry, forgot the test name.\n\n t/t3700-add.sh | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 7de42fa..c9f3a28 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -295,4 +295,20 @@ test_expect_success C_LOCALE_OUTPUT 'git add --dry-run --ignore-missing of non-e\n \ttest_cmp expect.err actual.err\n '\n \n+cat >expected <test/.gitignore <actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.7.4.74.g639db\n"},{"id":"166873","messageId":"4DBEC766.8020008@viscovery.net","threadId":"27237","inReplyTo":"1304340923-15927-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-05-02T15:01:58Z","receivedAt":"2011-05-02T15:01:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/2/2011 14:55, schrieb Nguyễn Thái Ngọc Duy:\n> .gitignore support both positive and negative patterns. One may negate\n> the other. Current code works well if both patterns target files in\n> the same directory.\n> \n> When a pattern targets a directory and an opposite pattern targets\n> some files/directories within that directory, we need to descend in\n> the directory until we're clear which ones are matched and which are\n> not.\n> \n> excluded_from_list() fails to handle this case. It too eagerly decides\n> the fate of the whole directory without looking further in.\n\nThis has been debated just recently, and I don't think the current\nbehavior is broken. See\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/169913\nhttp://thread.gmane.org/gmane.comp.version-control.git/157190/focus=157196\n\n-- Hannes\n"},{"id":"166875","messageId":"BANLkTik=GYXUOeo3hA5yyXfOuBydT40koA@mail.gmail.com","threadId":"27237","inReplyTo":"4DBEC766.8020008@viscovery.net","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-02T15:52:37Z","receivedAt":"2011-05-02T15:52:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/5/2 Johannes Sixt :\n> Am 5/2/2011 14:55, schrieb Nguyễn Thái Ngọc Duy:\n>> excluded_from_list() fails to handle this case. It too eagerly decides\n>> the fate of the whole directory without looking further in.\n>\n> This has been debated just recently, and I don't think the current\n> behavior is broken. See\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/169913\n> http://thread.gmane.org/gmane.comp.version-control.git/157190/focus=157196\n\nBecause it's expensive to do does not make it right not to do it. I\nagree that traversing all the way down just to make the final decision\nis expensive. But we could at least state that we only support looking\nfor .gitignore down to some level (0 as in current git), or based on\n.gitignore paths in index, then stop. The point is make it\nconfigurable with sane default. It's up to users to decide how they\nwant to pay.\n-- \nDuy\n"},{"id":"166917","messageId":"BANLkTikgNR1G5_TO3rmMZy3fN2PNF2Pqjg@mail.gmail.com","threadId":"27237","inReplyTo":"1304340464-14829-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/3] sparse checkout: do not eagerly decide the fate for whole directory","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2011-05-03T02:14:21Z","receivedAt":"2011-05-03T02:14:21Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"2011/5/2 Nguyễn Thái Ngọc Duy :\n> Sparse-setting code follows closely how files are excluded in\n> read_directory(), every entry (including directories) are fed to\n> excluded_from_list() to decide if the entry is suitable. Directories\n> are treated no different than files. If a directory is matched (or\n> not), the whole directory is considered matched (or not) and the\n> process moves on.\n>\n> This generally works as long as there are no patterns to exclude parts\n> of the directory. In case of sparse checkout code, the following patterns\n>\n>  t\n>  !t/t0000-basic.sh\n>\n> will produce a worktree with full directory \"t\" even if t0000-basic.sh\n> is requested to stay out.\n>\n> By the same reasoning, if a directory is to be excluded, any rules to\n> re-include certain files within that directory will be ignored.\n>\n> Fix it by always checking files against patterns. If no pattern can\n> decide (ie. excluded_from_list() returns -1), those files will be\nMaybe: can be decided?\n\n> included/excluded as same as their parent directory.\n>\n> Noticed-by: \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n>  The TODO better be solved once read_directory() fixes the same fault.\n>  I have a feeling that some code can be shared..\n>\n>  t/t1011-read-tree-sparse-checkout.sh |   41 ++++++++++++++++++++++\n>  unpack-trees.c                       |   63 ++++++++++++++++++---------------\n>  2 files changed, 75 insertions(+), 29 deletions(-)\n>\n> diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh\n> index 3f9d66f..20a50eb 100755\n> --- a/t/t1011-read-tree-sparse-checkout.sh\n> +++ b/t/t1011-read-tree-sparse-checkout.sh\n> @@ -106,6 +106,47 @@ test_expect_success 'match directories without trailing slash' '\n>        test -f sub/added\n>  '\n>\n> +test_expect_success 'match directories with negated patterns' '\n> +       cat >expected.swt-negation <<\\EOF &&\n> +S init.t\n> +S sub/added\n> +H sub/addedtoo\n> +S subsub/added\n> +EOF\n> +\n> +       cat >.git/info/sparse-checkout <<\\EOF &&\n> +sub\n> +!sub/added\n> +EOF\n> +       git read-tree -m -u HEAD &&\n> +       git ls-files -t >result &&\n> +       test_cmp expected.swt-negation result &&\n> +       test ! -f init.t &&\n> +       test ! -f sub/added &&\n> +       test -f sub/addedtoo\n> +'\n> +\n> +test_expect_success 'match directories with negated patterns (2)' '\n> +       cat >expected.swt-negation2 <<\\EOF &&\n> +H init.t\n> +H sub/added\n> +S sub/addedtoo\n> +H subsub/added\n> +EOF\n> +\n> +       cat >.git/info/sparse-checkout <<\\EOF &&\n> +/*\n> +!sub\n> +sub/added\n> +EOF\n> +       git read-tree -m -u HEAD &&\n> +       git ls-files -t >result &&\n> +       test_cmp expected.swt-negation2 result &&\n> +       test -f init.t &&\n> +       test -f sub/added &&\n> +       test ! -f sub/addedtoo\n> +'\n> +\n>  test_expect_success 'match directory pattern' '\n>        echo \"s?b\" >.git/info/sparse-checkout &&\n>        git read-tree -m -u HEAD &&\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 500ebcf..5b8419c 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -814,43 +814,45 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str\n>        return mask;\n>  }\n>\n> +static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n> +                           char *prefix, int prefix_len,\n> +                           int select_mask, int clear_mask,\n> +                           struct exclude_list *el, int defval);\n> +\n>  /* Whole directory matching */\n>  static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n>                              char *prefix, int prefix_len,\n>                              char *basename,\n>                              int select_mask, int clear_mask,\n> -                             struct exclude_list *el)\n> +                             struct exclude_list *el, int defval)\n>  {\n> -       struct cache_entry **cache_end = cache + nr;\n> +       struct cache_entry **cache_end;\n>        int dtype = DT_DIR;\n>        int ret = excluded_from_list(prefix, prefix_len, basename, &dtype, el);\n>\n>        prefix[prefix_len++] = '/';\n>\n> -       /* included, no clearing for any entries under this directory */\n> -       if (!ret) {\n> -               for (; cache != cache_end; cache++) {\n> -                       struct cache_entry *ce = *cache;\n> -                       if (strncmp(ce->name, prefix, prefix_len))\n> -                               break;\n> -               }\n> -               return nr - (cache_end - cache);\n> -       }\n> +       /* If undecided, use parent directory's decision in defval */\nWhat means \"use parent directory's decision\"? Could you make this\ncomment more clearer?\n\n> +       if (ret < 0)\n> +               ret = defval;\n>\n> -       /* excluded, clear all selected entries under this directory. */\nStart with capital letter?\n\n> -       if (ret == 1) {\n> -               for (; cache != cache_end; cache++) {\n> -                       struct cache_entry *ce = *cache;\n> -                       if (select_mask && !(ce->ce_flags & select_mask))\n> -                               continue;\n> -                       if (strncmp(ce->name, prefix, prefix_len))\n> -                               break;\n> -                       ce->ce_flags &= ~clear_mask;\n> -               }\n> -               return nr - (cache_end - cache);\n> +       for (cache_end = cache; cache_end != cache + nr; cache_end++) {\n> +               struct cache_entry *ce = *cache_end;\n> +               if (strncmp(ce->name, prefix, prefix_len))\n> +                       break;\n>        }\n>\n> -       return 0;\n> +       /*\n> +        * TODO: check el, if there are no patterns that may conflict\n> +        * with ret (iow, we know in advance the the incl/excl\nthe the ?\n\n> +        * decision for the entire directory), clear flag here without\n> +        * calling clear_ce_flags_1(). That function will call\n> +        * the expensive excluded_from_list() on every entry.\n> +        */\n> +       return clear_ce_flags_1(cache, cache_end - cache,\n> +                               prefix, prefix_len,\n> +                               select_mask, clear_mask,\n> +                               el, ret);\n>  }\n>\n>  /*\n> @@ -871,7 +873,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n>  static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n>                            char *prefix, int prefix_len,\n>                            int select_mask, int clear_mask,\n> -                           struct exclude_list *el)\n> +                           struct exclude_list *el, int defval)\n>  {\n>        struct cache_entry **cache_end = cache + nr;\n>\n> @@ -882,7 +884,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n>        while(cache != cache_end) {\n>                struct cache_entry *ce = *cache;\n>                const char *name, *slash;\n> -               int len, dtype;\n> +               int len, dtype, ret;\n>\n>                if (select_mask && !(ce->ce_flags & select_mask)) {\n>                        cache++;\n> @@ -911,7 +913,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n>                                                       prefix, prefix_len + len,\n>                                                       prefix + prefix_len,\n>                                                       select_mask, clear_mask,\n> -                                                      el);\n> +                                                      el, defval);\n>\n>                        /* clear_c_f_dir eats a whole dir already? */\n>                        if (processed) {\n> @@ -922,13 +924,16 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n>                        prefix[prefix_len + len++] = '/';\n>                        cache += clear_ce_flags_1(cache, cache_end - cache,\n>                                                  prefix, prefix_len + len,\n> -                                                 select_mask, clear_mask, el);\n> +                                                 select_mask, clear_mask, el, defval);\n>                        continue;\n>                }\n>\n>                /* Non-directory */\n>                dtype = ce_to_dtype(ce);\n> -               if (excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, el) > 0)\n> +               ret = excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, el);\n> +               if (ret < 0)\n> +                       ret = defval;\n> +               if (ret > 0)\n>                        ce->ce_flags &= ~clear_mask;\n>                cache++;\n>        }\n> @@ -943,7 +948,7 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,\n>        return clear_ce_flags_1(cache, nr,\n>                                prefix, 0,\n>                                select_mask, clear_mask,\n> -                               el);\n> +                               el, 0);\n>  }\n>\n>  /*\n> --\n> 1.7.4.74.g639db\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"166918","messageId":"BANLkTimDO7yRyiBt7xS5Hy7-iT5SUDwJtA@mail.gmail.com","threadId":"27237","inReplyTo":"BANLkTikgNR1G5_TO3rmMZy3fN2PNF2Pqjg@mail.gmail.com","subject":"Re: [PATCH 3/3] sparse checkout: do not eagerly decide the fate for whole directory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-03T04:43:35Z","receivedAt":"2011-05-03T04:43:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/5/3 Thiago Farina :\n>> This generally works as long as there are no patterns to exclude parts\n>> of the directory. In case of sparse checkout code, the following patterns\n>>\n>>  t\n>>  !t/t0000-basic.sh\n>>\n>> will produce a worktree with full directory \"t\" even if t0000-basic.sh\n>> is requested to stay out.\n>> ...\n>>        prefix[prefix_len++] = '/';\n>>\n>> -       /* included, no clearing for any entries under this directory */\n>> -       if (!ret) {\n>> -               for (; cache != cache_end; cache++) {\n>> -                       struct cache_entry *ce = *cache;\n>> -                       if (strncmp(ce->name, prefix, prefix_len))\n>> -                               break;\n>> -               }\n>> -               return nr - (cache_end - cache);\n>> -       }\n>> +       /* If undecided, use parent directory's decision in defval */\n> What means \"use parent directory's decision\"? Could you make this\n> comment more clearer?\n\nTake the example patterns in commit message, we know that we match\ndirectory \"t\" (pattern 1). When we check t/0001-init.sh, no patterns\nmatch it. But because it's under \"t\", so we consider it matched. On\nthe other hand, t/t0000-basic.sh will match pattern 2 and override\nparent directory's decision.\n\n>> +       if (ret < 0)\n>> +               ret = defval;\n>>\n>> -       /* excluded, clear all selected entries under this directory. */\n> Start with capital letter?\n\nIt's a line removal, what can I do about it?\n-- \nDuy\n"},{"id":"166955","messageId":"7voc3jis61.fsf@alter.siamese.dyndns.org","threadId":"27237","inReplyTo":"BANLkTik=GYXUOeo3hA5yyXfOuBydT40koA@mail.gmail.com","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-03T17:56:54Z","receivedAt":"2011-05-03T17:56:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> ... The point is make it\n> configurable with sane default. It's up to users to decide how they\n> want to pay.\n\nHmm, I am confused.\n\nIsn't that what we already have? If you want to pay, you just do not\ndefine a do-not-descend ignore entry at such a location so close to the\nroot of the working tree.\n"},{"id":"166980","messageId":"BANLkTi=ZL5aPnPMXai==gyjYRT_LHovisA@mail.gmail.com","threadId":"27237","inReplyTo":"7voc3jis61.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-03T23:43:36Z","receivedAt":"2011-05-03T23:43:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, May 4, 2011 at 12:56 AM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>> ... The point is make it\n>> configurable with sane default. It's up to users to decide how they\n>> want to pay.\n>\n> Hmm, I am confused.\n>\n> Isn't that what we already have?  If you want to pay, you just do not\n> define a do-not-descend ignore entry at such a location so close to the\n> root of the working tree.\n\nYes. But git still silently ignores some rules in the .gitignore.\nThere's another case when you rather update $GIT_DIR/info/exclude than\n .gitignore in the tree (import tree for example). Same trap again.\n-- \nDuy\n"},{"id":"166982","messageId":"7vd3jzgww8.fsf@alter.siamese.dyndns.org","threadId":"27237","inReplyTo":"BANLkTi=ZL5aPnPMXai==gyjYRT_LHovisA@mail.gmail.com","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-03T23:57:43Z","receivedAt":"2011-05-03T23:57:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> Yes. But git still silently ignores some rules in the .gitignore.\n\nDo you want git to report each and every entry in .gitignore saying \"this\nrule does not apply\"? That sounds like madness to me.\n"},{"id":"166983","messageId":"BANLkTinSx=MAkF5mt8gReNd1qEaeFTEBpA@mail.gmail.com","threadId":"27237","inReplyTo":"7vd3jzgww8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-04T00:41:21Z","receivedAt":"2011-05-04T00:41:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, May 4, 2011 at 6:57 AM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>> Yes. But git still silently ignores some rules in the .gitignore.\n>\n> Do you want git to report each and every entry in .gitignore saying \"this\n> rule does not apply\"?  That sounds like madness to me.\n\nThis rule should apply, but not because of \"efficiency reasons\". Not\njust about any rule.\n-- \nDuy\n"},{"id":"166984","messageId":"BANLkTi=TNjUaD4EP8sYXDqJ-tmBEzz4t4w@mail.gmail.com","threadId":"27237","inReplyTo":"BANLkTinSx=MAkF5mt8gReNd1qEaeFTEBpA@mail.gmail.com","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-04T01:05:08Z","receivedAt":"2011-05-04T01:05:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, May 4, 2011 at 7:41 AM, Nguyen Thai Ngoc Duy wrote:\n> On Wed, May 4, 2011 at 6:57 AM, Junio C Hamano wrote:\n>> Nguyen Thai Ngoc Duy writes:\n>>\n>>> Yes. But git still silently ignores some rules in the .gitignore.\n>>\n>> Do you want git to report each and every entry in .gitignore saying \"this\n>> rule does not apply\"?  That sounds like madness to me.\n>\n> This rule should apply, but not because of \"efficiency reasons\". Not\n> just about any rule.\n\nMaybe something like this instead of a implemantation fix?\n\ndiff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\nindex 8416f34..81e9d43 100644\n--- a/Documentation/gitignore.txt\n+++ b/Documentation/gitignore.txt\n@@ -71,6 +71,8 @@ PATTERN FORMAT\n matching file excluded by a previous pattern will become\n included again. If a negated pattern matches, this will\n override lower precedence patterns sources.\n+ If a directory is excluded by earlier patterns, negated\n+ patterns that touch files inside the directory will be ignored.\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\n-- \nDuy\n"},{"id":"166993","messageId":"4DC0ECED.2020409@viscovery.net","threadId":"27237","inReplyTo":"BANLkTi=TNjUaD4EP8sYXDqJ-tmBEzz4t4w@mail.gmail.com","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-05-04T06:06:37Z","receivedAt":"2011-05-04T06:06:37Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/4/2011 3:05, schrieb Nguyen Thai Ngoc Duy:\n> Maybe something like this instead of a implemantation fix?\n\nYes, but...\n\n> diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\n> index 8416f34..81e9d43 100644\n> --- a/Documentation/gitignore.txt\n> +++ b/Documentation/gitignore.txt\n> @@ -71,6 +71,8 @@ PATTERN FORMAT\n> matching file excluded by a previous pattern will become\n> included again. If a negated pattern matches, this will\n> override lower precedence patterns sources.\n> + If a directory is excluded by earlier patterns, negated\n> + patterns that touch files inside the directory will be ignored.\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> \n\n... as I already said here:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/170907/focus=170916\n\nI think that this is not quite the right place to mention this\nrestriction. See my proposal in the same post.\n\n-- Hannes\n"},{"id":"166995","messageId":"BANLkTik1w65vms3GFfjnaJJx3y6v8FT5UQ@mail.gmail.com","threadId":"27237","inReplyTo":"4DC0ECED.2020409@viscovery.net","subject":"Re: [PATCH 1/3] t3700: note a .gitignore matching fault","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-04T06:22:41Z","receivedAt":"2011-05-04T06:22:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, May 4, 2011 at 1:06 PM, Johannes Sixt wrote:\n> Am 5/4/2011 3:05, schrieb Nguyen Thai Ngoc Duy:\n>> Maybe something like this instead of a implemantation fix?\n>\n> Yes, but...\n>\n>> diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\n>> index 8416f34..81e9d43 100644\n>> --- a/Documentation/gitignore.txt\n>> +++ b/Documentation/gitignore.txt\n>> @@ -71,6 +71,8 @@ PATTERN FORMAT\n>>     matching file excluded by a previous pattern will become\n>>     included again.  If a negated pattern matches, this will\n>>     override lower precedence patterns sources.\n>> +   If a directory is excluded by earlier patterns, negated\n>> +   patterns that touch files inside the directory will be ignored.\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>>\n>\n> ... as I already said here:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/170907/focus=170916\n>\n> I think that this is not quite the right place to mention this\n> restriction. See my proposal in the same post.\n\nThanks. I missed that post. NOTES is fine although I prefer BUGS (or\nLIMITATIONS). We may decide not to look for more .gitignore inside,\nbut we should respect all patterns we have got. Can you please submit\nyour changes in a patch?\n-- \nDuy\n"},{"id":"167542","messageId":"7vei47o6oj.fsf@alter.siamese.dyndns.org","threadId":"27237","inReplyTo":"1304955781-13566-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/3] sparse checkout: do not eagerly decide the fate for whole directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T22:22:36Z","receivedAt":"2011-05-09T22:22:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Sparse-setting code follows closely how files are excluded in\n> read_directory(), every entry (including directories) are fed to\n> excluded_from_list() to decide if the entry is suitable. Directories\n> are treated no different than files. If a directory is matched (or\n> not), the whole directory is considered matched (or not) and the\n> process moves on.\n>\n> This generally works as long as there are no patterns to exclude parts\n> of the directory. In case of sparse checkout code, the following patterns\n>\n> t\n> !t/t0000-basic.sh\n>\n> will produce a worktree with full directory \"t\" even if t0000-basic.sh\n> is requested to stay out.\n\nThat roughly corresponds to having\n\n\t!t\n t/t0000-basic.sh\n\nin gitignore mechanism, right? Generally t/ is not to be excluded, but\nonly t0000-basic.sh should be. Sounds like a right thing to do.\n\n> Noticed-by: \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> Fix some comments as Thiago suggested. 1/3 of this series should be\n> dropped apparently.\n\nI am not quite sure what to do with this patch though.\n\nYou marked this as 3/3 without saying anything about where 1 and 2 are (or\nif they even exist). You hint there is a 1 that should be dropped here.\n\nIs this a re-rolled round, and if so where are the previous ones? Could\nyou make things easier to find (just saying something like [PATCH v2 3/3]\nin the Subject: is good enough) next time?\n"},{"id":"167647","messageId":"7vliyegm98.fsf@alter.siamese.dyndns.org","threadId":"27237","inReplyTo":"BANLkTimDO7yRyiBt7xS5Hy7-iT5SUDwJtA@mail.gmail.com","subject":"Re: [PATCH 3/3] sparse checkout: do not eagerly decide the fate for whole directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-10T23:37:55Z","receivedAt":"2011-05-10T23:37:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n>>> + /* If undecided, use parent directory's decision in defval */\n>> What means \"use parent directory's decision\"? Could you make this\n>> comment more clearer?\n>\n> Take the example patterns in commit message, we know that we match\n> directory \"t\" (pattern 1). When we check t/0001-init.sh, no patterns\n> match it. But because it's under \"t\", so we consider it matched. On\n> the other hand, t/t0000-basic.sh will match pattern 2 and override\n> parent directory's decision.\n\nSomebody ask about the comment you wrote, and you had to explain it.\nDoesn't it tell us something about the readability of the comment?\n\nAfter all the request was \"Could you make it clearer?\"\n"},{"id":"167663","messageId":"BANLkTimxpUwu3SQ9XNv1fUaM-0UwqPNUrg@mail.gmail.com","threadId":"27237","inReplyTo":"7vliyegm98.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] sparse checkout: do not eagerly decide the fate for whole directory","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-05-11T12:03:18Z","receivedAt":"2011-05-11T12:03:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, May 11, 2011 at 6:37 AM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>>>> +       /* If undecided, use parent directory's decision in defval */\n>>> What means \"use parent directory's decision\"? Could you make this\n>>> comment more clearer?\n>>\n>> Take the example patterns in commit message, we know that we match\n>> directory \"t\" (pattern 1). When we check t/0001-init.sh, no patterns\n>> match it. But because it's under \"t\", so we consider it matched. On\n>> the other hand, t/t0000-basic.sh will match pattern 2 and override\n>> parent directory's decision.\n>\n> Somebody ask about the comment you wrote, and you had to explain it.\n> Doesn't it tell us something about the readability of the comment?\n>\n> After all the request was \"Could you make it clearer?\"\n\nHe did not reply whether my further explanation was good enough. Maybe this?\n\nexcluded_from_list() can't decide whether the entry matches given\npatterns. If the current directory is matched before (ie. in defval),\nthen by default the entry is also matched.\n-- \nDuy\n"}]}