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: continue conversion to bug-resistant //go:build constraints #41184

Closed
rsc opened this issue Sep 2, 2020 · 44 comments
Closed

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

rsc opened this issue Sep 2, 2020 · 44 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted release-blocker
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Sep 2, 2020

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 rsc added this to the Proposal milestone Sep 2, 2020
@rsc rsc added this to Active in Proposals (old) Sep 2, 2020
@rsc rsc changed the title proposal: start conversion to bug-resistant //go:build constraints proposal: cmd/go: start conversion to bug-resistant //go:build constraints Sep 2, 2020
@rsc
Copy link
Contributor Author

rsc commented Sep 16, 2020

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
Copy link

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Sep 16, 2020
@rsc
Copy link
Contributor Author

rsc commented Sep 23, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Sep 23, 2020
@rsc
Copy link
Contributor Author

rsc commented Sep 23, 2020

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.

@rsc rsc modified the milestones: Proposal, Go1.16 Sep 23, 2020
@rsc rsc changed the title proposal: cmd/go: start conversion to bug-resistant //go:build constraints cmd/go: start conversion to bug-resistant //go:build constraints Sep 23, 2020
@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Oct 12, 2020
Show constraint-ignored non-.go files in go list, as Package.IgnoredOtherFiles
(same as go/build's IgnoredOtherFiles).

Pass full list of ignored files to vet, to help buildtag checker.

For #41184.

Change-Id: I749868de9082cbbc1efbc59370783c8c82fe735f
Reviewed-on: https://go-review.googlesource.com/c/go/+/240553
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue Oct 13, 2020
We are converting from using error-prone ad-hoc syntax // +build lines
to less error-prone, standard boolean syntax //go:build lines.
The timeline is:

Go 1.16: prepare for transition
 - Builds still use // +build for file selection.
 - Source files may not contain //go:build without // +build.
 - Builds fail when a source file contains //go:build lines without // +build lines. <<<

Go 1.17: start transition
 - Builds prefer //go:build for file selection, falling back to // +build
   for files containing only // +build.
 - Source files may contain //go:build without // +build (but they won't build with Go 1.16).
 - Gofmt moves //go:build and // +build lines to proper file locations.
 - Gofmt introduces //go:build lines into files with only // +build lines.
 - Go vet rejects files with mismatched //go:build and // +build lines.

Go 1.18: complete transition
 - Go fix removes // +build lines, leaving behind equivalent // +build lines.

This CL provides part of the <<< marked line above in the Go 1.16 step:
rejecting files containing //go:build but not // +build.
The standard go command checks only consider the top of the file.
This compiler check, along with a separate go vet check for ignored files,
handles the remainder of the file.

For #41184.

Change-Id: I014006eebfc84ab5943de18bc90449e534f150a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/240601
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue Oct 13, 2020
We are converting from using error-prone ad-hoc syntax // +build lines
to less error-prone, standard boolean syntax //go:build lines.
The timeline is:

Go 1.16: prepare for transition
 - Builds still use // +build for file selection.
 - Source files may not contain //go:build without // +build.
 - Builds fail when a source file contains //go:build lines without // +build lines. <<<

Go 1.17: start transition
 - Builds prefer //go:build for file selection, falling back to // +build
   for files containing only // +build.
 - Source files may contain //go:build without // +build (but they won't build with Go 1.16).
 - Gofmt moves //go:build and // +build lines to proper file locations.
 - Gofmt introduces //go:build lines into files with only // +build lines.
 - Go vet rejects files with mismatched //go:build and // +build lines.

Go 1.18: complete transition
 - Go fix removes // +build lines, leaving behind equivalent // +build lines.

This CL provides part of the <<< marked line above in the Go 1.16 step:
rejecting files containing //go:build but not // +build.

Reject any //go:build comments found after actual assembler code
(include #include etc directives), because the go command itself
doesn't read that far.

For #41184.

Change-Id: Ib460bfd380cce4239993980dd208afd07deff3f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/240602
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue Oct 13, 2020
We are converting from using error-prone ad-hoc syntax // +build lines
to less error-prone, standard boolean syntax //go:build lines.
The timeline is:

Go 1.16: prepare for transition
 - Builds still use // +build for file selection.
 - Source files may not contain //go:build without // +build.
 - Builds fail when a source file contains //go:build lines without // +build lines. <<<

Go 1.17: start transition
 - Builds prefer //go:build for file selection, falling back to // +build
   for files containing only // +build.
 - Source files may contain //go:build without // +build (but they won't build with Go 1.16).
 - Gofmt moves //go:build and // +build lines to proper file locations.
 - Gofmt introduces //go:build lines into files with only // +build lines.
 - Go vet rejects files with mismatched //go:build and // +build lines.

Go 1.18: complete transition
 - Go fix removes // +build lines, leaving behind equivalent // +build lines.

This CL provides part of the <<< marked line above in the Go 1.16 step:
rejecting files containing //go:build but not // +build.

For #41184.

Change-Id: I29b8a789ab1526ab5057f613d5533bd2060ba9cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/240600
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Oct 28, 2021
Now that Go 1.17 is out and Go 1.15 is unsupported,
removing // +build lines can be done safely: in the worst case,
if code is compiled using Go 1.16 the toolchain will detect
the presence of a //go:build without // +build and fail the build.
(It will not silently choose the wrong files.)

Note that +build lines will continue to work in Go sources forever.
This just provides a mechanism for users who are done with
Go 1.16 to remove them easily, by running "go fix".

Also update for new generics AST.

For #41184.
Fixes #48978.

Change-Id: I11a432c319e5abd05ad68dda9ccd7a7fdcc8bbb8
Reviewed-on: https://go-review.googlesource.com/c/go/+/240611
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Oct 28, 2021
cmd/go has its own //go:build evaluator, which is needed for
patterns like 'all'. The code is a modified copy of some unexported
routines from the go/build package. Update it by copying those
again and re-modifying them. The modifications are primarily the new
func eval and also ignoring errors.

This CL will need to be backported to Go 1.17, or else Go 1.17
will break when faced with certain //go:build-only repos during
'go list all' or 'go mod tidy'.

For #41184.
Fixes #49198.

Change-Id: Ie0fe3caa8d49004935ecd76d7977f767fe50e317
Reviewed-on: https://go-review.googlesource.com/c/go/+/359355
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Oct 28, 2021
When these packages are released as part of Go 1.18,
Go 1.16 will no longer be supported, so we can remove
the +build tags in these files.

Ran go fix -fix=buildtag std cmd and then reverted the bootstrapDirs
as defined in src/cmd/dist/buildtool.go, which need to continue
to build with Go 1.4 for now.

Also reverted src/vendor and src/cmd/vendor, which will need
to be updated in their own repos first.

Manual changes in runtime/pprof/mprof_test.go to adjust line numbers.

For #41184.

Change-Id: Ic0f93f7091295b6abc76ed5cd6e6746e1280861e
Reviewed-on: https://go-review.googlesource.com/c/go/+/344955
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Oct 28, 2021
The upstream cmd/bundle tool does not yet omit it,
and the longtest builders test that the generated
file is not modified.

For #41184.

Change-Id: Ib68f532139ed1436cbaf3a756f300fe60f520cab
Reviewed-on: https://go-review.googlesource.com/c/go/+/359484
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Oct 28, 2021
cmd/go has its own //go:build evaluator, which is needed for
patterns like 'all'. The code is a modified copy of some unexported
routines from the go/build package. Update it by copying those
again and re-modifying them. The modifications are primarily the new
func eval and also ignoring errors.

This CL will need to be backported to Go 1.17, or else Go 1.17
will break when faced with certain //go:build-only repos during
'go list all' or 'go mod tidy'.

For #41184.
Fixes #49198.

Change-Id: Ie0fe3caa8d49004935ecd76d7977f767fe50e317
Reviewed-on: https://go-review.googlesource.com/c/go/+/359355
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/359404
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Nov 6, 2021
CL 344955 and CL 359476 removed almost all // +build lines, but leaving
some assembly files and generating scripts. Also, some files were added
with // +build lines after CL 359476 was merged. Remove these or rename
files where more appropriate.

For #41184

Change-Id: I7eb85a498ed9788b42a636e775f261d755504ffa
Reviewed-on: https://go-review.googlesource.com/c/go/+/361480
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@gopherbot
Copy link

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

gopherbot pushed a commit to golang/sys that referenced this issue Nov 10, 2021
These were apparently overlooked in CL 357329, CL 294490, CL 296889,
and other various updates to this module. (I noticed them via gopls
while investigating golang/go#49466.)

Updates golang/go#41184

Change-Id: Id042bb6fe5282e6d528e9315acf2ad29d0df58ba
Reviewed-on: https://go-review.googlesource.com/c/sys/+/362577
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Nov 25, 2021
Forgot to 'git add' this test written as part of CL 359314.

For #41184.

Change-Id: I2ebd48fd62a2053c8b16e5a8c48c1e11d1b86d5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/366894
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
chriscasola pushed a commit to chriscasola/go that referenced this issue Jan 10, 2022
Make all our package sources use Go 1.17 gofmt format
(adding //go:build lines).

Not strictly necessary but will avoid spurious changes
as files are edited.

Part of //go:build change (golang#41184).
See https://golang.org/design/draft-gobuild

Change-Id: Id0e05ad476461716b1f58858e2dc94d13b45041c
Reviewed-on: https://go-review.googlesource.com/c/perf/+/294421
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
josharian pushed a commit to tailscale/go that referenced this issue Feb 15, 2022
…ld lines

Look for //go:build ignore, not // +build ignore, in deps_test.go.

For golang#41184.

Change-Id: Iba8617230aa620223e2bc170f18d0c54557318c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/359315
Trust: Russ Cox <rsc@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

(cherry picked from commit golang.org/cl/359315)
josharian pushed a commit to tailscale/go that referenced this issue Feb 15, 2022
…ld lines

Look for //go:build ignore, not // +build ignore, in deps_test.go.

For golang#41184.

Change-Id: Iba8617230aa620223e2bc170f18d0c54557318c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/359315
Trust: Russ Cox <rsc@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

(cherry picked from commit golang.org/cl/359315)
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Feb 18, 2022
For #44505 and #41184.

Change-Id: I9503292dace1aa60de167ca5807bf131554465b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/386774
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
For golang/go#41184

Change-Id: Ica67fdbf2745ad2eef63dbb9ef70136e9e6fd348
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/319469
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted release-blocker
Projects
No open projects
Development

No branches or pull requests

8 participants