Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: go1.9.4 and go1.10 break on file-names with leading comma-character (BREAKING CHANGE/regression) #24213

Closed
marystern opened this issue Mar 2, 2018 · 9 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@marystern
Copy link

What version of Go are you using (go version)?

go version go1.10 linux/amd64

Does this issue reproduce with the latest release?

yes (and only in the latest release: has worked in all release up until this one!)

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOOS="linux"

What did you do?

I have a generated file in my project with a leading comma-character (which my buld tools use to distinguish generated files). This has worked fine for many versions of go until the latest 1.10 release which complains (on go build) with:
'can't load package: package xxx: invalid input file name ",assets.go" '

What did you expect to see?

I expected it to be built as usual!

What did you see instead?

The file name appears as newly illegal and breaks my project!

(My guess is that it's new code in https://golang.org/src/cmd/go/internal/load/pkg.go for "// If first letter of input file is ASCII, it must be alphanumeric."). This seems to me to be a true regression and unintended (I can't see any new restrictions having been documented and presumably that would break the go1 promise of compatability anyway). Please fix urgently, thanks!

(My workaround is to stick with 1.9 until this bug is fixed)

@marystern
Copy link
Author

Note:
1.9.1: was working (this is the version I was using before I switched to 1.10)
1.9,2: works
1.9.3: untested
1.9.4: broken
1.10: broken
So it appears the break was introduced in 1.9.3 or 1.9.4.

@ALTree
Copy link
Member

ALTree commented Mar 2, 2018

Thanks for the report.

