golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.64k stars 17.61k forks source link

cmd/go: continue conversion to bug-resistant //go:build constraints #41184

Closed rsc closed 2 years ago

rsc commented 4 years ago

In June I posted a draft design for moving from // +build lines with ad-hoc syntax to //go:build lines with standard Boolean expressions; that doc also has links to a video overview and a prototype implementation.

I propose that we adopt this design, with N = 17 in the Transition section, meaning that the prep work would happen in Go 1.16, the main change would land in Go 1.17, and the transition would be finalized in Go 1.18.

rsc commented 4 years ago

Based on the emoji above and the lack of any objections (along with the overwhelmingly positive Reddit thread), this seems like a likely accept.

cristaloleg commented 4 years ago

Post on Reddit: https://www.reddit.com/r/golang/comments/hitexe/qa_gobuild_draft_design/

rsc commented 4 years ago

No change in consensus, so accepted.

rsc commented 4 years ago

Please note that accepting this means that //go:build lines will start working in Go 1.17, not Go 1.16. Don't everyone rush to put them in.

gopherbot commented 4 years ago

Change https://golang.org/cl/240600 mentions this issue: go/build: reject //go:build without // +build

gopherbot commented 4 years ago

Change https://golang.org/cl/240553 mentions this issue: cmd/go: add IgnoredOtherFiles to go list; pass IgnoredFiles to vet

gopherbot commented 4 years ago

Change https://golang.org/cl/240601 mentions this issue: cmd/compile: reject misplaced go:build comments

gopherbot commented 4 years ago

Change https://golang.org/cl/240602 mentions this issue: cmd/asm: reject misplaced go:build comments

gopherbot commented 4 years ago

Change https://golang.org/cl/240554 mentions this issue: cmd/vet: look at ignored files in buildtag check

gopherbot commented 4 years ago

Change https://golang.org/cl/261958 mentions this issue: cmd: go get golang.org/x/tools@d88ec18 && go mod vendor

rsc commented 4 years ago

All the Go 1.16 prep work for //go:build lines is now landed. I've retitled and remilestoned the issue to track the Go 1.17 work.

gopherbot commented 4 years ago

Change https://golang.org/cl/240604 mentions this issue: go/build/constraint: add parser for build tag constraint expressions

gopherbot commented 4 years ago

Change https://golang.org/cl/240609 mentions this issue: cmd/vet: update buildtag check for //go:build lines

gopherbot commented 4 years ago

Change https://golang.org/cl/240608 mentions this issue: go/printer: canonicalize //go:build and // +build lines while formatting

gopherbot commented 4 years ago

Change https://golang.org/cl/240607 mentions this issue: go/build: prefer //go:build over // +build lines

gopherbot commented 3 years ago

Change https://golang.org/cl/287412 mentions this issue: api/go1.16: add build/constraint APIs

gopherbot commented 3 years ago

Change https://golang.org/cl/287472 mentions this issue: doc/go1.16: document go/build/constraint package

gopherbot commented 3 years ago

Change https://golang.org/cl/240611 mentions this issue: cmd/fix: add buildtag fix

gopherbot commented 3 years ago

Change https://golang.org/cl/294430 mentions this issue: all: go fmt std cmd (but revert vendor)

zephyr commented 3 years ago

How will this effect the Go1.4-bootstrap once the transition will be finalized in Go 1.18?

I just took master@d4b2638234 and crudely removed all //+build lines:

$ find . -name '*.go' -exec sed -i '/\/\/ +build /d' {} \;

After that, i could no longer bootstrap master with go1.4:

dennis@inara:/opt/golang$ update-golang.py dennis
Building Go cmd/dist using /opt/golang1.4. (go1.4-bootstrap-20170531 linux/amd64)
# _/opt/golang/src/cmd/dist
cmd/dist/util_gccgo.go:9: useVFPv1 redeclared in this block
    previous declaration at cmd/dist/util_gc.go:11
cmd/dist/util_gccgo.go:11: useVFPv3 redeclared in this block
    previous declaration at cmd/dist/util_gc.go:15
cmd/dist/util_gccgo.go:13: useARMv6K redeclared in this block
    previous declaration at cmd/dist/util_gc.go:20
Error: /opt/golang/src/make.bash returned

So should go/build/constraint be backported to Go1.4?

rsc commented 3 years ago

The compiler has to be kept building with Go 1.4 for now. That means it has to keep its // +build comments. I do plan to propose bumping the bootstrap version to Go 1.17, though, for a variety of reasons, and after we've done that, we'd be able to remove the // +build comments.

rsc commented 3 years ago

All the Go 1.17 prep work for //go:build lines is now landed. I've remilestoned the issue to track the Go 1.18 work (submitting golang.org/cl/240611).

gopherbot commented 3 years ago

Change https://golang.org/cl/296889 mentions this issue: unix: add //go:build lines when generating files

gopherbot commented 3 years ago

Change https://golang.org/cl/319389 mentions this issue: all: add //go:build lines to assembly files

gopherbot commented 3 years ago

Change https://golang.org/cl/319410 mentions this issue: cmd/vendor: update golang.org/x/sys to latest

gopherbot commented 3 years ago

Change https://golang.org/cl/319469 mentions this issue: all: add //go:build lines to assembly files

gopherbot commented 3 years ago

Change https://golang.org/cl/320312 mentions this issue: all: update golang.org/x/sys to latest

gopherbot commented 3 years ago

Change https://golang.org/cl/323750 mentions this issue: runtime/internal/sys: generate //go:build lines in gengoos.go

gopherbot commented 3 years ago

Change https://golang.org/cl/341009 mentions this issue: all: gofmt more (but vendor, testdata, and top-level test directories)

cagedmantis commented 3 years ago

This issue is currently labeled as early-in-cycle for Go 1.18. That time is now, so this is a friendly ping so the issue is looked at again.

toothrot commented 3 years ago

Reminder that this is an early-in-cycle release-blocker for Go 1.18 and should be addressed soon.

gopherbot commented 3 years ago

Change https://golang.org/cl/344955 mentions this issue: all: go fix -fix=buildtag std (except for bootstrap deps)

heschi commented 3 years ago

ping @rsc -- to me this seems like something we would like to get in well before the freeze?

dmitshur commented 3 years ago

For quick reference, the outstanding CLs for this issue that I'm aware of and their status as of this posting:

gopherbot commented 2 years ago

Change https://golang.org/cl/359314 mentions this issue: cmd/dist: implement //go:build parsing

gopherbot commented 2 years ago

Change https://golang.org/cl/359315 mentions this issue: go/build: update for //go:build lines

gopherbot commented 2 years ago

Change https://golang.org/cl/359355 mentions this issue: cmd/go: update for //go:build lines

gopherbot commented 2 years ago

Change https://golang.org/cl/359476 mentions this issue: all: manual fixups for //go:build vs // +build

gopherbot commented 2 years ago

Change https://golang.org/cl/359404 mentions this issue: cmd/go: update for //go:build lines

gopherbot commented 2 years ago

Change https://golang.org/cl/359484 mentions this issue: net/http: restore generated // +build comment

gopherbot commented 2 years ago

Change https://golang.org/cl/361480 mentions this issue: all: remove more leftover // +build lines

gopherbot commented 2 years ago

Change https://golang.org/cl/362577 mentions this issue: all: add missing //go:build comments

gopherbot commented 2 years ago

Change https://golang.org/cl/366894 mentions this issue: cmd/dist: add buildtag parsing test

gopherbot commented 2 years ago

Change https://go.dev/cl/386774 mentions this issue: doc/go1.18: document Go 1.17 bootstrap and //go:build fix