Chromium Code Reviews
Help | Chromium Project | Sign in
(221)

Issue 15570002: Force load the Xcode arc libraries when building with llvm-build. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months ago by Justin Cohen
Modified:
11 months ago
CC:
chromium-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Force load the Xcode arc libraries when building with llvm-build.

It is necessary to force load libarclite from Xcode for
third_party/llvm-build because libarclite_* is only
distributed by Xcode.

BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201917

Patch Set 1 #

Total comments: 5

Patch Set 2 : Address thakis comments #

Patch Set 3 : Put back fobjc-arc #

Total comments: 2

Patch Set 4 : Gate -fobjc-arc behind clang\!=1 #

Total comments: 2

Patch Set 5 : Addressed Stuart comments, guard on ninja #

Patch Set 6 : Put gate in right place #

Total comments: 1

Patch Set 7 : Handle multiple arch for xcode too #

Patch Set 8 : Rebase #

Messages

Total messages: 15
justincohen
What do you think?
11 months ago #1
Nico (ooo Apr 18 - Apr 20)
Maybe this should be keyed on clang==1 instead of on the generator, as ninja should ...
11 months ago #2
justincohen
Changed it to clang==1, that seems more correct to me. Kept the -fobjc_arc, though, since ...
11 months ago #3
Nico (ooo Apr 18 - Apr 20)
lgtm, but some ios person should approve this too. https://codereview.chromium.org/15570002/diff/1/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/15570002/diff/1/build/common.gypi#newcode4047 build/common.gypi:4047: ...
11 months ago #4
justincohen
https://codereview.chromium.org/15570002/diff/8001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/15570002/diff/8001/build/common.gypi#newcode4047 build/common.gypi:4047: ], On 2013/05/21 20:02:38, Nico wrote: > Shouldn't this ...
11 months ago #5
Alexander Potapenko
I confirm this patch fixes the ASan build for iOS simulator. I've updated https://sites.google.com/a/chromium.org/dev/developers/testing/addresssanitizer?pli=1#TOC-Building-on-iOS
11 months ago #6
stuartmorgan
https://codereview.chromium.org/15570002/diff/19001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/15570002/diff/19001/build/common.gypi#newcode4075 build/common.gypi:4075: ['target_arch=="armv7"', { This section doesn't make sense for Xcode ...
11 months ago #7
justincohen
@stuart: done, ptal! https://codereview.chromium.org/15570002/diff/19001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/15570002/diff/19001/build/common.gypi#newcode4075 build/common.gypi:4075: ['target_arch=="armv7"', { On 2013/05/22 09:06:13, stuartmorgan ...
11 months ago #8
stuartmorgan
https://codereview.chromium.org/15570002/diff/34001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/15570002/diff/34001/build/common.gypi#newcode4081 build/common.gypi:4081: 'OTHER_LDFLAGS': [ Couldn't you do OTHER_LDFLAGS[arch=armv7] and [arch=x86] (or ...
11 months ago #9
justincohen
I didn't know Xcode handled [arch=]. I updated the gyp to handle this case, although ...
11 months ago #10
Nico (ooo Apr 18 - Apr 20)
Unrelated to this CL: All the other chrome ports expect the target arch to be ...
11 months ago #11
stuartmorgan
On 2013/05/22 16:52:22, Nico wrote: > Requiring this for xcode might make some things simpler ...
11 months ago #12
stuartmorgan
LGTM
11 months ago #13
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/justincohen@google.com/15570002/38004
11 months ago #14
I haz the power (commit-bot)
11 months ago #15
Message was sent while issue was closed.
Change committed as 201917
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6