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: build constraints disparity between compiler and documentation #63502

Closed
mark-pictor-csec opened this issue Oct 11, 2023 · 4 comments
Closed
Labels
Documentation FrozenDueToAge GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mark-pictor-csec
Copy link

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

Tested with go1.20 and go1.21

Does this issue reproduce with the latest release?

yes

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

Included, though I doubt it has an impact:

go env Output
$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='$HOME/Library/Caches/go-build'
GOENV='$HOME/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='$HOME/go/pkg/mod'
GONOPROXY='corp.host/repos/*'
GONOSUMDB='corp.host/repos/*'
GOOS='darwin'
GOPATH='$HOME/go'
GOPRIVATE='corp.host/repos/*'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='$HOME/sdk/go1.21.0'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='$HOME/sdk/go1.21.0/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.21.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='$HOME/go/src/corp.host/repos/blah/blah/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/mx/0hyblhts2jq55slmx983zjl80000gn/T/go-build2461970070=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

The documentation for build constraints states that constraints

must appear near the top of the file, preceded only by blank lines and other line comments.

However, during experimentation it became apparent that //go:build directives are honored even after block
comments. This didn't seem to hold true for the older //+build form.

I reached out to gophers slack's #tools channel for their input, and they thought it'd be
worth filing an issue. dominikh created https://go.dev/play/p/5jqlmjNEjOH.go showing his
results, which match mine.

What did you expect to see?

That the compiler and documentation match. If the current docs are correct, the compiler would ignore build constraints if proceeded by things other than line comments, while if the current compiler is correct, that the docs describe what the compiler allows.

For example, the docs seem to imply that the constraint should be ignored:

/* block comment*/

//go:build ignore

package etaoin

func shrdlu() {}

What did you see instead?

//go:build is honored even if following block comments.

Note: the old style //+build is ignored after block comments, matching the documentation.

@prattmic prattmic changed the title Build constraints: disparity between compiler and documentation cmd/go: build constraints disparity between compiler and documentation Oct 11, 2023
@prattmic prattmic added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 11, 2023
@prattmic
Copy link
Member

cc @rsc

@bcmills bcmills changed the title cmd/go: build constraints disparity between compiler and documentation go/build,cmd/go: build constraints disparity between compiler and documentation Oct 13, 2023
@bcmills bcmills added this to the Backlog milestone Oct 13, 2023
@bcmills bcmills changed the title go/build,cmd/go: build constraints disparity between compiler and documentation cmd/go: build constraints disparity between compiler and documentation Oct 13, 2023
@bcmills
Copy link
Contributor

bcmills commented Oct 13, 2023

The difference in behavior between // +build and //go:build is intentional; see https://go.dev/design/draft-gobuild#background-placement and https://go.dev/design/draft-gobuild#design-placement.

As far as I can tell we just need to update the cmd/go documentation to match the design and implementation.

@mark-pictor-csec, would you like to send a fix?

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go labels Oct 13, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 13, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/535635 mentions this issue: cmd/go/internal/help: update the documentation to match the design and implementation

@mark-pictor-csec
Copy link
Author

@quantonganh thanks for picking this up; I was OOO when @bcmills suggested I submit a fix.

@golang golang locked and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants