Issue 8134043: code review 8134043: go/build: disable cgo when cross compiling (Closed)
Descriptiongo/build: disable cgo when cross compiling
Fixes issue 5141.
Patch Set 1 #Patch Set 2 : diff -r 2433c3567273 https://code.google.com/p/go #Patch Set 3 : diff -r 2433c3567273 https://code.google.com/p/go #Patch Set 4 : diff -r c81bb3def1ef https://code.google.com/p/go #
Total comments: 2
Patch Set 5 : diff -r 282dcbf1f423 https://code.google.com/p/go #Patch Set 6 : diff -r cfe93dfc5788 https://code.google.com/p/go #Patch Set 7 : diff -r cfe93dfc5788 https://code.google.com/p/go #Patch Set 8 : diff -r e8d3578a3f34 https://code.google.com/p/go #MessagesTotal messages: 17
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
Sign in to reply to this message.
https://codereview.appspot.com/8134043/diff/10001/src/pkg/go/build/build.go File src/pkg/go/build/build.go (right): https://codereview.appspot.com/8134043/diff/10001/src/pkg/go/build/build.go#n... src/pkg/go/build/build.go:311: // amd64 to 386 but not the other way around. I suspect this section might need some tweaking. I'd like to hear from Minux in particular as he has a lot fo experience with this.
Sign in to reply to this message.
You can only cross-compile CGO packages for linux-386 on linux-amd64 when the extra 32-bit libraries are present. For example, on Archlinux you have to enable the multilib repository and install lib32-glibc (and any other required libraries). What should happen if the 32-bit libraries are not installed? Anthony
Sign in to reply to this message.
> For example, on Archlinux you have to enable > the multilib repository and install lib32-glibc > (and any other required libraries). > > What should happen if the 32-bit libraries > are not installed? The source installation (moved to the wiki recently) page called out the fact you need the i386 compatibility libs installed, but given you can always pass CGO_ENABLED=1 if you know what you are doing, perhaps these special cases should be removed. I'm open to suggestions.
Sign in to reply to this message.
https://codereview.appspot.com/8134043/diff/10001/src/pkg/go/build/build.go File src/pkg/go/build/build.go (right): https://codereview.appspot.com/8134043/diff/10001/src/pkg/go/build/build.go#n... src/pkg/go/build/build.go:306: if runtime.GOARCH == c.GOARCH && runtime.GOOS == c.GOOS { i suggest we just use runtime.GOOS == c.GOOS for this test to fix the trivial and more frequent case: "cannot use cgo when compiling for a different operating system" i think we shouldn't invest too much effort on this issue, as we'd better fix the real cause -- make cgo cross compilable (in fact, i think fixing that is actually simpler, just remove the check and let user set $CC appropriately)
Sign in to reply to this message.
> i suggest we just use runtime.GOOS == c.GOOS for this test
> to fix the trivial and more frequent case:
> "cannot use cgo when compiling for a different operating system"
I'd like to counter with the case I have seen most frequently on irc is people
cross compiling for arm from amd64.
> i think we shouldn't invest too much effort on this issue, as
> we'd better fix the real cause -- make cgo cross compilable
> (in fact, i think fixing that is actually simpler, just remove the
> check and let user set $CC appropriately)
I'm thinking that the least surprising option is to disable cgo (unless
explicitly enabled via env var) unless both runtime.{GOOS,GOARCH} and
c.{GOOS,GOARCH} match. That is, remove all the special cases.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, minux.ma@gmail.com, ality@pbrane.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
Sign in to reply to this message.
LGTM I wouldn't mind a message in verbose mode saying "# CGO_ENABLED not set. Defaulting to '0' for linux/arm." or whatever the convention is in verbose / -x mode. But that can come later too. Or not. On Mon, Apr 1, 2013 at 9:11 PM, <dave@cheney.net> wrote: > <ping /> > > > https://codereview.appspot.**com/8134043/<https://codereview.appspot.com/8134... > > -- > > ---You received this message because you are subscribed to the Google > Groups "golang-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou... > . > For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o... > . > > >
Sign in to reply to this message.
Let me see what I can come up with. This CL isn't time sensitive so there is no reason to do a followup to add some logging. On Tue, Apr 2, 2013 at 3:16 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote: > LGTM > > I wouldn't mind a message in verbose mode saying "# CGO_ENABLED not set. > Defaulting to '0' for linux/arm." or whatever the convention is in verbose > / -x mode. > > But that can come later too. Or not. > > > > On Mon, Apr 1, 2013 at 9:11 PM, <dave@cheney.net> wrote: > >> <ping /> >> >> >> https://codereview.appspot.**com/8134043/<https://codereview.appspot.com/8134... >> >> -- >> >> ---You received this message because you are subscribed to the Google >> Groups "golang-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou... >> . >> For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o... >> . >> >> >> >
Sign in to reply to this message.
Please take another look. I've added the logging as requested, but because go/build doesn't have access to the cmd/go -x flag state, the following is output whenever go/build.DefaultContext() is called % GOARCH=arm go build net # CGO_ENABLED not set. Defaulting to '0' for linux/arm.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, minux.ma@gmail.com, ality@pbrane.org, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
LGTM without the printf. I'd rather it not be noisy by default. If there are no cgo packages involved, for instance, it shouldn't spam. We can fix the pedagogical issues later. I'd submit it without for now. On Wed, Apr 3, 2013 at 1:08 AM, <dave@cheney.net> wrote: > Hello golang-dev@googlegroups.com, minux.ma@gmail.com, ality@pbrane.org, > bradfitz@golang.org (cc: golang-dev@googlegroups.com), > > Please take another look. > > > https://codereview.appspot.**com/8134043/<https://codereview.appspot.com/8134... >
Sign in to reply to this message.
SGTM. Will revert to the previous patchset and submit. r/brad - should I add a change to the release notes for this change in behavior of the tool ? On Wed, Apr 3, 2013 at 7:11 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > LGTM without the printf. > > I'd rather it not be noisy by default. If there are no cgo packages > involved, for instance, it shouldn't spam. > > We can fix the pedagogical issues later. I'd submit it without for now. > > > > On Wed, Apr 3, 2013 at 1:08 AM, <dave@cheney.net> wrote: >> >> Hello golang-dev@googlegroups.com, minux.ma@gmail.com, ality@pbrane.org, >> bradfitz@golang.org (cc: golang-dev@googlegroups.com), >> >> Please take another look. >> >> >> https://codereview.appspot.com/8134043/ > >
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=139b5fe32880 *** go/build: disable cgo when cross compiling Fixes issue 5141. R=golang-dev, minux.ma, ality, bradfitz CC=golang-dev https://codereview.appspot.com/8134043
Sign in to reply to this message.
Message was sent while issue was closed.
de2fcae372bd4b06b684380ea6b55100683e433a
Sign in to reply to this message.
|
