{"thread":{"id":"30617","subject":"[PATCH] Teach ignore machinery about pattern \"/\"","startedAt":"2012-05-25T12:47:36Z","lastAt":"2012-06-03T22:50:46Z","messageCount":7,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy","Caleb Marchent"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"192161","messageId":"1337950056-26491-1-git-send-email-pclouds@gmail.com","threadId":"30617","inReplyTo":null,"subject":"[PATCH] Teach ignore machinery about pattern \"/\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-25T12:47:36Z","receivedAt":"2012-05-25T12:47:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Pattern \"/\" is ambiguous because the slash can mean \"anchor the\npattern to this location\" (e.g. /path), but it can also mean\n\"match directories only\" (e.g. path/). Currently git interprets it as\nthe latter except that 'path' is an empty string, which makes this\npattern totally useless.\n\nOn the other hand, it's intuitive to read '/' as \"match root\ndirectory\", or equivalent to \"/*\". (The other way to see it is \"match\nall directories\", which leads to the same result).\n\nOne may wonder why bother an \"ignore all\" pattern. The pattern is\nuseful when you want to keep just a small portion of the working\ndirectory. But that's still a rare case.\n\nA more popular case would be sparse checkout, where ignore rules are\nused to _include_. The thus now \"include all\" pattern is used to say\n\"make a sparse checkout that includes everything except this and\nthis\".\n\nRecognize this special pattern and turn it the working equivalence\npattern \"/*\"\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n dir.c | 3 +++\n t/t3001-ls-files-others-exclude.sh | 6 ++++++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/dir.c b/dir.c\nindex c6a98cc..b65d37c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -308,6 +308,9 @@ void add_exclude(const char *string, const char *base,\n \tint to_exclude = 1;\n \tint flags = 0;\n \n+\tif (string[0] == '/' && string[1] == '\\0')\n+\t\tstring = \"/*\";\n+\n \tif (*string == '!') {\n \t\tto_exclude = 0;\n \t\tstring++;\ndiff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh\nindex c8fe978..7cb790d 100755\n--- a/t/t3001-ls-files-others-exclude.sh\n+++ b/t/t3001-ls-files-others-exclude.sh\n@@ -175,6 +175,12 @@ test_expect_success 'negated exclude matches can override previous ones' '\n \tgrep \"^a.1\" output\n '\n \n+test_expect_success '\"/\" pattern is equivalent to \"/*\" (exclude all)' '\n+\tgit ls-files --others --exclude=/ >output &&\n+\t: >expected &&\n+\ttest_cmp expected output\n+'\n+\n test_expect_success 'subdirectory ignore (setup)' '\n \tmkdir -p top/l1/l2 &&\n \t(\n-- \n1.7.10.2.549.g9354186\n"},{"id":"192175","messageId":"7vlikgtbcx.fsf@alter.siamese.dyndns.org","threadId":"30617","inReplyTo":"1337950056-26491-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] Teach ignore machinery about pattern \"/\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-25T17:32:46Z","receivedAt":"2012-05-25T17:32:46Z","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> Pattern \"/\" is ambiguous because the slash can mean \"anchor the\n> pattern to this location\" (e.g. /path), but it can also mean\n> \"match directories only\" (e.g. path/). Currently git interprets it as\n> the latter except that 'path' is an empty string, which makes this\n> pattern totally useless.\n\nDid the old version interprete it as the former? How does the above\nobservation relate to the end-user help request in the other thread?\n\nWhen talking about an ambiguous expression X that can be in multiple ways\nand each interpretation gives surprisingly different result, if there are\nways A, B and C to unambiguously spell each and every of its possible and\nuseful interpretations, and if one of the interpretations C is to ignore\nthe expression altogether, it is preferrable to either\n\n (1) warn if it does not trigger anything useful to write a no-op; or\n (2) just ignore it if a no-op is a meaningful behaviour\n\nwhen you see X.\n\n> On the other hand, it's intuitive to read '/' as \"match root\n> directory\", or equivalent to \"/*\". (The other way to see it is \"match\n> all directories\", which leads to the same result).\n\nI am a bit confused about the above, especially \"The other way\" part.\n\nDoes this alternative interpretation view \"/\" as \"/foo\" whose \"foo\"\nhappens to be an empty string, or does it view \"/\" as \"foo/\" whose \"foo\"\nhappens to be an empty string?\n\nThe former would mean \"ignore foo, and don't bother descending into foo if\nit is a directory, as everything in it is ignored\", while the latter would\nmean \"anywhere in this directory and its subdirectories, if we see foo\nthat is a directory, don't bother descending into it as everything in it\nis ignored.\"\n\nWhat I am trying to get at is why you are making '/' the same as '/*' as\nopposed to '*/', '/*/', or even a plain '*'.\n\n> One may wonder why bother an \"ignore all\" pattern. The pattern is\n> useful when you want to keep just a small portion of the working\n> directory. But that's still a rare case.\n>\n> A more popular case would be sparse checkout, where ignore rules are\n> used to _include_. The thus now \"include all\" pattern is used to say\n> \"make a sparse checkout that includes everything except this and\n> this\".\n\nIf the \"sparse checkout\" hack writes \"/\" for whatever reason, it should be\ncorrected to write \"*\" (or perhaps \"/*\") instead. I do not see it as a\nvalid factor to affect when we decide what should be done for a possibly\nambiguous \"/\" entry in the exclude machinery.\n\n> Recognize this special pattern and turn it the working equivalence\n> pattern \"/*\"\n\nRegardless of the answer to the \"is it *, /*, */ or /*/\" question above,\nI do not think silent conversion is a right solution for the ambiguity.\n"},{"id":"192223","messageId":"CACsJy8BjUQsCVVdzZtJxA9Qu2G0izZa8dvP1EKsQQAHWXZbHdg@mail.gmail.com","threadId":"30617","inReplyTo":"7vlikgtbcx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Teach ignore machinery about pattern \"/\"","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-26T04:30:36Z","receivedAt":"2012-05-26T04:30:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, May 26, 2012 at 12:32 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy   writes:\n>\n>> Pattern \"/\" is ambiguous because the slash can mean \"anchor the\n>> pattern to this location\" (e.g. /path), but it can also mean\n>> \"match directories only\" (e.g. path/). Currently git interprets it as\n>> the latter except that 'path' is an empty string, which makes this\n>> pattern totally useless.\n>\n> Did the old version interprete it as the former?\n\nThat calls for bit of testing, which I'll do soon.\n\n> How does the above\n> observation relate to the end-user help request in the other thread?\n\nI assume you meant the recent sparse-checkout thread. \"/\" happens to\nwork due to (probably) a mistake in sparse-checkout v1.7.2 even though\n\"/\" behavior is undefined. This sets a specific behavior to \"/\" and\nmakes 1.7.10 treat \"/\" like 1.7.2.\n\n> When talking about an ambiguous expression X that can be in multiple ways\n> and each interpretation gives surprisingly different result, if there are\n> ways A, B and C to unambiguously spell each and every of its possible and\n> useful interpretations, and if one of the interpretations C is to ignore\n> the expression altogether, it is preferrable to either\n>\n>  (1) warn if it does not trigger anything useful to write a no-op; or\n>  (2) just ignore it if a no-op is a meaningful behaviour\n>\n> when you see X.\n\nexcept that we do the ignoring in (2) but I don't see this as a\nmeaningful behavior.\n\n>> On the other hand, it's intuitive to read '/' as \"match root\n>> directory\", or equivalent to \"/*\". (The other way to see it is \"match\n>> all directories\", which leads to the same result).\n>\n> I am a bit confused about the above, especially \"The other way\" part.\n>\n> Does this alternative interpretation view \"/\" as \"/foo\" whose \"foo\"\n> happens to be an empty string, or does it view \"/\" as \"foo/\" whose \"foo\"\n> happens to be an empty string?\n\nThe latter. Though there may be two way of interpreting the empty\nstring as pattern. The computer would happily say \"match nothing\", but\nmy human brain tends to thinkg \"match everything\" as \"match nothing\"\ndoes not really makes sense as a pattern.\n\n> The former would mean \"ignore foo, and don't bother descending into foo if\n> it is a directory, as everything in it is ignored\", while the latter would\n> mean \"anywhere in this directory and its subdirectories, if we see foo\n> that is a directory, don't bother descending into it as everything in it\n> is ignored.\"\n>\n> What I am trying to get at is why you are making '/' the same as '/*' as\n> opposed to '*/', '/*/', or even a plain '*'.\n\nBecause I see \"/\" as a special case of \"/foo\". It makes more sense\nthan \"/\" as a special case of \"foo/\", at least not without more\nthoughts on it.\n\n>> One may wonder why bother an \"ignore all\" pattern. The pattern is\n>> useful when you want to keep just a small portion of the working\n>> directory. But that's still a rare case.\n>>\n>> A more popular case would be sparse checkout, where ignore rules are\n>> used to _include_. The thus now \"include all\" pattern is used to say\n>> \"make a sparse checkout that includes everything except this and\n>> this\".\n>\n> If the \"sparse checkout\" hack writes \"/\" for whatever reason, it should be\n> corrected to write \"*\" (or perhaps \"/*\") instead.  I do not see it as a\n> valid factor to affect when we decide what should be done for a possibly\n> ambiguous \"/\" entry in the exclude machinery.\n\nThat's one way of seeing it. What I propose in the patch is \"this is\nuseless no-op pattern (*), let's assign some meaning to it, what makes\nmost sense\"\n\n(*) still needs verification though.\n\n>\n>> Recognize this special pattern and turn it the working equivalence\n>> pattern \"/*\"\n>\n> Regardless of the answer to the \"is it *, /*, */ or /*/\" question above,\n> I do not think silent conversion is a right solution for the ambiguity.\n-- \nDuy\n"},{"id":"192257","messageId":"7vwr3yp2t8.fsf@alter.siamese.dyndns.org","threadId":"30617","inReplyTo":"CACsJy8BjUQsCVVdzZtJxA9Qu2G0izZa8dvP1EKsQQAHWXZbHdg@mail.gmail.com","subject":"Re: [PATCH] Teach ignore machinery about pattern \"/\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-27T06:15:31Z","receivedAt":"2012-05-27T06:15:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n>> When talking about an ambiguous expression X that can be in multiple ways\n>> and each interpretation gives surprisingly different result, if there are\n>> ways A, B and C to unambiguously spell each and every of its possible and\n>> useful interpretations, and if one of the interpretations C is to ignore\n>> the expression altogether, it is preferrable to either\n>>\n>> (1) warn if it does not trigger anything useful to write a no-op; or\n>> (2) just ignore it if a no-op is a meaningful behaviour\n>>\n>> when you see X.\n>\n> except that we do the ignoring in (2) but I don't see this as a\n> meaningful behavior.\n\nThat would mean you would want to update the code to do (1), not silently\ntrigger a different behaviour.\n"},{"id":"192753","messageId":"CACsJy8BSHFzGfsm6qY1LEX9wubKjKXXbJ_WMAiOAUhVGa631Ag@mail.gmail.com","threadId":"30617","inReplyTo":"CACsJy8BjUQsCVVdzZtJxA9Qu2G0izZa8dvP1EKsQQAHWXZbHdg@mail.gmail.com","subject":"Re: [PATCH] Teach ignore machinery about pattern \"/\"","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-06-03T12:36:23Z","receivedAt":"2012-06-03T12:36:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, May 26, 2012 at 11:30 AM, Nguyen Thai Ngoc Duy\n wrote:\n> On Sat, May 26, 2012 at 12:32 AM, Junio C Hamano wrote:\n>> Nguyễn Thái Ngọc Duy   writes:\n>>\n>>> Pattern \"/\" is ambiguous because the slash can mean \"anchor the\n>>> pattern to this location\" (e.g. /path), but it can also mean\n>>> \"match directories only\" (e.g. path/). Currently git interprets it as\n>>> the latter except that 'path' is an empty string, which makes this\n>>> pattern totally useless.\n>>\n>> Did the old version interprete it as the former?\n>\n> That calls for bit of testing, which I'll do soon.\n\n\"/\" is no-op since 1.4.0 (tested with refs/tags/v*), tested with \"git\nadd .\" with $GIT_DIR/info/exclude containing only \"/\". \"git add\" in\npre-1.4 is shell script and needs to be installed to work properly so\nI only tested v1.3.0 and v1.0.0, and it worked too (i.e. \"/\" is still\nno-op). I think it's safe to make it a meaningful pattern (or warn\nabout its uselessness).\n-- \nDuy\n"},{"id":"192774","messageId":"7vbol0xdzj.fsf@alter.siamese.dyndns.org","threadId":"30617","inReplyTo":"CACsJy8BSHFzGfsm6qY1LEX9wubKjKXXbJ_WMAiOAUhVGa631Ag@mail.gmail.com","subject":"Re: [PATCH] Teach ignore machinery about pattern \"/\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-03T21:49:36Z","receivedAt":"2012-06-03T21:49:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> On Sat, May 26, 2012 at 11:30 AM, Nguyen Thai Ngoc Duy\n> wrote:\n>> On Sat, May 26, 2012 at 12:32 AM, Junio C Hamano wrote:\n>>> Nguyễn Thái Ngọc Duy   writes:\n>>>\n>>>> Pattern \"/\" is ambiguous because the slash can mean \"anchor the\n>>>> pattern to this location\" (e.g. /path), but it can also mean\n>>>> \"match directories only\" (e.g. path/). Currently git interprets it as\n>>>> the latter except that 'path' is an empty string, which makes this\n>>>> pattern totally useless.\n>>>\n>>> Did the old version interprete it as the former?\n>>\n>> That calls for bit of testing, which I'll do soon.\n>\n> \"/\" is no-op since 1.4.0 (tested with refs/tags/v*),...\n\nSo historically we had three possible interpretations of \"/\",\ni.e. \"/path\" that happens to have an empty path, \"path/\" that\nhappens to have an empty path, and a no-op.\n\nThat strongly suggests that we should at least warn when we see \"/\"\nthat the user is playing with fire by feeding the exclude machinery\nsomething without clear semantics.\n\nDoing a no-op might be a safer option than adding it new special\ncase semantics to it, but even then I do not think we should\nsilently ignore it. Silently giving new semantics for this\nparticular special case is out of the question, I would have to say.\n\nThanks for digging.\n"},{"id":"192779","messageId":"9C3609BEC91A814BB52DBF9C41F04D1E58DEDB2D@uk-swa-ex01","threadId":"30617","inReplyTo":"7vbol0xdzj.fsf@alter.siamese.dyndns.org","subject":"RE: [PATCH] Teach ignore machinery about pattern \"/\"","fromName":"Caleb Marchent","fromEmail":"cmarchent@aminocom.com","sentAt":"2012-06-03T22:50:46Z","receivedAt":"2012-06-03T22:50:46Z","isPatch":true,"sender":{"key":"cmarchent@aminocom.com","avatar":null},"body":"Hi Guy's,\n\nSeeing the discussion about the interpretation of '/' - I would like to add some additional support for my case that it should represent the contents of the top level dir:\n\nIf I do \"ls /\" on a unix system - I get a list of the contents of the root of the files system, I think we should maintain this in git, such that '/' represents the top of the tree rather than suppressing it. \n\nBest Regards,\n\nCaleb\n\nCaleb Marchent, Software Developer, Amino Communications Ltd, Buckingway Business Park, Anderson Road, Swavesey, Cambridge, CB24 4UQ, UK cmarchent@aminocom.com www.aminocom.com\n\nTel: +44 1954 234 100\n\n________________________________________\nFrom: git-owner@vger.kernel.org [git-owner@vger.kernel.org] on behalf of Junio C Hamano [gitster@pobox.com]\nSent: 03 June 2012 22:49\nTo: Nguyen Thai Ngoc Duy\nCc: git@vger.kernel.org\nSubject: Re: [PATCH] Teach ignore machinery about pattern \"/\"\n\nNguyen Thai Ngoc Duy writes:\n\n> On Sat, May 26, 2012 at 11:30 AM, Nguyen Thai Ngoc Duy\n> wrote:\n>> On Sat, May 26, 2012 at 12:32 AM, Junio C Hamano wrote:\n>>> Nguyễn Thái Ngọc Duy writes:\n>>>\n>>>> Pattern \"/\" is ambiguous because the slash can mean \"anchor the\n>>>> pattern to this location\" (e.g. /path), but it can also mean\n>>>> \"match directories only\" (e.g. path/). Currently git interprets it as\n>>>> the latter except that 'path' is an empty string, which makes this\n>>>> pattern totally useless.\n>>>\n>>> Did the old version interprete it as the former?\n>>\n>> That calls for bit of testing, which I'll do soon.\n>\n> \"/\" is no-op since 1.4.0 (tested with refs/tags/v*),...\n\nSo historically we had three possible interpretations of \"/\",\ni.e. \"/path\" that happens to have an empty path, \"path/\" that\nhappens to have an empty path, and a no-op.\n\nThat strongly suggests that we should at least warn when we see \"/\"\nthat the user is playing with fire by feeding the exclude machinery\nsomething without clear semantics.\n\nDoing a no-op might be a safer option than adding it new special\ncase semantics to it, but even then I do not think we should\nsilently ignore it. Silently giving new semantics for this\nparticular special case is out of the question, I would have to say.\n\nThanks for digging.\n--\nTo unsubscribe from this list: send the line \"unsubscribe git\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html\n"}]}