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

Jump to bottom

cmd/go: consider matching newly added _goos.go, _goarch.go or _goos_goarch.go suffixes based on go.mod go version (rather than Go toolchain version only) #44626

Open
ConradIrwin opened this issue Feb 26, 2021 · 6 comments
Open

cmd/go: consider matching newly added _goos.go, _goarch.go or _goos_goarch.go suffixes based on go.mod go version (rather than Go toolchain version only) #44626

ConradIrwin opened this issue Feb 26, 2021 · 6 comments
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Unfortunate
Milestone
Backlog

Comments

@ConradIrwin
Copy link
Contributor

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

$ go version
go version go1.16 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/conrad/Library/Caches/go-build"
GOENV="/Users/conrad/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/conrad/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/conrad/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.16/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.16/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/conrad/superhuman/backend/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vm/zrl4q9b56y76b3y_d7zspppr0000gn/T/go-build3563792657=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I upgraded to go1.16 and ran go test.

What did you expect to see?

No failures

What did you see instead?

My build failed because I had a file called send_log_ios.go. In go before 1.16, this file was included in the build (because there was no ios build tag). In go 1.16 this file is excluded from the build because it assumed to match the ios build tag.

My go.mod still lists go 1.15.

This is not a problem for me (I can just rename the file), but I do think this is surprising and a breakage of the go1 compatibility promise – a program that used to build no longer builds because I upgraded my version of go.

I would propose that this change should only happen in module mode if I change the version in go.mod.
@ConradIrwin ConradIrwin changed the title compile: in go1.16 files with names like _ios.go are no longer included compile: in go1.16 files with names like _ios.go are no longer built by default Feb 26, 2021
@ianlancetaylor
Copy link
Contributor

Yes, ios is now considered to be a valid GOOS value, so it applies to file names as with other GOOS values.

I'm not sure whether it makes sense to adjust this based on the language in the go.mod file. That is more intended for language changes, and this is not a language change. This is going to be a surprising change no matter when it happens, and I don't see why it is better for it to happen when the language version is updated than when updating to a new release.

@gopherbot
Copy link

Change https://golang.org/cl/296849 mentions this issue: _content/doc/go1.16: explicit point out ios as a build tag

@ianlancetaylor ianlancetaylor changed the title compile: in go1.16 files with names like _ios.go are no longer built by default cmd/go: in go1.16 files with names like _ios.go are no longer built by default Feb 26, 2021
@ConradIrwin
Copy link
Contributor Author

ConradIrwin commented Mar 1, 2021 via email

@dmitshur dmitshur added the GoCommand cmd/go label Mar 5, 2021
@dmitshur dmitshur added this to the Backlog milestone Mar 5, 2021
@dmitshur dmitshur added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 5, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Mar 5, 2021 •
edited

I agree this is working as intended, and it's consistent with the behavior when new GOOS/GOARCH values were reserved in the past.

Since this issue is still open, I've added a NeedsInvestigation label so it can be used to determine if anything should be done differently in the future (e.g., evaluate whether it'd be better to tie the change to go.mod's version as you suggested).

CC @bcmills, @matloob, @jayconrod.

@dmitshur dmitshur changed the title cmd/go: in go1.16 files with names like _ios.go are no longer built by default cmd/go: consider matching newly added _goos.go, _goarch.go or _goos_goarch.go suffixes based on go.mod go version (rather than Go toolchain version only) Mar 6, 2021
@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Mar 6, 2021
@bcmills
Copy link
Contributor

bcmills commented Mar 8, 2021

The go version intentionally does not gate explicit build tags, even ones corresponding to GOOS or GOARCH values. However, for any future expansions to GOARCH and GOOS, perhaps we should make the newly-added implicit build constraints contingent on the go language version in the go.mod file.

Changing the interpretation of an existing filename (from a non-constraint to a constraint or vice-versa) seems like it is pretty clearly a redefinition (in the sense of http://golang.org/design/28221-go2-transitions#language-redefinitions). On the other hand, it would be really confusing to apply the implicit constraints for certain GOOS values but not others, and removing the implicit-build-constraint support entirely would be an equally bad redefinition.

I don't think there are any good options here.

@ianlancetaylor
Copy link
Contributor

We made an effort to add all plausible CPU values to the list of valid GOARCH values. I wonder if there are values we could add to the list of valid GOOS values.

gopherbot pushed a commit to golang/website that referenced this issue Mar 15, 2021
@ianlancetaylor
_content/doc/go1.16: explicitly point out ios as a build tag
686df8b 
For golang/go#44626
For golang/go#40700

Change-Id: I42c064f1d7a7bda42cd1de161a923720021eeda5
Reviewed-on: https://go-review.googlesource.com/c/website/+/296849
Trust: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
passionSeven added a commit to passionSeven/website that referenced this issue Oct 18, 2022
@passionSeven
_content/doc/go1.16: explicitly point out ios as a build tag
92f609b 
For golang/go#44626
For golang/go#40700

Change-Id: I42c064f1d7a7bda42cd1de161a923720021eeda5
Reviewed-on: https://go-review.googlesource.com/c/website/+/296849
Trust: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Unfortunate
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@ConradIrwin @dmitshur @ianlancetaylor @bcmills @gopherbot