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.

Sign up for GitHub

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

Jump to bottom

cmd/go: do not treat android.go or darwin.go or linux.go or windows.go or 386.go specially #8838

Closed
rsc opened this issue Sep 29, 2014 · 11 comments
Closed

cmd/go: do not treat android.go or darwin.go or linux.go or windows.go or 386.go specially #8838

rsc opened this issue Sep 29, 2014 · 11 comments
Labels
FrozenDueToAge
Milestone
Go1.4

Comments

@rsc
Copy link
Contributor

rsc commented Sep 29, 2014 

foo_android.go is still android-specific, and foo_newos.go is still newos-specific, but
android.go and newos.go are not.

This will break code where people want darwin.go to be darwin-specific, but it will stop
breaking code when we add new GOOS values as long as people do not put _ in their file
names.
@rsc
Copy link
Contributor Author

rsc commented Sep 29, 2014

Comment 1:

Labels changed: added release-go1.4.

@griesemer
Copy link
Contributor

Comment 2:

Labels changed: added repo-main.

@robpike
Copy link
Contributor

robpike commented Oct 6, 2014

Comment 3:

In go/build, the function func (ctxt *Context) goodOSArchFile(name string, allTags
map[string]bool) answers the question, "is this file included due to GOOS and GOARCH"?
I propose we add a little check at the top of that function, after dropping the suffix,
that goes something like:
// We do not consider the file "android.go" to be tagged for android because the addition
// of GOOS=android to the ecosystem breaks programs that have such files already. Thus,
// after the addition of android to the ecosystem we do not define as matching any plain
// file names that match new GOOS values directly. They match only with a prefix, as in
// "file_android.go".
switch name {
case "android":
  return
}

@ianlancetaylor
Copy link
Contributor

Comment 4:

I agree, but I think we should do it by copying the current values of goosList (minus
anroid) and goarchList and saying that those files are OK without an underscore, but new
entries are not.

@rsc
Copy link
Contributor Author

rsc commented Oct 6, 2014

Comment 5:

I updated the subject. I think we should fix all of them. This will break things. But it
will break fewer things than not, and it avoids an inconsistency that will only grow.
(Why does amd64.go work but power64.go does not? Oh, that's because amd64 was one of the
early ports.)

@robpike
Copy link
Contributor

robpike commented Oct 6, 2014

Comment 7:

/Users/r/go/src/cmd/dist/arm.c
/Users/r/go/src/cmd/dist/plan9.c
/Users/r/go/src/cmd/dist/windows.c
/Users/r/go/src/lib9/windows.c
/Users/r/src/code.google.com/p/go.text/collate/tools/colcmp/darwin.go

@rsc
Copy link
Contributor Author

rsc commented Oct 6, 2014

Comment 8:

Re #7, only code.google.com/p/go.text/collate/tools/colcmp/darwin.go would be affected.

@robpike
Copy link
Contributor

robpike commented Oct 6, 2014

Comment 9:

Status changed to Started.

@gopherbot
Copy link

Comment 10:

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

@robpike
Copy link
Contributor

robpike commented Oct 6, 2014

Comment 11:

This issue was closed by revision d396b9d.

Status changed to Fixed.

@bradfitz
Copy link
Contributor

bradfitz commented Oct 6, 2014

Comment 12:

This issue was updated by revision f8f9559.

LGTM=r
R=r
CC=golang-codereviews, rsc
https://golang.org/cl/152220045

@rsc rsc added fixed labels Oct 6, 2014
@rsc rsc self-assigned this Oct 6, 2014
@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@robpike @wheatman
go/build: do not consider "android.go" to be android-specific
ed83053 
A file name must have a non-empty underscore-separated
prefix before its suffix matches GOOS. This is what the
documentation already said but is not what the code did.

Fixes golang#8838.

This needs to be called out in the release notes.
The he single affected file
        code.google.com/p/go.text/collate/tools/colcmp/darwin.go
could use a renaming but works because it has a build tag inside.

LGTM=adg, rsc
R=golang-codereviews, adg, rsc
CC=golang-codereviews
https://golang.org/cl/147690043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@robpike @wheatman
go/build: do not consider "android.go" to be android-specific
3d4910f 
A file name must have a non-empty underscore-separated
prefix before its suffix matches GOOS. This is what the
documentation already said but is not what the code did.

Fixes golang#8838.

This needs to be called out in the release notes.
The he single affected file
        code.google.com/p/go.text/collate/tools/colcmp/darwin.go
could use a renaming but works because it has a build tag inside.

LGTM=adg, rsc
R=golang-codereviews, adg, rsc
CC=golang-codereviews
https://golang.org/cl/147690043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@robpike @wheatman
go/build: do not consider "android.go" to be android-specific
06519e4 
A file name must have a non-empty underscore-separated
prefix before its suffix matches GOOS. This is what the
documentation already said but is not what the code did.

Fixes golang#8838.

This needs to be called out in the release notes.
The he single affected file
        code.google.com/p/go.text/collate/tools/colcmp/darwin.go
could use a renaming but works because it has a build tag inside.

LGTM=adg, rsc
R=golang-codereviews, adg, rsc
CC=golang-codereviews
https://golang.org/cl/147690043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@robpike @wheatman
go/build: do not consider "android.go" to be android-specific
03287ae 
A file name must have a non-empty underscore-separated
prefix before its suffix matches GOOS. This is what the
documentation already said but is not what the code did.

Fixes golang#8838.

This needs to be called out in the release notes.
The he single affected file
        code.google.com/p/go.text/collate/tools/colcmp/darwin.go
could use a renaming but works because it has a build tag inside.

LGTM=adg, rsc
R=golang-codereviews, adg, rsc
CC=golang-codereviews
https://golang.org/cl/147690043
@rsc rsc removed their assignment Jun 23, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.4
Development

No branches or pull requests
6 participants
@bradfitz @rsc @ianlancetaylor @robpike @griesemer @gopherbot