{"thread":{"id":"39093","subject":"[PATCH] dir: allow a BOM at the beginning of exclude files","startedAt":"2015-04-16T14:05:12Z","lastAt":"2015-04-20T21:50:08Z","messageCount":24,"participants":["Carlos Martín Nieto","Johannes Schindelin","Junio C Hamano","Jeff King","Torsten Bögershausen","Karsten Blees"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"259495","messageId":"1429193112-41184-1-git-send-email-cmn@elego.de","threadId":"39093","inReplyTo":null,"subject":"[PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2015-04-16T14:05:12Z","receivedAt":"2015-04-16T14:05:12Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Some text editors like Notepad or LibreOffice write an UTF-8 BOM in\norder to indicate that the file is Unicode text rather than whatever the\ncurrent locale would indicate.\n\nIf someone uses such an editor to edit a gitignore file, we are left\nwith those three bytes at the beginning of the file. If we do not skip\nthem, we will attempt to match a filename with the BOM as prefix, which\nwon't match the files the user is expecting.\n\n---\n\nIf you're wondering how I came up with LibreOffice, I was doing a\nworkshop recently and one of the participants was not content with the\nchoice of vim or nano, so he opened LibreOffice to edit the gitignore\nfile with confusing consequences.\n\nThis codepath doesn't go as far as the config code in validating that\nwe do not have a partial BOM which would mean there's some invalid\ncontent, but we don't really have invalid content any other way, as\nwe're just dealing with a list of paths in the file.\n\n dir.c                      | 8 +++++++-\n t/t7061-wtstatus-ignore.sh | 2 ++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 0943a81..6368247 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -581,6 +581,7 @@ int add_excludes_from_file_to_list(const char *fname,\n \tstruct stat st;\n \tint fd, i, lineno = 1;\n \tsize_t size = 0;\n+\tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\n \tchar *buf, *entry;\n \n \tfd = open(fname, O_RDONLY);\n@@ -617,7 +618,12 @@ int add_excludes_from_file_to_list(const char *fname,\n \t}\n \n \tel->filebuf = buf;\n-\tentry = buf;\n+\n+\tif (size >= 3 && !memcmp(buf, utf8_bom, 3))\n+\t\tentry = buf + 3;\n+\telse\n+\t\tentry = buf;\n+\n \tfor (i = 0; i < size; i++) {\n \t\tif (buf[i] == '\\n') {\n \t\t\tif (entry != buf + i && entry[0] != '#') {\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex 460789b..0a06fbf 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -13,6 +13,8 @@ EOF\n \n test_expect_success 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n+\tsed -e \"s/^/\\xef\\xbb\\xbf/\" .gitignore >.gitignore.new &&\n+\tmv .gitignore.new .gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n \t: >untracked/uncommitted &&\n-- \n2.0.0.5.gbce14aa\n"},{"id":"259500","messageId":"13f82e720ac12a6bc12e8b9f566dd48e@www.dscho.org","threadId":"39093","inReplyTo":"1429193112-41184-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-04-16T15:03:05Z","receivedAt":"2015-04-16T15:03:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlos,\n\nOn 2015-04-16 16:05, Carlos Martín Nieto wrote:\n> Some text editors like Notepad or LibreOffice write an UTF-8 BOM in\n> order to indicate that the file is Unicode text rather than whatever the\n> current locale would indicate.\n> \n> If someone uses such an editor to edit a gitignore file, we are left\n> with those three bytes at the beginning of the file. If we do not skip\n> them, we will attempt to match a filename with the BOM as prefix, which\n> won't match the files the user is expecting.\n> \n> ---\n> \n> If you're wondering how I came up with LibreOffice, I was doing a\n> workshop recently and one of the participants was not content with the\n> choice of vim or nano, so he opened LibreOffice to edit the gitignore\n> file with confusing consequences.\n> \n> This codepath doesn't go as far as the config code in validating that\n> we do not have a partial BOM which would mean there's some invalid\n> content, but we don't really have invalid content any other way, as\n> we're just dealing with a list of paths in the file.\n\nYeah, users are entertaining!\n\nI agree that this is a good patch. *Maybe* we would need the same handling in more places, in which case it might make sense to refactor the test into its own function.\n\nIn any case, though, the Git project requires a [Developer's Certificate of Origin](https://github.com/git/git/blob/v2.3.5/Documentation/SubmittingPatches#L234-L277); Would you mind adding that?\n\nThanks,\nDscho\n"},{"id":"259502","messageId":"1429196958.3097.15.camel@elego.de","threadId":"39093","inReplyTo":"13f82e720ac12a6bc12e8b9f566dd48e@www.dscho.org","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2015-04-16T15:09:18Z","receivedAt":"2015-04-16T15:09:18Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Thu, 2015-04-16 at 17:03 +0200, Johannes Schindelin wrote:\n> Hi Carlos,\n> \n> On 2015-04-16 16:05, Carlos Martín Nieto wrote:\n> > Some text editors like Notepad or LibreOffice write an UTF-8 BOM in\n> > order to indicate that the file is Unicode text rather than whatever the\n> > current locale would indicate.\n> > \n> > If someone uses such an editor to edit a gitignore file, we are left\n> > with those three bytes at the beginning of the file. If we do not skip\n> > them, we will attempt to match a filename with the BOM as prefix, which\n> > won't match the files the user is expecting.\n> > \n> > ---\n> > \n> > If you're wondering how I came up with LibreOffice, I was doing a\n> > workshop recently and one of the participants was not content with the\n> > choice of vim or nano, so he opened LibreOffice to edit the gitignore\n> > file with confusing consequences.\n> > \n> > This codepath doesn't go as far as the config code in validating that\n> > we do not have a partial BOM which would mean there's some invalid\n> > content, but we don't really have invalid content any other way, as\n> > we're just dealing with a list of paths in the file.\n> \n> Yeah, users are entertaining!\n> \n> I agree that this is a good patch. *Maybe* we would need the same handling in more places, in which case it might make sense to refactor the test into its own function.\n\nYes, this was my train of thought. If we (discover that) need it in a\nthird place, then we can unify the test and skip. For two places I\nreckoned it was fine if we duplicated a bit.\n\n> \n> In any case, though, the Git project requires a [Developer's Certificate of Origin](https://github.com/git/git/blob/v2.3.5/Documentation/SubmittingPatches#L234-L277); Would you mind adding that?\n> \n\nYeah, sorry. I keep forgetting to do that. I'll reply to my original\ne-mail with it.\n\nCheers,\n   cmn\n"},{"id":"259501","messageId":"1429197002.3097.16.camel@elego.de","threadId":"39093","inReplyTo":"1429193112-41184-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2015-04-16T15:10:02Z","receivedAt":"2015-04-16T15:10:02Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Thu, 2015-04-16 at 16:05 +0200, Carlos Martín Nieto wrote:\n> Some text editors like Notepad or LibreOffice write an UTF-8 BOM in\n> order to indicate that the file is Unicode text rather than whatever the\n> current locale would indicate.\n> \n> If someone uses such an editor to edit a gitignore file, we are left\n> with those three bytes at the beginning of the file. If we do not skip\n> them, we will attempt to match a filename with the BOM as prefix, which\n> won't match the files the user is expecting.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n\nwhich I keep forgetting.\n\n> \n> ---\n> \n> If you're wondering how I came up with LibreOffice, I was doing a\n> workshop recently and one of the participants was not content with the\n> choice of vim or nano, so he opened LibreOffice to edit the gitignore\n> file with confusing consequences.\n> \n> This codepath doesn't go as far as the config code in validating that\n> we do not have a partial BOM which would mean there's some invalid\n> content, but we don't really have invalid content any other way, as\n> we're just dealing with a list of paths in the file.\n> \n>  dir.c                      | 8 +++++++-\n>  t/t7061-wtstatus-ignore.sh | 2 ++\n>  2 files changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/dir.c b/dir.c\n> index 0943a81..6368247 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -581,6 +581,7 @@ int add_excludes_from_file_to_list(const char *fname,\n>  \tstruct stat st;\n>  \tint fd, i, lineno = 1;\n>  \tsize_t size = 0;\n> +\tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\n>  \tchar *buf, *entry;\n>  \n>  \tfd = open(fname, O_RDONLY);\n> @@ -617,7 +618,12 @@ int add_excludes_from_file_to_list(const char *fname,\n>  \t}\n>  \n>  \tel->filebuf = buf;\n> -\tentry = buf;\n> +\n> +\tif (size >= 3 && !memcmp(buf, utf8_bom, 3))\n> +\t\tentry = buf + 3;\n> +\telse\n> +\t\tentry = buf;\n> +\n>  \tfor (i = 0; i < size; i++) {\n>  \t\tif (buf[i] == '\\n') {\n>  \t\t\tif (entry != buf + i && entry[0] != '#') {\n> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\n> index 460789b..0a06fbf 100755\n> --- a/t/t7061-wtstatus-ignore.sh\n> +++ b/t/t7061-wtstatus-ignore.sh\n> @@ -13,6 +13,8 @@ EOF\n>  \n>  test_expect_success 'status untracked directory with --ignored' '\n>  \techo \"ignored\" >.gitignore &&\n> +\tsed -e \"s/^/\\xef\\xbb\\xbf/\" .gitignore >.gitignore.new &&\n> +\tmv .gitignore.new .gitignore &&\n>  \tmkdir untracked &&\n>  \t: >untracked/ignored &&\n>  \t: >untracked/uncommitted &&\n"},{"id":"259505","messageId":"xmqqsic0hyk4.fsf@gitster.dls.corp.google.com","threadId":"39093","inReplyTo":"1429193112-41184-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T15:39:55Z","receivedAt":"2015-04-16T15:39:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto <cmn@elego.de> writes:\n\n> diff --git a/dir.c b/dir.c\n> index 0943a81..6368247 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -581,6 +581,7 @@ int add_excludes_from_file_to_list(const char *fname,\n>  \tstruct stat st;\n>  \tint fd, i, lineno = 1;\n>  \tsize_t size = 0;\n> +\tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\n>  \tchar *buf, *entry;\n>  \n>  \tfd = open(fname, O_RDONLY);\n> @@ -617,7 +618,12 @@ int add_excludes_from_file_to_list(const char *fname,\n>  \t}\n>  \n>  \tel->filebuf = buf;\n> -\tentry = buf;\n> +\n> +\tif (size >= 3 && !memcmp(buf, utf8_bom, 3))\n\nOK.\n\n> +\t\tentry = buf + 3;\n> +\telse\n> +\t\tentry = buf;\n> +\n>  \tfor (i = 0; i < size; i++) {\n>  \t\tif (buf[i] == '\\n') {\n>  \t\t\tif (entry != buf + i && entry[0] != '#') {\n> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\n> index 460789b..0a06fbf 100755\n> --- a/t/t7061-wtstatus-ignore.sh\n> +++ b/t/t7061-wtstatus-ignore.sh\n> @@ -13,6 +13,8 @@ EOF\n>  \n>  test_expect_success 'status untracked directory with --ignored' '\n>  \techo \"ignored\" >.gitignore &&\n> +\tsed -e \"s/^/\\xef\\xbb\\xbf/\" .gitignore >.gitignore.new &&\n> +\tmv .gitignore.new .gitignore &&\n\nIs this \"write literal in \\xHEX on the replacement side of sed\nsubstitution\" potable?  In any case, replacing the above three with\nsomething like:\n\n\tprintf \"<bom>ignored\\n\" >.gitignore\n\nmay be more sensible, no?\n\nAlso do we need a similar change to the attribute side, or are we\nalready covered and we forgot to do the same for the ignore files?\n\n\n>  \tmkdir untracked &&\n>  \t: >untracked/ignored &&\n>  \t: >untracked/uncommitted &&\n"},{"id":"259508","messageId":"20150416155558.GA10390@peff.net","threadId":"39093","inReplyTo":"xmqqsic0hyk4.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-04-16T15:55:58Z","receivedAt":"2015-04-16T15:55:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 16, 2015 at 08:39:55AM -0700, Junio C Hamano wrote:\n\n> >  test_expect_success 'status untracked directory with --ignored' '\n> >  \techo \"ignored\" >.gitignore &&\n> > +\tsed -e \"s/^/\\xef\\xbb\\xbf/\" .gitignore >.gitignore.new &&\n> > +\tmv .gitignore.new .gitignore &&\n> \n> Is this \"write literal in \\xHEX on the replacement side of sed\n> substitution\" potable?  In any case, replacing the above three with\n> something like:\n> \n> \tprintf \"<bom>ignored\\n\" >.gitignore\n> \n> may be more sensible, no?\n\nI'm not sure about sed, but I agree it is suspect. And note that printf\nwith hex codes is not portable, either You have to use octal:\n\n  printf '\\357\\273\\277ignored\\n' >.gitignore\n\nAlso, as a nit, I'd much rather see this in its own test rather than\ncrammed into another test_expect_success. It's much easier to diagnose\nfailures if the test description mentions the goal, and it is not tied\nup with testing other parts that might fail.\n\n-Peff\n"},{"id":"259511","messageId":"26acc1b2979a49a2f65e8e14cd2d944b@www.dscho.org","threadId":"39093","inReplyTo":"xmqqsic0hyk4.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-04-16T16:08:12Z","receivedAt":"2015-04-16T16:08:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn 2015-04-16 17:39, Junio C Hamano wrote:\n\n> Also do we need a similar change to the attribute side, or are we\n> already covered and we forgot to do the same for the ignore files?\n\nI fear so: https://github.com/git/git/blob/v2.3.5/attr.c#L359-L376\n\nAs for the config, we are safe: https://github.com/git/git/blob/v2.3.5/config.c#L419-L439\n\nCiao,\nDscho\n"},{"id":"259512","messageId":"552FDF05.4060000@web.de","threadId":"39093","inReplyTo":"1429193112-41184-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-04-16T16:10:45Z","receivedAt":"2015-04-16T16:10:45Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-04-16 16.05, Carlos Martín Nieto wrote:\n[]\nMay be it is easier to move this into an own function, like remove_utf8_bom() ?\n\n>  dir.c                      | 8 +++++++-\n>  t/t7061-wtstatus-ignore.sh | 2 ++\n>  2 files changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/dir.c b/dir.c\n> index 0943a81..6368247 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -581,6 +581,7 @@ int add_excludes_from_file_to_list(const char *fname,\n>  \tstruct stat st;\n>  \tint fd, i, lineno = 1;\n>  \tsize_t size = 0;\n> +\tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\nDo we really need to cast here (and if, is the cast dropping the \"const\" ?)\n\nAnother suggestion, see below:\neither:\n\tstatic const size_t bom_len = 3;\nor\n\tstatic const size_t bom_len = strlen(utf8_bom);\n\n>  \tchar *buf, *entry;\n>  \n>  \tfd = open(fname, O_RDONLY);\n> @@ -617,7 +618,12 @@ int add_excludes_from_file_to_list(const char *fname,\n>  \t}\n>  \n>  \tel->filebuf = buf;\n> -\tentry = buf;\n> +\nAnd now we can avoid magic numbers:\n\tif (size >= bom_len && !memcmp(buf, utf8_bom, bom_len))\n\t\tentry = buf + bom_len;\n\telse\n\t\tentry = buf;\n[]\n"},{"id":"259518","messageId":"xmqqoamohu2m.fsf@gitster.dls.corp.google.com","threadId":"39093","inReplyTo":"20150416155558.GA10390@peff.net","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T17:16:49Z","receivedAt":"2015-04-16T17:16:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Apr 16, 2015 at 08:39:55AM -0700, Junio C Hamano wrote:\n>\n>> >  test_expect_success 'status untracked directory with --ignored' '\n>> >  \techo \"ignored\" >.gitignore &&\n>> > +\tsed -e \"s/^/\\xef\\xbb\\xbf/\" .gitignore >.gitignore.new &&\n>> > +\tmv .gitignore.new .gitignore &&\n>> \n>> Is this \"write literal in \\xHEX on the replacement side of sed\n>> substitution\" potable?  In any case, replacing the above three with\n>> something like:\n>> \n>> \tprintf \"<bom>ignored\\n\" >.gitignore\n>> \n>> may be more sensible, no?\n>\n> I'm not sure about sed, but I agree it is suspect. And note that printf\n> with hex codes is not portable, either You have to use octal:\n>\n>   printf '\\357\\273\\277ignored\\n' >.gitignore\n>\n> Also, as a nit, I'd much rather see this in its own test rather than\n> crammed into another test_expect_success. It's much easier to diagnose\n> failures if the test description mentions the goal, and it is not tied\n> up with testing other parts that might fail.\n\nYeah, I totally agree.\n\nCarlos, something like this squashed in, perhaps?\n\n t/t7061-wtstatus-ignore.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nindex 0a06fbf..cdc0747 100755\n--- a/t/t7061-wtstatus-ignore.sh\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -13,8 +13,6 @@ EOF\n \n test_expect_success 'status untracked directory with --ignored' '\n \techo \"ignored\" >.gitignore &&\n-\tsed -e \"s/^/\\xef\\xbb\\xbf/\" .gitignore >.gitignore.new &&\n-\tmv .gitignore.new .gitignore &&\n \tmkdir untracked &&\n \t: >untracked/ignored &&\n \t: >untracked/uncommitted &&\n@@ -22,6 +20,15 @@ test_expect_success 'status untracked directory with --ignored' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'same with gitignore starting with BOM' '\n+\tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n+\tmkdir -p untracked &&\n+\t: >untracked/ignored &&\n+\t: >untracked/uncommitted &&\n+\tgit status --porcelain --ignored >actual &&\n+\ttest_cmp expected actual\n+'\n+\n cat >expected <<\\EOF\n ?? .gitignore\n ?? actual\n"},{"id":"259522","messageId":"1429206774-10087-1-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"xmqqoamohu2m.fsf@gitster.dls.corp.google.com","subject":"[PATCH 0/3] UTF8 BOM follow-up","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T17:52:51Z","receivedAt":"2015-04-16T17:52:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is on top of the \".gitignore can start with UTF8 BOM\" patch\nfrom Carlos.\n\nJunio C Hamano (3):\n  utf8-bom: introduce skip_utf8_bom() helper\n  config: use utf8_bom[] from utf.[ch] in git_parse_source()\n  attr: skip UTF8 BOM at the beginning of the input file\n\n attr.c   |  9 +++++++--\n config.c |  6 +++---\n dir.c    |  8 +++-----\n utf8.c   | 11 +++++++++++\n utf8.h   |  3 +++\n 5 files changed, 27 insertions(+), 10 deletions(-)\n\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259520","messageId":"1429206774-10087-2-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"1429206774-10087-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] utf8-bom: introduce skip_utf8_bom() helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T17:52:52Z","receivedAt":"2015-04-16T17:52:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the recent change to ignore the UTF8 BOM at the beginning of\n.gitignore files, we now have two codepaths that do such a skipping\n(the other one is for reading the configuration files).\n\nIntroduce utf8_bom[] constant string and skip_utf8_bom() helper\nand teach .gitignore code how to use it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n dir.c  |  8 +++-----\n utf8.c | 11 +++++++++++\n utf8.h |  3 +++\n 3 files changed, 17 insertions(+), 5 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 10c1f90..9e04a23 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -12,6 +12,7 @@\n #include \"refs.h\"\n #include \"wildmatch.h\"\n #include \"pathspec.h\"\n+#include \"utf8.h\"\n \n struct path_simplify {\n \tint len;\n@@ -538,7 +539,6 @@ int add_excludes_from_file_to_list(const char *fname,\n \tstruct stat st;\n \tint fd, i, lineno = 1;\n \tsize_t size = 0;\n-\tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\n \tchar *buf, *entry;\n \n \tfd = open(fname, O_RDONLY);\n@@ -576,10 +576,8 @@ int add_excludes_from_file_to_list(const char *fname,\n \n \tel->filebuf = buf;\n \n-\tif (size >= 3 && !memcmp(buf, utf8_bom, 3))\n-\t\tentry = buf + 3;\n-\telse\n-\t\tentry = buf;\n+\tentry = buf;\n+\tskip_utf8_bom(&entry, size);\n \n \tfor (i = 0; i < size; i++) {\n \t\tif (buf[i] == '\\n') {\ndiff --git a/utf8.c b/utf8.c\nindex 520fbb4..28e6d76 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -633,3 +633,14 @@ int is_hfs_dotgit(const char *path)\n \n \treturn 1;\n }\n+\n+const char utf8_bom[] = \"\\357\\273\\277\";\n+\n+int skip_utf8_bom(char **text, size_t len)\n+{\n+\tif (len < strlen(utf8_bom) ||\n+\t    memcmp(*text, utf8_bom, strlen(utf8_bom)))\n+\t\treturn 0;\n+\t*text += strlen(utf8_bom);\n+\treturn 1;\n+}\ndiff --git a/utf8.h b/utf8.h\nindex e4d9183..e7b2aa4 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -13,6 +13,9 @@ int same_encoding(const char *, const char *);\n __attribute__((format (printf, 2, 3)))\n int utf8_fprintf(FILE *, const char *, ...);\n \n+extern const char utf8_bom[];\n+extern int skip_utf8_bom(char **, size_t);\n+\n void strbuf_add_wrapped_text(struct strbuf *buf,\n \t\tconst char *text, int indent, int indent2, int width);\n void strbuf_add_wrapped_bytes(struct strbuf *buf, const char *data, int len,\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259521","messageId":"1429206774-10087-3-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"1429206774-10087-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] config: use utf8_bom[] from utf.[ch] in git_parse_source()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T17:52:53Z","receivedAt":"2015-04-16T17:52:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Because the function reads one character at the time, unfortunately\nwe cannot use the easier skip_utf8_bom() helper, but at least we do\nnot have to duplicate the constant string this way.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n config.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 752e2e2..9618aa4 100644\n--- a/config.c\n+++ b/config.c\n@@ -12,6 +12,7 @@\n #include \"quote.h\"\n #include \"hashmap.h\"\n #include \"string-list.h\"\n+#include \"utf8.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -412,8 +413,7 @@ static int git_parse_source(config_fn_t fn, void *data)\n \tstruct strbuf *var = &cf->var;\n \n \t/* U+FEFF Byte Order Mark in UTF8 */\n-\tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\n-\tconst unsigned char *bomptr = utf8_bom;\n+\tconst char *bomptr = utf8_bom;\n \n \tfor (;;) {\n \t\tint c = get_next_char();\n@@ -421,7 +421,7 @@ static int git_parse_source(config_fn_t fn, void *data)\n \t\t\t/* We are at the file beginning; skip UTF8-encoded BOM\n \t\t\t * if present. Sane editors won't put this in on their\n \t\t\t * own, but e.g. Windows Notepad will do it happily. */\n-\t\t\tif ((unsigned char) c == *bomptr) {\n+\t\t\tif (c == (*bomptr & 0377)) {\n \t\t\t\tbomptr++;\n \t\t\t\tcontinue;\n \t\t\t} else {\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259523","messageId":"1429206774-10087-4-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"1429206774-10087-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] attr: skip UTF8 BOM at the beginning of the input file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T17:52:54Z","receivedAt":"2015-04-16T17:52:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n attr.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex cd54697..7c530f4 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -12,6 +12,7 @@\n #include \"exec_cmd.h\"\n #include \"attr.h\"\n #include \"dir.h\"\n+#include \"utf8.h\"\n \n const char git_attr__true[] = \"(builtin)true\";\n const char git_attr__false[] = \"\\0(builtin)false\";\n@@ -369,8 +370,12 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)\n \t\treturn NULL;\n \t}\n \tres = xcalloc(1, sizeof(*res));\n-\twhile (fgets(buf, sizeof(buf), fp))\n-\t\thandle_attr_line(res, buf, path, ++lineno, macro_ok);\n+\twhile (fgets(buf, sizeof(buf), fp)) {\n+\t\tchar *bufp = buf;\n+\t\tif (!lineno)\n+\t\t\tskip_utf8_bom(&bufp, strlen(bufp));\n+\t\thandle_attr_line(res, bufp, path, ++lineno, macro_ok);\n+\t}\n \tfclose(fp);\n \treturn res;\n }\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259526","messageId":"20150416181407.GA12517@peff.net","threadId":"39093","inReplyTo":"1429206774-10087-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 1/3] utf8-bom: introduce skip_utf8_bom() helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-04-16T18:14:07Z","receivedAt":"2015-04-16T18:14:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 16, 2015 at 10:52:52AM -0700, Junio C Hamano wrote:\n\n> @@ -576,10 +576,8 @@ int add_excludes_from_file_to_list(const char *fname,\n>  \n>  \tel->filebuf = buf;\n>  \n> -\tif (size >= 3 && !memcmp(buf, utf8_bom, 3))\n> -\t\tentry = buf + 3;\n> -\telse\n> -\t\tentry = buf;\n> +\tentry = buf;\n> +\tskip_utf8_bom(&entry, size);\n>  \n>  \tfor (i = 0; i < size; i++) {\n>  \t\tif (buf[i] == '\\n') {\n\nI'm surprised that in both yours and the original that we do not need to\nsubtract 3 from \"size\".\n\nIt looks like we advance \"entry\" here, not \"buf\", and then iterate over\n\"buf\". But I think that makes the later logic weird:\n\n   if (entry != buf + i && entry[0] != '#')\n\nbecause if there is a BOM, we end up with \"entry > buf + i\", which I\nthink this code isn't expecting. I'm not sure it does anything bad, but\nI think it might be simpler as just:\n\n  /* save away the \"real\" copy for later, as we do now */\n  el->filebuf = buf;\n\n  /*\n   * now pretend as if the BOM was not there at all by advancing\n   * the pointer and shrinking the size\n   */\n  skip_utf8_bom(&buf, &size);\n\n  /*\n   * and now we do our usual magic with \"entry\"\n   */\n  entry = buf;\n  for (i = 0; i < size; i++)\n     ...\n\n-Peff\n"},{"id":"259527","messageId":"xmqqk2xchqzg.fsf@gitster.dls.corp.google.com","threadId":"39093","inReplyTo":"20150416181407.GA12517@peff.net","subject":"Re: [PATCH 1/3] utf8-bom: introduce skip_utf8_bom() helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T18:23:31Z","receivedAt":"2015-04-16T18:23:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Apr 16, 2015 at 10:52:52AM -0700, Junio C Hamano wrote:\n>\n>> @@ -576,10 +576,8 @@ int add_excludes_from_file_to_list(const char *fname,\n>>  \n>>  \tel->filebuf = buf;\n>>  \n>> -\tif (size >= 3 && !memcmp(buf, utf8_bom, 3))\n>> -\t\tentry = buf + 3;\n>> -\telse\n>> -\t\tentry = buf;\n>> +\tentry = buf;\n>> +\tskip_utf8_bom(&entry, size);\n>>  \n>>  \tfor (i = 0; i < size; i++) {\n>>  \t\tif (buf[i] == '\\n') {\n>\n> I'm surprised that in both yours and the original that we do not need to\n> subtract 3 from \"size\".\n\nOr we start scanning from the beginning of \"buf\", i.e.\n\n\tfor (i = 0; i < size; i++)\n\nAfter you pointed it out, I wondered why we do not adjust the\ninitial value of \"i\" (without futzing with \"size\").  But...\n\n> It looks like we advance \"entry\" here, not \"buf\", and then iterate over\n> \"buf\". But I think that makes the later logic weird:\n>\n>    if (entry != buf + i && entry[0] != '#')\n>\n> because if there is a BOM, we end up with \"entry > buf + i\", which I\n> think this code isn't expecting. I'm not sure it does anything bad, but\n> I think it might be simpler as just:\n>\n>   /* save away the \"real\" copy for later, as we do now */\n>   el->filebuf = buf;\n>\n>   /*\n>    * now pretend as if the BOM was not there at all by advancing\n>    * the pointer and shrinking the size\n>    */\n>   skip_utf8_bom(&buf, &size);\n>\n>   /*\n>    * and now we do our usual magic with \"entry\"\n>    */\n>   entry = buf;\n>   for (i = 0; i < size; i++)\n>      ...\n\n... this would work much better for this caller.\n\nThanks.\n"},{"id":"259528","messageId":"1429208863.3097.19.camel@elego.de","threadId":"39093","inReplyTo":"xmqqoamohu2m.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] dir: allow a BOM at the beginning of exclude files","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2015-04-16T18:27:43Z","receivedAt":"2015-04-16T18:27:43Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Thu, 2015-04-16 at 10:16 -0700, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > On Thu, Apr 16, 2015 at 08:39:55AM -0700, Junio C Hamano wrote:\n> >\n> >> >  test_expect_success 'status untracked directory with --ignored' '\n> >> >  \techo \"ignored\" >.gitignore &&\n> >> > +\tsed -e \"s/^/\\xef\\xbb\\xbf/\" .gitignore >.gitignore.new &&\n> >> > +\tmv .gitignore.new .gitignore &&\n> >> \n> >> Is this \"write literal in \\xHEX on the replacement side of sed\n> >> substitution\" potable?  In any case, replacing the above three with\n> >> something like:\n> >> \n> >> \tprintf \"<bom>ignored\\n\" >.gitignore\n> >> \n> >> may be more sensible, no?\n> >\n> > I'm not sure about sed, but I agree it is suspect. And note that printf\n> > with hex codes is not portable, either You have to use octal:\n> >\n> >   printf '\\357\\273\\277ignored\\n' >.gitignore\n> >\n> > Also, as a nit, I'd much rather see this in its own test rather than\n> > crammed into another test_expect_success. It's much easier to diagnose\n> > failures if the test description mentions the goal, and it is not tied\n> > up with testing other parts that might fail.\n> \n> Yeah, I totally agree.\n> \n> Carlos, something like this squashed in, perhaps?\n> \n>  t/t7061-wtstatus-ignore.sh | 11 +++++++++--\n>  1 file changed, 9 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\n> index 0a06fbf..cdc0747 100755\n> --- a/t/t7061-wtstatus-ignore.sh\n> +++ b/t/t7061-wtstatus-ignore.sh\n> @@ -13,8 +13,6 @@ EOF\n>  \n>  test_expect_success 'status untracked directory with --ignored' '\n>  \techo \"ignored\" >.gitignore &&\n> -\tsed -e \"s/^/\\xef\\xbb\\xbf/\" .gitignore >.gitignore.new &&\n> -\tmv .gitignore.new .gitignore &&\n>  \tmkdir untracked &&\n>  \t: >untracked/ignored &&\n>  \t: >untracked/uncommitted &&\n> @@ -22,6 +20,15 @@ test_expect_success 'status untracked directory with --ignored' '\n>  \ttest_cmp expected actual\n>  '\n>  \n> +test_expect_success 'same with gitignore starting with BOM' '\n> +\tprintf \"\\357\\273\\277ignored\\n\" >.gitignore &&\n> +\tmkdir -p untracked &&\n> +\t: >untracked/ignored &&\n> +\t: >untracked/uncommitted &&\n> +\tgit status --porcelain --ignored >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n>  cat >expected <<\\EOF\n>  ?? .gitignore\n>  ?? actual\n> \n\nYeah, that makes sense. I had something similar in my patch at one point\nbefore going with modifying the current one.\n\n   cmn\n"},{"id":"259529","messageId":"1429209548-32297-1-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"xmqqoamohu2m.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2 0/4] UTF8 BOM follow-up","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T18:39:04Z","receivedAt":"2015-04-16T18:39:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is on top of the \".gitignore can start with UTF8 BOM\" patch\nfrom Carlos.\n\nSecond try; the first patch is new to clarify the logic in the\ncodeflow after Carlos's patch, and the second one has been adjusted\naccordingly.\n\nJunio C Hamano (4):\n  add_excludes_from_file: clarify the bom skipping logic\n  utf8-bom: introduce skip_utf8_bom() helper\n  config: use utf8_bom[] from utf.[ch] in git_parse_source()\n  attr: skip UTF8 BOM at the beginning of the input file\n\n attr.c   |  9 +++++++--\n config.c |  6 +++---\n dir.c    | 10 +++++-----\n utf8.c   | 11 +++++++++++\n utf8.h   |  3 +++\n 5 files changed, 29 insertions(+), 10 deletions(-)\n\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259531","messageId":"1429209548-32297-2-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"1429209548-32297-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 1/4] add_excludes_from_file: clarify the bom skipping logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T18:39:05Z","receivedAt":"2015-04-16T18:39:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even though the previous step shifts where the \"entry\" begins, we\nstill iterate over the original buf[], which may begin with the\nUTF-8 BOM we are supposed to be skipping.  At the end of the first\nline, the code grabs the contents of it starting at \"entry\", so\nthere is nothing wrong per-se, but the logic looks really confused.\n\nInstead, move the buf pointer and shrink its size, to truly\npretend that UTF-8 BOM did not exist in the input.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n dir.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 10c1f90..b5bb389 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -576,10 +576,11 @@ int add_excludes_from_file_to_list(const char *fname,\n \n \tel->filebuf = buf;\n \n-\tif (size >= 3 && !memcmp(buf, utf8_bom, 3))\n-\t\tentry = buf + 3;\n-\telse\n-\t\tentry = buf;\n+\tif (size >= 3 && !memcmp(buf, utf8_bom, 3)) {\n+\t\tbuf += 3;\n+\t\tsize -= 3;\n+\t}\n+\tentry = buf;\n \n \tfor (i = 0; i < size; i++) {\n \t\tif (buf[i] == '\\n') {\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259530","messageId":"1429209548-32297-3-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"1429209548-32297-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 2/4] utf8-bom: introduce skip_utf8_bom() helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T18:39:06Z","receivedAt":"2015-04-16T18:39:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the recent change to ignore the UTF8 BOM at the beginning of\n.gitignore files, we now have two codepaths that do such a skipping\n(the other one is for reading the configuration files).\n\nIntroduce utf8_bom[] constant string and skip_utf8_bom() helper\nand teach .gitignore code how to use it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n dir.c  |  9 ++++-----\n utf8.c | 11 +++++++++++\n utf8.h |  3 +++\n 3 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex b5bb389..4c4bf91 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -12,6 +12,7 @@\n #include \"refs.h\"\n #include \"wildmatch.h\"\n #include \"pathspec.h\"\n+#include \"utf8.h\"\n \n struct path_simplify {\n \tint len;\n@@ -538,7 +539,6 @@ int add_excludes_from_file_to_list(const char *fname,\n \tstruct stat st;\n \tint fd, i, lineno = 1;\n \tsize_t size = 0;\n-\tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\n \tchar *buf, *entry;\n \n \tfd = open(fname, O_RDONLY);\n@@ -576,10 +576,9 @@ int add_excludes_from_file_to_list(const char *fname,\n \n \tel->filebuf = buf;\n \n-\tif (size >= 3 && !memcmp(buf, utf8_bom, 3)) {\n-\t\tbuf += 3;\n-\t\tsize -= 3;\n-\t}\n+\tif (skip_utf8_bom(&buf, size))\n+\t\tsize -= buf - el->filebuf;\n+\n \tentry = buf;\n \n \tfor (i = 0; i < size; i++) {\ndiff --git a/utf8.c b/utf8.c\nindex 520fbb4..28e6d76 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -633,3 +633,14 @@ int is_hfs_dotgit(const char *path)\n \n \treturn 1;\n }\n+\n+const char utf8_bom[] = \"\\357\\273\\277\";\n+\n+int skip_utf8_bom(char **text, size_t len)\n+{\n+\tif (len < strlen(utf8_bom) ||\n+\t    memcmp(*text, utf8_bom, strlen(utf8_bom)))\n+\t\treturn 0;\n+\t*text += strlen(utf8_bom);\n+\treturn 1;\n+}\ndiff --git a/utf8.h b/utf8.h\nindex e4d9183..e7b2aa4 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -13,6 +13,9 @@ int same_encoding(const char *, const char *);\n __attribute__((format (printf, 2, 3)))\n int utf8_fprintf(FILE *, const char *, ...);\n \n+extern const char utf8_bom[];\n+extern int skip_utf8_bom(char **, size_t);\n+\n void strbuf_add_wrapped_text(struct strbuf *buf,\n \t\tconst char *text, int indent, int indent2, int width);\n void strbuf_add_wrapped_bytes(struct strbuf *buf, const char *data, int len,\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259533","messageId":"1429209548-32297-4-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"1429209548-32297-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 3/4] config: use utf8_bom[] from utf.[ch] in git_parse_source()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T18:39:07Z","receivedAt":"2015-04-16T18:39:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Because the function reads one character at the time, unfortunately\nwe cannot use the easier skip_utf8_bom() helper, but at least we do\nnot have to duplicate the constant string this way.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n config.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 752e2e2..9618aa4 100644\n--- a/config.c\n+++ b/config.c\n@@ -12,6 +12,7 @@\n #include \"quote.h\"\n #include \"hashmap.h\"\n #include \"string-list.h\"\n+#include \"utf8.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -412,8 +413,7 @@ static int git_parse_source(config_fn_t fn, void *data)\n \tstruct strbuf *var = &cf->var;\n \n \t/* U+FEFF Byte Order Mark in UTF8 */\n-\tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\n-\tconst unsigned char *bomptr = utf8_bom;\n+\tconst char *bomptr = utf8_bom;\n \n \tfor (;;) {\n \t\tint c = get_next_char();\n@@ -421,7 +421,7 @@ static int git_parse_source(config_fn_t fn, void *data)\n \t\t\t/* We are at the file beginning; skip UTF8-encoded BOM\n \t\t\t * if present. Sane editors won't put this in on their\n \t\t\t * own, but e.g. Windows Notepad will do it happily. */\n-\t\t\tif ((unsigned char) c == *bomptr) {\n+\t\t\tif (c == (*bomptr & 0377)) {\n \t\t\t\tbomptr++;\n \t\t\t\tcontinue;\n \t\t\t} else {\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259532","messageId":"1429209548-32297-5-git-send-email-gitster@pobox.com","threadId":"39093","inReplyTo":"1429209548-32297-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 4/4] attr: skip UTF8 BOM at the beginning of the input file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-16T18:39:08Z","receivedAt":"2015-04-16T18:39:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n attr.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex cd54697..7c530f4 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -12,6 +12,7 @@\n #include \"exec_cmd.h\"\n #include \"attr.h\"\n #include \"dir.h\"\n+#include \"utf8.h\"\n \n const char git_attr__true[] = \"(builtin)true\";\n const char git_attr__false[] = \"\\0(builtin)false\";\n@@ -369,8 +370,12 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)\n \t\treturn NULL;\n \t}\n \tres = xcalloc(1, sizeof(*res));\n-\twhile (fgets(buf, sizeof(buf), fp))\n-\t\thandle_attr_line(res, buf, path, ++lineno, macro_ok);\n+\twhile (fgets(buf, sizeof(buf), fp)) {\n+\t\tchar *bufp = buf;\n+\t\tif (!lineno)\n+\t\t\tskip_utf8_bom(&bufp, strlen(bufp));\n+\t\thandle_attr_line(res, bufp, path, ++lineno, macro_ok);\n+\t}\n \tfclose(fp);\n \treturn res;\n }\n-- \n2.4.0-rc2-171-g98ddf7f\n"},{"id":"259536","messageId":"20150416192629.GA13601@peff.net","threadId":"39093","inReplyTo":"1429209548-32297-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v2 0/4] UTF8 BOM follow-up","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-04-16T19:26:30Z","receivedAt":"2015-04-16T19:26:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 16, 2015 at 11:39:04AM -0700, Junio C Hamano wrote:\n\n> This is on top of the \".gitignore can start with UTF8 BOM\" patch\n> from Carlos.\n> \n> Second try; the first patch is new to clarify the logic in the\n> codeflow after Carlos's patch, and the second one has been adjusted\n> accordingly.\n> \n> Junio C Hamano (4):\n>   add_excludes_from_file: clarify the bom skipping logic\n>   utf8-bom: introduce skip_utf8_bom() helper\n>   config: use utf8_bom[] from utf.[ch] in git_parse_source()\n>   attr: skip UTF8 BOM at the beginning of the input file\n\nThis one looks OK to me. The manual adjustment of \"size\" in the second\none is a little funny, but I guess avoiding a pointer for that size\nparameter makes the final one (that uses strlen) a lot easier.\n\n-Peff\n"},{"id":"259606","messageId":"55318CE2.1000706@gmail.com","threadId":"39093","inReplyTo":"1429209548-32297-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v2 0/4] UTF8 BOM follow-up","fromName":"Karsten Blees","fromEmail":"karsten.blees@gmail.com","sentAt":"2015-04-17T22:44:50Z","receivedAt":"2015-04-17T22:44:50Z","isPatch":true,"sender":{"key":"karsten.blees@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1111200?v=4"},"body":"Am 16.04.2015 um 20:39 schrieb Junio C Hamano:\n> This is on top of the \".gitignore can start with UTF8 BOM\" patch\n> from Carlos.\n> \n> Second try; the first patch is new to clarify the logic in the\n> codeflow after Carlos's patch, and the second one has been adjusted\n> accordingly.\n> \n> Junio C Hamano (4):\n>   add_excludes_from_file: clarify the bom skipping logic\n>   utf8-bom: introduce skip_utf8_bom() helper\n>   config: use utf8_bom[] from utf.[ch] in git_parse_source()\n>   attr: skip UTF8 BOM at the beginning of the input file\n> \n\n\nWouldn't it be better to just strip the BOM on commit, e.g. via a clean filter or pre-commit hook (as suggested in [1])? Or is this patch series only meant to supplement such a solution (i.e. only strip the BOM when reading files from the working-copy rather than the committed tree)?\n\n\nAccording to rfc3629 chapter 6 [2], the use of a BOM as encoding signature should be forbidden if the encoding is *known* to be always UTF-8. And .gitignore, .gitattributes and .gitmodules contain path names, which are always UTF-8 as of Git for Windows v1.7.10.\n\nIOW, allowing a BOM would mean that files *without* BOM are *not* UTF-8 and need to be decoded from e.g. system encoding (which unfortunately cannot be set to UTF-8 on Windows). But this makes no sense as the repository would not be portable. E.g. a .gitattributes file created on a Greek Windows, containing greek path names in Cp1253, would not work on platforms with different encoding.\n\nOn the other hand, just ignoring the BOM (as this patch series does) leaves us with two alternative binary representations of the same content file...i.e. we'll eventually end up with spurious 1st line changes as users add / remove BOMs from committed .git[ignore|attributes|modules] files, depending on their editor preference...\n\n\nFor local files (.gitconfig, .git/info/exclude, .git/COMMIT_EDITMSG...), auto-detecting encoding based on the presence of a BOM makes somewhat more sense. However, this will most likely break editors that follow the recommendation of the Unicode specification (\"Use of a BOM is neither required nor recommended for UTF-8\" [3]). So we'd probably need a core.editorEncoding or core.editorUseBom setting to tell git whether \"no BOM\" means UTF-8 or system encoding...\n\nJust as a reminder: we should update the Git for Windows Unicode document [4] if we improve support for BOM-adamant editors.\n\nCheers,\nKarsten\n\n[1] http://stackoverflow.com/questions/27223985/git-ignore-bom-prevent-git-diff-from-showing-byte-order-mark-changes\n[2] https://tools.ietf.org/html/rfc3629\n[3] http://www.unicode.org/versions/Unicode7.0.0/ch02.pdf  p.40\n[4] https://github.com/msysgit/msysgit/wiki/Git-for-Windows-Unicode-Support#editor\n"},{"id":"259710","messageId":"xmqqegnejwq7.fsf@gitster.dls.corp.google.com","threadId":"39093","inReplyTo":"55318CE2.1000706@gmail.com","subject":"Re: [PATCH v2 0/4] UTF8 BOM follow-up","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-20T21:50:08Z","receivedAt":"2015-04-20T21:50:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karsten Blees <karsten.blees@gmail.com> writes:\n\n> Wouldn't it be better to just strip the BOM on commit, e.g. via a\n> clean filter or pre-commit hook (as suggested in [1])?\n\nThe users can do whatever they want and if they think having a BOM\nin these files is a bad idea, I'd encourage them to use whatever\nmeans to ensure that.  The code and history hygiene is a good thing.\n\nBut you should realize that $HOME/.gitconfig, $GIT_DIR/info/exclude,\n$GIT_DIR/config, etc.  are not even committed files in the first\nplace.  These are not even defined to be \"UTF-8 only\" by us.  Their\ncontents is entirely up to the end users.\n\nHere with these changes, we are only being nice to the users by\nstripping a well-known two-byte sequence that is known to be left\ncommonly by some tools users would use. In a sense, this is the same\ndegree of niceness that we strip the CR at the end of the line\nbefore LF.  Just like you _could_ have said these files must be\nencoded in UTF-8 and must not have BOM at the beginning, we _could_\nhave defined that these files must be recorded with LF end-of-line.\nBut obviously we don't, as there is no need to make lives of end\nusers unnecessarily more complex, and it is easy to help users use\nboth LF and CRLF with simply stripping on our reader's side.  We do\nthis BOM stripping for the same reason to make it easier for users.\n"}]}