Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1344)

Issue 57100043: code review 57100043: cmd/go,cmd/cgo,make.bash: Cross compiling with cgo enabled (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by elias.naur
Modified:
10 years, 1 month ago
Reviewers:
minux1, gobot, iant
CC:
golang-codereviews, minux1, iant, rsc, Dominik Honnef
Visibility:
Public.

Description

cmd/go, cmd/cgo, make.bash: cross compiling with cgo enabled Introduce two new environment variables, CC_FOR_TARGET and CXX_FOR_TARGET. CC_FOR_TARGET defaults to CC and is used when compiling for GOARCH, while CC remains for compiling for GOHOSTARCH. CXX_FOR_TARGET defaults to CXX and is used when compiling C++ code for GOARCH. CGO_ENABLED defaults to disabled when cross compiling and has to be explicitly enabled. Update issue 4714

Patch Set 1 #

Patch Set 2 : diff -r 94165b19719e https://code.google.com/p/go #

Patch Set 3 : diff -r 94165b19719e https://code.google.com/p/go #

Total comments: 2

Patch Set 4 : diff -r 94165b19719e https://code.google.com/p/go #

Patch Set 5 : diff -r 94165b19719e https://code.google.com/p/go #

Patch Set 6 : diff -r 8946341a227b https://code.google.com/p/go #

Patch Set 7 : diff -r 8946341a227b https://code.google.com/p/go #

Total comments: 3

Patch Set 8 : diff -r 8946341a227b https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+80 lines, -55 lines) Patch
M src/cmd/dist/a.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M src/cmd/dist/build.c View 1 2 3 4 5 6 7 3 chunks +18 lines, -7 lines 0 comments Download
M src/cmd/dist/buildgo.c View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/go/build.go View 1 2 3 4 5 3 chunks +46 lines, -39 lines 0 comments Download
M src/make.bash View 1 2 3 4 5 6 7 2 chunks +12 lines, -5 lines 0 comments Download
M src/pkg/go/build/build.go View 1 2 3 4 5 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 20
elias.naur
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
10 years, 2 months ago (2014-01-26 19:43:04 UTC) #1
minux1
https://codereview.appspot.com/57100043/diff/40001/src/cmd/dist/buildgo.c File src/cmd/dist/buildgo.c (right): https://codereview.appspot.com/57100043/diff/40001/src/cmd/dist/buildgo.c#newcode15 src/cmd/dist/buildgo.c:15: // const defaultCXX = <defaultcxx> please update the comments ...
10 years, 2 months ago (2014-01-26 19:51:55 UTC) #2
minux1
Please remove the period in "Update issue 4714." in the description, or it won't take ...
10 years, 2 months ago (2014-01-26 19:52:36 UTC) #3
elias.naur
Thanks! PTAL On 2014/01/26 19:51:55, minux wrote: > https://codereview.appspot.com/57100043/diff/40001/src/cmd/dist/buildgo.c > File src/cmd/dist/buildgo.c (right): > > ...
10 years, 2 months ago (2014-01-26 21:02:03 UTC) #4
minux1
LGTM. Please wait for at least rsc or iant. One more nit in the description: ...
10 years, 2 months ago (2014-01-26 21:19:04 UTC) #5
elias.naur
On 2014/01/26 21:19:04, minux wrote: > LGTM. Please wait for at least rsc or iant. ...
10 years, 2 months ago (2014-01-26 21:31:44 UTC) #6
Dominik Honnef
Very minor nitpick: the CL description says that "XCC and XCXX" replace "XCC and XCXX" ...
10 years, 2 months ago (2014-01-27 11:53:07 UTC) #7
Dominik Honnef
Very minor nitpick: the CL description says that "XCC and XCXX" replace "XCC and XCXX" ...
10 years, 2 months ago (2014-01-27 11:53:07 UTC) #8
elias.naur
On 2014/01/27 11:53:07, Dominik Honnef wrote: > Very minor nitpick: the CL description says that ...
10 years, 2 months ago (2014-01-27 14:05:29 UTC) #9
elias.naur
PTAL (Trying to flip the "waiting for author" status on the go-dev dashboard) On 2014/01/27 ...
10 years, 1 month ago (2014-02-03 22:29:39 UTC) #10
iant
I'm not sure I understand why we should use XCC and XCCX rather than simply ...
10 years, 1 month ago (2014-02-03 22:43:55 UTC) #11
elias.naur
On 2014/02/03 22:43:55, iant wrote: > I'm not sure I understand why we should use ...
10 years, 1 month ago (2014-02-03 23:08:33 UTC) #12
iant
I'll take the discussion to golang-dev, probably better than on this CL.
10 years, 1 month ago (2014-02-03 23:17:49 UTC) #13
elias.naur
PTAL. I've reworked the CL to implement your proposal on golang-dev. On 2014/02/03 23:17:49, iant ...
10 years, 1 month ago (2014-02-05 15:11:12 UTC) #14
iant
https://codereview.appspot.com/57100043/diff/110001/src/cmd/dist/build.c File src/cmd/dist/build.c (right): https://codereview.appspot.com/57100043/diff/110001/src/cmd/dist/build.c#newcode176 src/cmd/dist/build.c:176: xgetenv(&b, "CXX"); Since CXX is not used by dist, ...
10 years, 1 month ago (2014-02-06 01:29:32 UTC) #15
elias.naur
PTAL. On 2014/02/06 01:29:32, iant wrote: > https://codereview.appspot.com/57100043/diff/110001/src/cmd/dist/build.c > File src/cmd/dist/build.c (right): > > https://codereview.appspot.com/57100043/diff/110001/src/cmd/dist/build.c#newcode176 ...
10 years, 1 month ago (2014-02-06 07:06:35 UTC) #16
iant
LGTM Thanks.
10 years, 1 month ago (2014-02-06 17:05:47 UTC) #17
iant
*** Submitted as https://code.google.com/p/go/source/detail?r=6d3bdbd27761 *** cmd/go, cmd/cgo, make.bash: cross compiling with cgo enabled Introduce two ...
10 years, 1 month ago (2014-02-06 17:11:10 UTC) #18
gobot
This CL appears to have broken the freebsd-amd64 builder.
10 years, 1 month ago (2014-02-06 17:15:31 UTC) #19
iant
10 years, 1 month ago (2014-02-07 00:26:34 UTC) #20
On Thu, Feb 6, 2014 at 9:15 AM,  <gobot@golang.org> wrote:
> This CL appears to have broken the freebsd-amd64 builder.

http://golang.org/issue/7159
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b