all: Figuring out line-endings for Windows

[minux]

I have core.autocrlf set to yes, and can reproduce this problem.

http://build.golang.org/log/c34d0dabf9c4e187a2120709e7b1e61541ad9e0a

c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\cgocall.go:160: args escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\chan.go:67: t escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\chan.go:257: c escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\chan.go:544: t escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\chan.go:593: t escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\cpuprof.go:104: hz escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\env_posix.go:42: arg escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\env_posix.go:52: arg escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\extern.go:94: rpc escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\hashmap.go:237: t escapes to heap, not allowed in runtime.
c:\gobuilder\windows-386-d3072127ccac\go\src\runtime\hashmap.go:237: too many errors
lib9

[minux]

OK. Found the culprit. cmd/gc/lex.c:getlinepragma() couldn't handle crlf EOL.

[minux]

I fixed this issue, but get tired of fixing other test failures due to changes in EOL in testdata.

Here is part of the test log:

--- FAIL: TestDeflateInflateString (0.31s)
    deflate_test.go:288: level: 0, len(compress(data)) = 100019 > limit = 100018
    deflate_test.go:197: --testSync 1, 100004, 2.718281828...
    deflate_test.go:216: #0: write 0-50002
    deflate_test.go:234: #0: read 50002
    deflate_test.go:216: #1: write 50002-100004
    deflate_test.go:234: #1: read 50002
// ...
    deflate_test.go:197: --testSync 9, 100004, 2.718281828...
    deflate_test.go:216: #0: write 0-50002
    deflate_test.go:234: #0: read 50002
    deflate_test.go:216: #1: write 50002-100004
    deflate_test.go:234: #1: read 50002
FAIL
FAIL    compress/flate  0.516s
--- FAIL: Test (0.06s)
    doc_test.go:137: package error1
            got:
        // 
        PACKAGE error1

        IMPORTPATH
            testdata/error1
// ............     
FAIL
FAIL    go/doc  0.125s
--- FAIL: TestFiles (0.08s)
    printer_test.go:146: 
        length changed: len(testdata\empty.input) = 94, len(testdata\empty.golden) = 99
        testdata\empty.input:1:41: / a comment at the beginning of the file
        testdata\empty.golden:1:41: / a comment at the beginning of the file
// ....
    printer_test.go:146: 
        length changed: len(testdata\slow.input) = 2070, len(testdata\slow.golden) = 2155
        testdata\slow.input:1:54: / Copyright 2011 The Go Authors. All rights reserved.
        testdata\slow.golden:1:54: / Copyright 2011 The Go Authors. All rights reserved.

FAIL
FAIL    go/printer  0.312s
--- FAIL: TestServeFile (0.03s)
    fs_test.go:112: range="bytes=0-4": Content-Range = "bytes 0-4/12", want "bytes 0-4/11"
    fs_test.go:112: range="bytes=2-": Content-Range = "bytes 2-11/12", want "bytes 2-10/11"
    fs_test.go:119: range="bytes=2-": body = "23456789\r\n", want "23456789\r"
    fs_test.go:112: range="bytes=-5": Content-Range = "bytes 7-11/12", want "bytes 6-10/11"
    fs_test.go:119: range="bytes=-5": body = "789\r\n", want "6789\r"
    fs_test.go:112: range="bytes=3-7": Content-Range = "bytes 3-7/12", want "bytes 3-7/11"
    fs_test.go:152: range="bytes=0-0,-2": part Content-Range = "bytes 0-0/12"; want "bytes 0-0/11"
    fs_test.go:152: range="bytes=0-0,-2": part Content-Range = "bytes 10-11/12"; want "bytes 9-10/11"
    fs_test.go:161: range="bytes=0-0,-2": body = "\r\n", want "9\r"
    fs_test.go:152: range="bytes=0-1,5-8": part Content-Range = "bytes 0-1/12"; want "bytes 0-1/11"
    fs_test.go:152: range="bytes=0-1,5-8": part Content-Range = "bytes 5-8/12"; want "bytes 5-8/11"
    fs_test.go:152: range="bytes=0-1,5-": part Content-Range = "bytes 0-1/12"; want "bytes 0-1/11"
    fs_test.go:152: range="bytes=0-1,5-": part Content-Range = "bytes 5-11/12"; want "bytes 5-10/11"
    fs_test.go:161: range="bytes=0-1,5-": body = "56789\r\n", want "56789\r"
    fs_test.go:112: range="bytes=5-1000": Content-Range = "bytes 5-11/12", want "bytes 5-10/11"
    fs_test.go:119: range="bytes=5-1000": body = "56789\r\n", want "56789\r"
    fs_test.go:112: range="bytes=0-9": Content-Range = "bytes 0-9/12", want "bytes 0-9/11"
    fs_test.go:112: range="bytes=0-10": Content-Range = "bytes 0-10/12", want "bytes 0-10/11"
    fs_test.go:112: range="bytes=0-11": Content-Range = "bytes 0-11/12", want "bytes 0-10/11"
    fs_test.go:119: range="bytes=0-11": body = "0123456789\r\n", want "0123456789\r"
--- FAIL: TestServeIndexHtml (0.02s)
    fs_test.go:462: for path "/testdata/" got "index.html says hello\r\n", want "index.html says hello\n"
    fs_test.go:462: for path "/testdata/index.html" got "index.html says hello\r\n", want "index.html says hello\n"
FAIL
FAIL    net/http    9.469s
--- FAIL: TestFowler (0.02s)
    exec_test.go:336: testdata\basic.dat
    exec_test.go:466: skip: NOTE    all standard compliant implementations should pass these : 2002-05-31
    exec_test.go:472: testdata\basic.dat:2: too few fields: 
    exec_test.go:502: testdata\basic.dat:3: cannot parse result "(7,18)\r"
// ....
    exec_test.go:502: testdata\repetition.dat:137: cannot parse result "(0,6)(3,6)(6,6)\r"
    exec_test.go:472: testdata\repetition.dat:138: too few fields: 
    exec_test.go:472: testdata\repetition.dat:141: too few fields: 
    exec_test.go:538: testdata\repetition.dat:150: `(ab|a|c|bcd){4,}(d*)` should not compile
    exec_test.go:538: testdata\repetition.dat:159: `(ab|a|c|bcd){4,10}(d*)` should not compile
FAIL
FAIL    regexp  0.234s
--- FAIL: TestParseFilesWithData (0.00s)
    exec_test.go:645: test: expected
            "template1\n\ny\ntemplate2\n\nx\n"
        got
            "template1\r\n\r\ny\r\ntemplate2\r\n\r\nx\r\n"
--- FAIL: TestParseGlobWithData (0.00s)
    exec_test.go:645: test: expected
            "template1\n\ny\ntemplate2\n\nx\n"
        got
            "template1\r\n\r\ny\r\ntemplate2\r\n\r\nx\r\n"
FAIL
FAIL    text/template   0.078s

Could we please just require every windows user to set core.autocrlf to input or
we add a .gitattribute file to override the autocrlf setting?

I'm pretty sure that even if we fixed all these test failures, the sub-repos also
contains bugs like these.

[dsymonds]

No-one should set core.autocrlf to true. That's a bad move. Treat files as blobs of bytes; don't magically reinterpret them. core.autocrlf causes too much pain.

I'd be in favour of forcing core.autocrlf to input in .gitattributes.

[minux]

@dsymonds could we just create a .gitattributes file in the root directory of each branch:
* eol=lf

Is there any better way to force this? (It seems some older version of msysgit defaults core.autocrlf to true)

[dsymonds]

I would be fine with that (though it probably really only needs to be on master, not every branch). Are Windows users with stupid editors going to complain about that though?

[mattn]

@dsymonds But @rsc think go file should handle CRLF. #9264

[dsymonds]

That's an external-facing bug. go generate should work on both line endings, just as bufio.ScanLines works on both. That doesn't mean the Go source repo and all its own unit tests should need to cope with every variation; @minux's bugs here are about Go's own unit tests failing when reading their own data files.

[dsymonds]

To be more explicit, if I were to check in a net/http test that served from a 12 byte file on disk, I shouldn't have to be concerned that the test will fail on Windows because the Windows user's Git magically turned that 12 byte file into a 13 byte file.

[bradfitz]

I agree with both @rsc and @dsymonds:

Russ thinks the compiler & tools should deal with other user's CRLF line endings, if they choose to use them.

David thinks that the Go project (for our own files) should enforce Unix line endings, which means our unit tests can assume that files are disk are exactly as they were when they were submitted.

[mattn]

I don't have fixate. do it as you are thinking as best. 👍

[minux]

@dsymonds, we didn't get any complaints while using hg, which keeps line
endings, so
I think forcing eol=lf should also be fine.

[bradfitz]

@minux or @mattn, feel free to send a change to enforce that.

[minux]

To summarize, when the tree reopens (or now? @bradfitz, do you want me to
send CLs to fix the windows builders?)
I will send these two CLs;

1. a CL to fix this (the original) issue, this bug is an external facing
   one. //line and //go: pragmas couldn't handle CRLF input.
2. a CL to add .gitattributes that will force git to check out files with
   LF eol style.

[mattn]

Please note that don't set eol=lf to all of files. :)

*.go eol=lf
*.sh eol=lf
*.bat eol=crlf
*.cmd eol=crlf

[bradfitz]

Minux, you can send CLs to fix Windows now.

The reason the tree is frozen right now is just to get all our infrastructure updated to the new world (git + gerrit), but this is part of that.

[bradfitz]

@mattn, why does that matter? We never had problems with *.bat or *.cmd files in Mercurial having Unix line endings, did we?

I don't want to make exceptions based on filename.

[minux]

Right now, all the .bat files are using \n as EOL, and it's working fine,
why
should we change them?

(In fact, all the files tracked in the repository are using \n as EOL.)

[mattn]

Windows doesn't handle UNIX fileformat in batch file correctly.

http://stackoverflow.com/questions/486380/problem-with-windows-batch-file-on-windows-7

[bradfitz]

If we find a case where Windows can't deal with a unix-line-ended batch file, we'll change it in the version control system to Windows line endings.

But so far we've been okay.

[mattn]

Okay! Do it please.

[minux]

Windows does have a bug dealing with batch files in \n EOL style, however,
that only happens
when you double click the batch file.

If you run the batch file using cmd.exe, it should work fine.

I don't think user should double-click all.bat, as when the build fails,
the console window will just
close and the user won't be able to see the messages. In fact, I can barely
remember one past
issue about this problem.

[mattn]

build succeeded! cool! 👍 http://build.golang.org/

[marcopeereboom]

with .gitattributes having * eol=lf the png and jpg files get mangled.

My steps are: git clone https://github.com/golang/go.git
cd go
git diff

and now I see tons of:
warning: CRLF will be replaced by LF in doc/go-logo-blue.png.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in doc/go-logo-white.png.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in doc/gopher/appenginegophercolor.jpg.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in doc/gopher/biplane.jpg.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in doc/gopher/bumper.png.
The file will have its original line endings in your working directory.

Which of course prevents pulls.
$ git pull --ff-only --rebase --prune --all
Cannot pull with rebase: You have unstaged changes.
Please commit or stash them.

What's the remedy?

[marcopeereboom]

Actually what fixes it for me is replacing "* eol=lf" with "text eol=lf". Let me send a PR unless someone pick it up.

[minux]

Chaning "* eol=lf" to "text eol=lf" fixes the binary file problem, but then
will break the windows builders
which set core.autocrlf=yes.

