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

go/build: document build comment requirements #3539

Closed
jhawk28 opened this issue Apr 16, 2012 · 19 comments
Closed

go/build: document build comment requirements #3539

jhawk28 opened this issue Apr 16, 2012 · 19 comments
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@jhawk28
Copy link

jhawk28 commented Apr 16, 2012

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. I try to build https://github.com/alecthomas/gozmq. It works on OSX, but it does not
compile on Windows

What is the expected output?
A compiled gozmq

What do you see instead?
D:\repos\golibs\gozmq>go build
# _/D_/repos/golibs/gozmq
zmq_windows.go:24[C:\Users\****\AppData\Local\Temp\go-build075945734\_\D_\repos\golibs\gozmq\_obj\zmq_windows.cgo1.go:8]:
ZmqOsSocketType redeclared in this block
        previous declaration at zmq_bsd.go:27[C:\Users\****\AppData\Local\Temp\go-build075945734\_\D_\repos\golibs\gozmq\_obj\zmq_bsd.cgo1.go:8]
zmq_windows.go:26[C:\Users\****\AppData\Local\Temp\go-build075945734\_\D_\repos\golibs\gozmq\_obj\zmq_windows.cgo1.go:12]:
ZmqOsSocketType.ToRaw redeclared
in this block
        previous declaration at zmq_bsd.go:29[C:\Users\****\AppData\Local\Temp\go-build075945734\_\D_\repos\golibs\gozmq\_obj\zmq_bsd.cgo1.go:12]

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
Windows 7 x64

Which version are you using?  (run 'go version')
go1

Please provide any additional information below.
@bradfitz
Copy link
Contributor

Comment 1:

The +build lines need to come "early" in the file.  Try moving the +build lines above
the large copyright block and report back if that fixes it for you.

Status changed to WaitingForReply.

@jhawk28
Copy link
Author

jhawk28 commented Apr 16, 2012

Comment 2:

I moved the +build line to the first line and it still failed.

@rsc
Copy link
Contributor

rsc commented Apr 16, 2012

Comment 3:

Try
// +build freebsd netbsd openbsd
at the top of the file.

@jhawk28
Copy link
Author

jhawk28 commented Apr 16, 2012

Comment 4:

I added the "// +build freebsd netbsd openbsd" to the top of the file and it still
fails. 
I then added "// +build freebsd netbsd openbsd !windows" and it also failed.
Ironically, its just the zmq_bsd.go that is failing. If I leave the zmq_linux.go there
and remove the zmq_bsd.go, it will compile.

@alexbrainman
Copy link
Member

Comment 5:

Your zmq_bsd.go file starts with:
>>>
/*
  Copyright 2012 Alec Thomas
  Licensed under the Apache License, Version 2.0 (the "License");
  you may not use this file except in compliance with the License.
  You may obtain a copy of the License at
    http://www.apache.org/licenses/LICENSE-2.0
  Unless required by applicable law or agreed to in writing, software
  distributed under the License is distributed on an "AS IS" BASIS,
  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  See the License for the specific language governing permissions and
  limitations under the License.
*/
// For some reason, Go on Windows tries to build both this file and zmq_windows.go.
// +build !windows
<<<
This contradicts http://golang.org/pkg/go/build/#Build_Constraints:
... Constraints may appear in any kind of source file (not just Go), but they must be
appear near the top of the file, preceded only by blank lines and other line comments. 
...
You have "general comment", not "line comment", in front of your build constrain. Follow
Brad's advice, move "// +build !windows" line to the beginning of the file.
Alex

Status changed to WorkingAsIntended.

@jhawk28
Copy link
Author

jhawk28 commented Apr 17, 2012

Comment 6:

Ah, I see. It was missing an extra new line after the +build line.
This works:
// +build !windows
/*
This doesn't:
// +build !windows
/*

@alexbrainman
Copy link
Member

Comment 7:

You are correct. It seems go needs blank line after "// +build !windows". I can't find
any doco for that. Perhaps, this needs to be clarified. Leaving for others to decide.
Thank you.
Alex

@rsc
Copy link
Contributor

rsc commented Apr 17, 2012

Comment 8:

Thank you very much for diagnosing that problem.
// +build foo
/*
other comment
*/
is supposed to work.

Labels changed: added priority-soon, removed priority-triage.

Status changed to Accepted.

@ality
Copy link
Member

ality commented Apr 28, 2012

Comment 9:

I don't think that's right. Godoc will likely get the synopsis wrong
for the following if we allow build directives without a subsequent
blank line.
// +build foo
/*
Package lorem ipsum dolor set
*/
package lorem

@rsc
Copy link
Contributor

rsc commented Apr 30, 2012

Comment 10:

Godoc is a different program.  Not all /* */ comments are package comments.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 11:

I believe this is correct behavior now, but we need to document the behavior.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 12:

Issue #3730 has been merged into this issue.

@gopherbot
Copy link

Comment 13 by seed@mail.nanosouffle.net:

rsc wrote:
> I believe this is correct behavior now, but we need to document the behavior.
Which behaviour? I don't think the format:
"// +build foo
/*
other comment
*/
package bar"
works yet. I feel that it should.
I feel that in general, a `+build'
directive as the very first line
in the file should be taken into
account.
What do you feel about this?

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 14:

Labels changed: added go1.1.

@rsc
Copy link
Contributor

rsc commented Sep 13, 2012

Comment 15:

I don't expect that to work. The rule is that it must be followed by a
blank line. The // +build parser intentionally does not know what a /*
*/ comment is.

@rsc
Copy link
Contributor

rsc commented Dec 10, 2012

Comment 16:

Labels changed: added size-m.

@rsc
Copy link
Contributor

rsc commented Dec 10, 2012

Comment 17:

Labels changed: added suggested.

@adg
Copy link
Contributor

adg commented Jan 24, 2013

Comment 18:

This issue was closed by revision 2e51f78.

Status changed to Fixed.

@minux
Copy link
Member

minux commented Mar 5, 2013

Comment 19:

Issue #4985 has been merged into this issue.

@jhawk28 jhawk28 added fixed Suggested Issues that may be good for new contributors looking for work to do. labels Mar 5, 2013
@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

8 participants