Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(60)

Issue 538563002: Don't include TargetConditionals.h when building for android. (Closed)

Created:
4 years, 8 months ago by Torne
Modified:
4 years, 8 months ago
Reviewers:
Nico
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Don't include TargetConditionals.h when building for android. Move the include of the Mac TargetConditionals.h file to after we've checked if we're building for Android. Apparently this file doesn't exist on all macs used to build android and the rest of android builds fine without it, so including it there causes the build to fail. It appears to only be used to test TARGET_OS_IPHONE which will never be true on android. BUG= Committed: https://crrev.com/ecb23485e511b4c9eaf6e20ee48c21f216abe523 Cr-Commit-Position: refs/heads/master@{#293495}

Patch Set 1 #

Patch Set 2 : Add comment to explain why #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -4 lines) Patch
M build/build_config.h View 1 2 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
Torne
Nico, someone suggested you for this one, sorry ;)
4 years, 8 months ago (2014-09-03 16:37:41 UTC) #2
Nico
lgtm Might break my local chrome/android-hosted-on-mac thing, but oh well
4 years, 8 months ago (2014-09-03 16:41:02 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/torne@chromium.org/538563002/20001
4 years, 8 months ago (2014-09-04 10:01:36 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: linux_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_triggered_tests/builds/53591) mac_gpu_retina_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_triggered_tests/builds/43679) mac_gpu_triggered_tests ...
4 years, 8 months ago (2014-09-04 18:04:18 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/torne@chromium.org/538563002/20001
4 years, 8 months ago (2014-09-05 09:25:34 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001) as 90db00ea926f657987aa75a24530ba735a81bc95
4 years, 8 months ago (2014-09-05 10:34:08 UTC) #10
commit-bot: I haz the power
4 years, 8 months ago (2014-09-10 03:38:39 UTC) #11
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/ecb23485e511b4c9eaf6e20ee48c21f216abe523
Cr-Commit-Position: refs/heads/master@{#293495}

Powered by Google App Engine
This is Rietveld 408576698