The building of files with a name starting with , was broken by 1dcb583 (cmd/go: accept only limited compiler and linker flags in #cgo directives).

This seems to me to be a true regression and unintended (I can't see any new restrictions having been documented and presumably that would break the go1 promise of compatibility anyway).

I don't know if this was expected (and it needs to be documented) or it wasn't. Marking this as NeedsDecision. cc @rsc

@ALTree ALTree changed the title go 1.10 breaks on file-names with leading comma-character (BREAKING CHANGE/regression) cmd/go: go1.9.4 and go1.10 break on file-names with leading comma-character (BREAKING CHANGE/regression) Mar 2, 2018
@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 2, 2018
@ianlancetaylor
Copy link
Contributor

This is a tough call. We discovered that there were security problems in using files starting with '-' or '@'. For safety, we banned files starting with anything other than letters, digits, dot, underscore, and slash. As far as I know, comma is OK in a file name. But we don't want to turn around and permit anything.

Note that the go tool is not covered by the Go 1 compatibility guarantee. See https://golang.org/doc/go1compat#tools.

@ianlancetaylor ianlancetaylor added this to the Go1.10.1 milestone Mar 2, 2018
@andybons
Copy link
Member

andybons commented Mar 4, 2018

My vote is to stick with the stricter naming convention and not allow a special case of files beginning with commas (arguably a relatively small use-case).

There are other naming methods that could be used to denote a generated file. XXX_file.go, file_GENERATED.go, file_GEN.go the list could go on.

@marystern I understand this may not be particularly nice to hear considering it requires changes to your build environment, but as Ian said, the go tool is not covered by the compatibility guarantee and this is a fairly esoteric use-case (although if you have other examples of this in the wild please let us know).

That said, my vote may be overridden anyway, so I will leave it to the rest of the team to weigh in.

@rsc
Copy link
Contributor

rsc commented Mar 5, 2018

Sorry, but no. We're intentionally restricting the syntax to avoid ambiguity with a range of tooling. Comma is already special for certain options and is good to keep out. Please rename your source files. If you want to follow the convention of the standard library, we put a z at the front. Or use any other equally good convention.

@rsc rsc closed this as completed Mar 5, 2018
@marystern
Copy link
Author

To quote from ref: https://golang.org/doc/go1compat

"It is intended that programs written to the Go 1 specification will continue to compile and run correctly, unchanged, over the lifetime of that specification.
Compatibility is at the source level.

Of course, for all of these possibilities, should they arise, we would endeavor whenever feasible to update the specification, compilers, or libraries without affecting existing code.
These caveats aside, we believe that Go 1 will be a firm foundation for the development of Go and its ecosystem.
"

Surely naming of files is included in the Go1 promise of compatability (I understand that the tools can change, but source file names are an external source API, no?)?

@rsc: re "We're intentionally restricting the syntax"
Maybe you could at a minimum document this for us poor users who don't work at Google so that we can be informed about your new restrictions and the rationale behind these decisions?

re: "Or use any other equally good convention."
Well that's exactly what we were happily doing until these latest releases broke our projects (equal is apparently not equal when it comes to inside and outside Go)!

FYI: I also work at a large multi-national corporation and work on many projects which already rely on this convention for our tooling (part of your "Go ecosystem"), so it's not just a question of "rename your source files", our whole ecosystem has to be changed.

This probably isn't the right time or place, but we've resisted getting involved in the Go2 discussions exactly for this type of dis-regard that we've noticed by the Go Authors for other people's concerns. For the record, we enjoy coding in Go and would like to evangelise more (and do greatly appreciate your time and effort, elegant solutions, simplicity, performance, etc), but when we see things like this it puts us off from encouraging other groups to enter the fray (other language communities are way more inclusive in their attitudes): sorry for the rant, but this will cost a significant amount of time and effort to change all our projects, standards and tooling, so as you can appreciate, we're not happy!

Very disappointing conclusion, but unfortunately, not entirely unexpected. :(

@cznic
Copy link
Contributor

cznic commented Mar 9, 2018

To quote from ref: https://golang.org/doc/go1compat

@marystern

Please note that Ian has already wrote eralier:

Note that the go tool is not covered by the Go 1 compatibility guarantee. See https://golang.org/doc/go1compat#tools.

And wrt yours

Surely naming of files is included in the Go1 promise of compatability (I understand that the tools can change, but source file names are an external source API, no?)?

Surely not. Only the content in your source Go files should continue to compile wrt the compatibility promise.

@ianlancetaylor
Copy link
Contributor

For the rationale for the change, see #23672. A security hole in go get was discovered and reported to us. Because the problem is subtle and it's hard to know whether it has been fixed reliably, we chose to lock down the go tool enough to feel some confidence that is, and will remain, secure. In particular note that permitting file names starting with - and @ opens security holes. While I'm not aware of any problem with starting file names with ,, it's hard to be fully confident that there isn't one, especially since , is indeed treated as a special character by some compiler options, special in a way that can open security holes.

(I'll note that even if the behavior of the Go tool were covered by the Go 1 compatibility promise, which it is not, this change would still be permitted by the promise because it is a security fix. That is the first permitted exception listed at https://golang.org/doc/go1compat.)

I'm sorry that you feel that we disregard other people's concerns. It seems to me that we do not. But we must always balance many different sets of concerns and make choices that seem to us to be best. Security concerns naturally weigh very heavily.

I'm also sorry that you feel that you should avoid taking place in the Go 2 discussions. We are doing our best to understand everyone's issues with Go in considering what changes to make in Go 2. That is the point of creating https://golang.org/wiki/ExperienceReports . If you don't feel comfortable sharing your concerns, then we won't know what they are, and it will be impossible for us to take them into account.

@marystern
Copy link
Author

@ianlancetaylor Thanks for the helpful pointers: I can understand the issue a little more now, so have calmed down! :) I still think it would be useful to document the new restrictions in the file naming as it was an unpleasant upgrade experience, and maybe others can be fore-warned. Keep up the good work, and Best Regards, (a happy gopher again), Mary.

@andybons andybons removed this from the Go1.10.1 milestone Mar 26, 2018
@golang golang locked and limited conversation to collaborators Mar 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

7 participants