I'm afraid we will have to explicit set -text for certain binary files in
the repo (or set eol=lf for all text files,
but that's hard, I've tried that. Do you expect *.raw file to be text? We
have one, see
src/go/printer/testdata/expressions.raw.

Perhaps the right way is to make cmd/dist brak when it sees core.autocrlf
is set to true/yes.

[bradfitz]

@minux, yes, let's just be strict. git should operate in everything-is-binary-files-always mode and if we detect that they're trying to use autocrlf with the Go repos, then we should fail to run the build. It could also be in make.bat if that's easier.

[minux]

How about this?

* -text

[bradfitz]

What does that mean? Everything is not text? If that does what I wanted above (treat everything as binary, always), that's fine.

[minux]

Yeah, that means everything is not text, and I just checked on Windows, it
works.
Because every file is not text, git will not try to do any eol conversion.

However, the whole system is not that simple. see
http://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line/

To recap our requirements:

1. we have files with crlf eol style in the repo (for
   example, misc/makerelease/windows/installer.wxs),
   and we probably want to keep that file in crlf eol style.
2. we don't want git to convert text files to crlf eol when checking out,
   otherwise a lot of tests will break.
3. do we want to make git convert certain sources (*.go, *.html, basically)
   with crlf style to lf style when
   checking in?

That post recommends setting:
.txt -text
if we want that "these files will never run through the CRLF to LF
replacement.", and this will
suffice for requirement 1 and 2 listed above. What about 3? Setting "
-text" will allow people to
accidentally checkin source files in crlf eol style. Should we enforce
this? In pre-commit hooks?
by git-review?

btw, the new system works from git 1.7.2 and above, so we need to require
at least git 1.7.2.

[bradfitz]

Mailed https://go-review.googlesource.com/#/c/1490/

[alexbrainman]

@minux and @bradfitz tank you for fixing this.

Alex

[dsymonds]

Sherman, no problem.

[alexbrainman]

golang.org/x/text is still broken http://build.golang.org/log/f6a9e9bc43b2156709560ce3c8fd2cbced3fd818

ok golang.org/x/text/cases 2.521s
ok golang.org/x/text/cldr 0.395s
ok golang.org/x/text/collate 0.277s
ok golang.org/x/text/collate/build 0.169s
ok golang.org/x/text/collate/colltab 0.219s
? golang.org/x/text/collate/tools/colcmp [no test files]
ok golang.org/x/text/display 0.142s
--- FAIL: TestFiles (0.04s)
encoding_test.go:736: Decode, UTF-16LE (Ignore BOM): transformed bytes did not match golden file
encoding_test.go:736: Encode, UTF-16LE (Ignore BOM): transformed bytes did not match golden file
FAIL
FAIL golang.org/x/text/encoding 0.668s
? golang.org/x/text/encoding/charmap [no test files]
? golang.org/x/text/encoding/japanese [no test files]
? golang.org/x/text/encoding/korean [no test files]
? golang.org/x/text/encoding/simplifiedchinese [no test files]
? golang.org/x/text/encoding/traditionalchinese [no test files]
? golang.org/x/text/encoding/unicode [no test files]
ok golang.org/x/text/internal/triegen 1.759s
ok golang.org/x/text/internal/ucd 0.222s
ok golang.org/x/text/language 0.119s
ok golang.org/x/text/transform 0.236s
ok golang.org/x/text/unicode/norm 4.174s

and

golang.org/x/net is broken http://build.golang.org/log/2afb4b91e984f5f4f9ef68db8d2977d47ba87c25

ok golang.org/x/net/context 2.838s
? golang.org/x/net/dict [no test files]
--- FAIL: TestParser (0.00s)
parse_test.go:220: got "#data\r\n" want "#data\n"
FAIL
FAIL golang.org/x/net/html 0.251s
ok golang.org/x/net/html/atom 0.301s
ok golang.org/x/net/html/charset 0.094s
ok golang.org/x/net/icmp 0.115s
ok golang.org/x/net/idna 0.103s
? golang.org/x/net/internal/iana [no test files]
? golang.org/x/net/internal/nettest [no test files]
ok golang.org/x/net/ipv4 0.123s
ok golang.org/x/net/ipv6 0.080s
ok golang.org/x/net/netutil 1.087s
ok golang.org/x/net/proxy 0.098s
ok golang.org/x/net/publicsuffix 0.420s
ok golang.org/x/net/spdy 0.150s
ok golang.org/x/net/webdav 0.073s
ok golang.org/x/net/websocket 0.110s

Both can be fixed by adding .gitattributes. Is that what we want to do? What about other repos? Should we update others on as needed basis?

Alex

[alexbrainman]

golang.org/x/tools needs .gitattributes too.

Alex

[bradfitz]

Alex, all yours.

[minux]

Should we just add .gitattributes to each of the sub-repos?

[bradfitz]

Yes.

[alexbrainman]

Cool. I will do that.

[alexbrainman]

I have updated all repos listed on http://build.golang.org.

[pbennett]

This is something that worked fine in 1.4 that is now broken in 1.5 (b2) for me.
I checked in 1.5 beta 2 (we check in our tooling..) and if users fetch it on a windows system (our vcs gets 'text' files in local os default format) then they can't build. An example go install fails with:

go\src\runtime\cgocall.go:144: args escapes to heap, not allowed in runtime.
go\src\runtime\chan.go:91: t escapes to heap, not allowed in runtime.
go\src\runtime\chan.go:295: c escapes to heap, not allowed in runtime.
go\src\runtime\chan.go:582: t escapes to heap, not allowed in runtime.
go\src\runtime\chan.go:632: t escapes to heap, not allowed in runtime.
go\src\runtime\env_posix.go:34: arg escapes to heap, not allowed in runtime.
go\src\runtime\env_posix.go:45: arg escapes to heap, not allowed in runtime.
go\src\runtime\extern.go:119: rpc escapes to heap, not allowed in runtime.
go\src\runtime\hashmap.go:272: t escapes to heap, not allowed in runtime.
go\src\runtime\hashmap.go:320: t escapes to heap, not allowed in runtime.
go\src\runtime\hashmap.go:320: too many errors

I'm kind of dumbstruck that the go compiler is getting confused about simple line endings when reading .go files. It doesn't seem to care at all about user source line endings.

[ianlancetaylor]

@pbennett This issue is closed. If you are having trouble with 1.5, please open a new issue with a description of how to reproduce the problem. Thanks.

[pbennett]

Opened.
#11771 #11771

On Jul 17, 2015, at 6:24 PM, Ian Lance Taylor notifications@github.com wrote:

@pbennett https://github.com/pbennett This issue is closed. If you are having trouble with 1.5, please open a new issue with a description of how to reproduce the problem. Thanks.

—
Reply to this email directly or view it on GitHub #9281 (comment).