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: document build constraints in 'go help' #37018

Closed
pjweinb opened this issue Feb 4, 2020 · 12 comments
Closed

cmd/go: document build constraints in 'go help' #37018

pjweinb opened this issue Feb 4, 2020 · 12 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@pjweinb
Copy link

pjweinb commented Feb 4, 2020

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

go version devel +a50c3ffbd4 linux/amd64

Does this issue reproduce with the latest release?

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

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/pjw/.cache/go-build"
GOENV="/usr/local/google/home/pjw/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"

What did you do?

In x/tools/internal/lsp/span/uri_windows_test.go I removed // +build window, and renamed the test function to TestURIA (to avoid conflicts with uri_test.go)

I then ran go test -run URI .

What did you expect to see?

I expected to see it run TestURI (from uri_test.go) and TestURIA (from uri_windows_test.go)

What did you see instead?

It just ran TestURI

[If I rename uri_window_test.go then go test will run TestURIA]

@stamblerre stamblerre changed the title Tests not run after removing // +build windows x/tools/gopls: invalidate metadata after a change to build tags Feb 4, 2020
@gopherbot gopherbot added this to the Unreleased milestone Feb 4, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Feb 4, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.4.0 Feb 4, 2020
@ALTree
Copy link
Member

ALTree commented Feb 4, 2020

The build tag in the filename is preventing the test from being run. Having _GOOS_ in the filename adds a +GOOS build constraint to the file.

@pjweinbgo
Copy link
Contributor

It would be nice if that were documented somewhere in ordinary help, rather than hoping users will paw through the source code.

@stamblerre stamblerre changed the title x/tools/gopls: invalidate metadata after a change to build tags Tests not run after removing // +build windows Feb 4, 2020
@stamblerre stamblerre removed Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Feb 4, 2020
@ALTree
Copy link
Member

ALTree commented Feb 4, 2020

This is documented. From https://golang.org/pkg/go/build

If a file's name, after stripping the extension and a possible _test suffix, 
matches any of the following patterns:

*_GOOS
*_GOARCH
*_GOOS_GOARCH

(example: source_windows_amd64.go) where GOOS and GOARCH 
represent any known operating system and architecture values 
respectively, then the file is considered to have an implicit build 
constraint requiring those terms (in addition to any explicit 
constraints in the file). 

@jayconrod
Copy link
Contributor

It would be nice if that were documented somewhere in ordinary help, rather than hoping users will paw through the source code.

I agree with this. Build constraints are mentioned a few times in https://golang.org/cmd/go/, but they aren't defined. There's only one reference to the go/build package documentation under go help filetype. It's pretty hard to find.

I think we should add a go help build-constraints page that describes tags and suffixes more thoroughly, and it should be referenced from any section that mentions build constraints.

@jayconrod jayconrod changed the title Tests not run after removing // +build windows cmd/go: document build constraints in 'go help' Feb 4, 2020
@jayconrod jayconrod added Documentation NeedsFix The path to resolution is known, but the work has not been done. labels Feb 4, 2020
@jayconrod jayconrod modified the milestones: gopls/v0.4.0, Go1.15 Feb 4, 2020
@ALTree
Copy link
Member

ALTree commented Feb 4, 2020

@jayconrod

There's only one reference to the go/build package documentation under go help filetype

This is not true; the feature is pretty clearly documented in the https://golang.org/pkg/go/build section I pasted above (I mean the filename thing that is the topic of this issue).

@jayconrod
Copy link
Contributor

Very true, but I don't think it's enough. If you're reading go help pages, it's not currently clear that you need to read the docs for go/build. go/build is only referenced twice in the context of build constraints: once in the description for the -tags flag, and again at the end of go help filetype. It's not obvious in either case that GOOS / GOARCH suffixes are meaningful.

@mlowicki
Copy link
Contributor

@jayconrod I can work on this one. I'll add go help build-constraints. It seems that content there will be pretty much the same as in https://golang.org/pkg/go/build/#hdr-Build_Constraints. Should I replace https://golang.org/pkg/go/build/#hdr-Build_Constraints with reference to new page (See 'go help build-constraints') to avoid duplication?

@jayconrod
Copy link
Contributor

@mlowicki Thanks for looking at this.

I don't think the documentation in go/build should be replaced. Documentation in go help should probably just be a quick reference with pointers to more in-depth HTML documentation. The go/build documentation is the closest we have right now to the latter, but we may want to expand it somewhere else in the future.

@gopherbot
Copy link

Change https://golang.org/cl/228017 mentions this issue: cmd/go: add 'go help buildconstraint'

@jayconrod
Copy link
Contributor

Reopening this. @rsc pointed out there's no need to document build constraints in two places. go help buildconstraint is almost as long as the section in go/build. The go help page is more discoverable, so we should make that version complete and canonical and just point there from go/build.

I'll send a CL that does this shortly.

@jayconrod jayconrod reopened this May 8, 2020
@jayconrod jayconrod self-assigned this May 8, 2020
@gopherbot
Copy link

Change https://golang.org/cl/232981 mentions this issue: go/build: move build constraint docs to 'go help buildconstraint'

xujianhai666 pushed a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
Fixes golang#37018

Change-Id: I1d32c1cb432bc2d7a4d8d6b5c3a54fee558141ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/228017
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/248038 mentions this issue: internal/goversion: update Version to 1.16

gopherbot pushed a commit that referenced this issue Aug 12, 2020
This is the start of the Go 1.16 development cycle, so update the
Version value accordingly. It represents the Go 1.x version that
will soon open up for development (and eventually become released).

Historically, we used to bump this at an arbitrary time throughout
the development cycle, but it's better to be more predictable about
updating it. The start of a development cycle should be the most
appropriate time: it clearly marks the boundary between 1.15 and
1.16 development, and doing it early can help catch issues in other
tooling. See issue #38704 for more background.

There is no longer a need to update the list of Go versions in
src/go/build/doc.go because it does not exist as of CL 232981.

For #40705.
Updates #38704.
Updates #37018.

Change-Id: Id8ee733b5e79c53b6cd03509c6560614d8743833
Reviewed-on: https://go-review.googlesource.com/c/go/+/248038
Reviewed-by: Carlos Amedee <carlos@golang.org>
@golang golang locked and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants