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

Issue 12350044: code review 12350044: build: on OS X use clang instead of gcc (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by rsc
Modified:
4 years, 3 months ago
Reviewers:
fss, bradfitz
CC:
golang-dev, bradfitz, khr1
Visibility:
Public.

Description

build: on OS X 10.8 and later, use clang instead of gcc Fixes issue 5822. Will no doubt cause other problems, but Apple has forced our hand.

Patch Set 1 #

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

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

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

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

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

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -31 lines) Patch
M src/cmd/cgo/gcc.go View 1 2 3 4 5 6 3 chunks +14 lines, -14 lines 0 comments Download
M src/cmd/cgo/out.go View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/dist/a.h View 1 2 3 4 5 6 2 chunks +5 lines, -0 lines 0 comments Download
M src/cmd/dist/build.c View 1 2 3 4 5 6 6 chunks +23 lines, -6 lines 0 comments Download
M src/cmd/dist/unix.c View 1 2 3 4 5 6 2 chunks +14 lines, -7 lines 0 comments Download
M src/cmd/go/build.go View 1 2 3 4 5 6 2 chunks +3 lines, -2 lines 0 comments Download
M src/run.bash View 1 2 3 4 5 1 chunk +4 lines, -1 line 0 comments Download

Messages

Total messages: 13
rsc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
4 years, 3 months ago (2013-08-02 16:08:33 UTC) #1
bradfitz
LGTM On all versions? Even 10.6? Did clang work then, having all the flags we ...
4 years, 3 months ago (2013-08-02 16:12:28 UTC) #2
rsc
I have no idea what the version story is. We may need to be more ...
4 years, 3 months ago (2013-08-02 17:23:49 UTC) #3
bradfitz
Go for it. I have a 10.6 machine at home still which I can test ...
4 years, 3 months ago (2013-08-02 17:33:40 UTC) #4
rsc
Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
4 years, 3 months ago (2013-08-02 17:46:41 UTC) #5
rsc
PTAL Added logic to cmd/cgo and disabled the cgo -godefs test.
4 years, 3 months ago (2013-08-02 17:46:52 UTC) #6
khr1
I get errors when running this on 10.6: # crypto/x509 /var/folders/a0/a0ATNr3xHliked0p+c3lIU+++TI/-Tmp-/go-build255322058/crypto/x509/_obj/_cgo_gotypes.go:14: syntax error: unexpected type, ...
4 years, 3 months ago (2013-08-02 17:59:30 UTC) #7
rsc
PTAL Clang is now only used as the default on OS X 10.8 and later.
4 years, 3 months ago (2013-08-02 18:43:58 UTC) #8
bradfitz
LGTM
4 years, 3 months ago (2013-08-02 18:55:53 UTC) #9
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=ede1d772501b *** build: on OS X 10.8 and later, use clang instead ...
4 years, 3 months ago (2013-08-02 18:58:34 UTC) #10
bradfitz
Breaks on Linux: # Building C bootstrap tool. cmd/dist cmd/dist/build.c: In function 'install': cmd/dist/build.c:642:5: error: ...
4 years, 3 months ago (2013-08-02 18:59:47 UTC) #11
rsc
Fixing, thanks.
4 years, 3 months ago (2013-08-02 19:11:10 UTC) #12
fss
4 years, 3 months ago (2013-08-02 19:13:20 UTC) #13
Breaks on 10.8 too:

% clang --version
Apple LLVM version 4.2 (clang-425.0.24) (based on LLVM 3.2svn)
Target: x86_64-apple-darwin12.4.0
Thread model: posix
% ./make.bash
# Building C bootstrap tool.
cmd/dist
cmd/dist/build.c:642:6: error: variable 'clang' is uninitialized when
used here [-Werror,-Wuninitialized]
                if(clang) {
                   ^~~~~
cmd/dist/build.c:592:39: note: initialize the variable 'clang' to
silence this warning
        bool islib, ispkg, isgo, stale, clang;
                                             ^
                                              = 0
1 error generated.



On Fri, Aug 2, 2013 at 3:59 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> Breaks on Linux:
>
> # Building C bootstrap tool.
> cmd/dist
> cmd/dist/build.c: In function 'install':
> cmd/dist/build.c:642:5: error: 'clang' may be used uninitialized in this
> function [-Werror=uninitialized]
> cc1: all warnings being treated as errors
> Build complete, duration 1.367061013s. Result: error: exit status 1
>
>
>
> On Fri, Aug 2, 2013 at 11:58 AM, <rsc@golang.org> wrote:
>>
>> *** Submitted as
>> https://code.google.com/p/go/source/detail?r=ede1d772501b ***
>>
>> build: on OS X 10.8 and later, use clang instead of gcc
>>
>>
>> Fixes issue 5822.
>> Will no doubt cause other problems, but Apple has forced our hand.
>>
>> R=golang-dev, bradfitz, khr
>> CC=golang-dev
>> https://codereview.appspot.com/12350044
>>
>>
>> https://codereview.appspot.com/12350044/
>
>
> --
>
> ---
> 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.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>



-- 
Francisco Souza
Sign in to reply to this message.

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