cmd/compile: 1.5 beta 2 can't handle CRLF terminated go system source

[pbennettinin]

The easiest way to reproduce is to change all the .go files in go/src/ (just go/src/runtime is fine) to be CRLF terminated instead of LF terminated. Since we check our build tools into source control, this happens as soon as a developer on a Windows machine fetches the tools.
Upon building something that makes go install want to rebuild system libs, you'll get errors like this:

# runtime
/usr/local/go/src/runtime/cgocall.go:144: args escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/chan.go:91: t escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/chan.go:290: c escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/chan.go:577: t escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/chan.go:627: t escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/env_posix.go:34: arg escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/env_posix.go:45: arg escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/extern.go:119: rpc escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/hashmap.go:272: t escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/hashmap.go:320: t escapes to heap, not allowed in runtime.
/usr/local/go/src/runtime/hashmap.go:320: too many errors

This looks like the same issue that was supposedly already fixed in #9281

This is not an issue in 1.4.

[ianlancetaylor]

The upshot of issue #9281 was that the Go repo itself can only be built if the file use Unix-style line endings. Anything else got too complex, because of changes to test files. So, in the general case, what you are doing is not expected to work.

However, it does seem to me that the compiler should support the specific case of magic comments in files with CRLF line endings. The Go spec says that carriage returns are ignored. The Go spec does not discuss the special case of magic comments, but it seems to me that for them too carriage returns should be ignored.

[gopherbot]

CL https://golang.org/cl/12379 mentions this issue.

[pbennettinin]

Any ‘text’ file WILL have different line endings depending on the platform. This is not a new phenomenon. So, any test case requiring a specific line ending in a text file (which I would argue is itself broken), should ensure that file is in the format it needs one way or the other. Whether that means it just reads an embedded string constant with escape sequences, writes that out to a file first (if using file IO is required in the test), or even translates after reading it in.

One of the tests mentioned in #9281 for eg. is just reading an 11 byte text file from disk and expecting an exact BYTE size - why? There’s no reason for this at all. Read it as a text file, correctly (Scanner and Reader.ReadLine already handle this properly) and it’s a non-issue. Worst case, it could have those 11 bytes as a local constant.

Ian, it looks like you’ve already got a diff up fixing the Go compiler itself being broken by EOL differences which is great, but technically, test cases failing because of ‘text’ file differences is broken as well.
For my immediate needs the former is really all I require but the latter staying broken would be unfortunate I think.

Thanks for all the great work in 1.5 guys…

[ianlancetaylor]

To fix the test cases, we simply told git that every file in the Go source tree is a binary file, and that none should be treated as a text file. That makes the right thing happen for the Go source tree. Anybody who wants to copy the Go source tree out of git needs to do the same thing. That is, perhaps, slightly onerous, but it's simple.

[ianlancetaylor]

I'm going to mark this as fixed. We aren't going to change the Go sources to support CRLF endings in the test files. Mark the test files as binary.

[pbennett]

Thanks for the fix. As for the test cases, will ALL test files be in testdata/ directories? If so, it's easy to just set up a rule for that.

[bradfitz]

No promises. But probably. We may use a *.go file as a test data file in the future. You've chosen to do something unique, so now you get to deal with the consequences. You're using the Go tree in an unsupported (by us) way. I recommend that when you vendor the Go sources into your version control system, you mark the whole subtree as binary.

[pbennett]

What is it about what I'm asking or doing that you think is unique?
Clearly, 'text' files having different line endings isn't something unique to me. Even ignoring the historical reasons for CRLF, LFCR, LF, or CR endings, most text based internet protocols in fact dictate CRLF. Besides the expected CRLF, HTTP even has acceptance of both bare LF and CR as required. Source files consumed by compilers have historically never cared either.

So if you have test code that explicitly requires 'text' files they work on to remain in a very specific format then those files should be named in such a way as to make this clear or they should write those files themselves (or work in memory). Ideally, they would be written properly and not care. If you guys want to kick the can and mark your entire source tree as 'binary,' that's certainly your right, but to imply that me having an expectation of properly handling text files in development, runtime, and in source code management is somehow unique to me is really strange.

So either that, or you're saying me 'vendoring' Go itself is unique? If so, that's just the nature of our business. If we have to support and patch a product that's ten years old then we need all the tools used to be versioned alongside everything else so it can still be built. Developers don't install much of anything if we can avoid it. It's all checked in and versioned appropriately and developers fetch the code to build as well as the code that builds it all together. Recent Microsoft tools (C++ from v8 on) being a perfect example of garbage that can't simply be 'checked in.' :\

[bradfitz]

We also vendor Go and patches. That's fine.

Nothing in Go the language or resulting binaries cares about line endings. The only places that do are some of our standard library tests, just for convenience. It's easier for us to treat everything as binary. Nobody using Go has to write their tests this way, but we do. If you vendor Go's tests (which is fine), just be aware that we assume the world is binary in our tests. If you don't want to assume that, it's on you to also carry patches in your vendored tree to "fix" the tests for the updated world's assumptions. (your world with CRLF)