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: difficult to diagnose when build constraints are not followed by a blank line #36510

Closed
ilyapashuk opened this issue Jan 11, 2020 · 5 comments
Closed

cmd/go: difficult to diagnose when build constraints are not followed by a blank line #36510

ilyapashuk opened this issue Jan 11, 2020 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Backlog

Comments

@ilyapashuk
Copy link

go version go1.13.5 windows/amd64

I have the package wich contains two files, one for windows and other for unix.
it is github.com/brimstone/go-shellcode

when I try to build them on windows, i get this error:
exec: "gcc": executable file not found in %PATH%

the windows implementation contains only pure go code, but unix version have some c code in it, and go tries to process this code with help of gcc.
but this file contains _unix in it's name, and there are build tag "// +build linux freebsd darwin" inside it.
so, from go build rules, this file should not be processed by any way.
so, I think, the processing stages are ordered incorrectly, because, as I think, build tag processor should run firstly and, if build tags or simply file names signals to don't process this file, simply close it instantly and don't use it in next processing, and only then run gcc and other stages for remaining files.

and when I delete the unix implementation file, windows version builds perfectly.
@smasher164
Copy link
Member

Can you list the specific commands you used to build this package? On my mojave machine, running

$ GOOS=windows go build

causes the package to build properly.

@smasher164 smasher164 added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 11, 2020
@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2020

Per https://golang.org/pkg/go/build/#hdr-Build_Constraints:

To distinguish build constraints from package documentation, a series of build constraints must be followed by a blank line.

The comment in https://github.com/brimstone/go-shellcode/blob/ed69073cc3bfaacbe8c0e37e1b5e1dd8be2eda16/shellcode_unix.go#L1 is not followed by a blank line, so it is interpreted as a package comment rather than a build constraint.

@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2020 •
edited

I am curious, though: do any of go vet, golint, and/or staticcheck flag the erroneous build constraint comment? This seems like a pretty obvious candidate for a vet check.

CC @matloob @jayconrod @dominikh

@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jan 22, 2020
@bcmills bcmills changed the title incorrect go file processing stages ordering cmd/go: difficult to diagnose when build constraints are not followed by a blank line Jan 22, 2020
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 22, 2020
@bcmills bcmills added this to the Backlog milestone Jan 22, 2020
@jayconrod
Copy link
Contributor

The buildtag analyzer checks this. Pretty sure it's one of the analyzers that runs by default in go test.

@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2020

Indeed, go vet emits a warning for this case.

go-shellcode$ go vet .
# github.com/brimstone/go-shellcode
./shellcode_unix.go:1:1: +build comment must appear before package clause and be followed by a blank line

In that case, I don't think there is anything more to be done for this issue.

@bcmills bcmills closed this as completed Jan 22, 2020
@golang golang locked and limited conversation to collaborators Jan 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@jayconrod @bcmills @gopherbot @smasher164 @ilyapashuk