Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1510973005:
[Chromecast] GN - Only apply linker flags for arm (Closed)
Description[Chromecast] GN - Only apply linker flags for arm
BUG= internal b/25566835
Committed: https://crrev.com/1c863d917bad2dac8aed686d8220fcaae3b466b6
Cr-Commit-Position: refs/heads/master@{#364489}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Style fix. #Messages
Total messages: 23 (8 generated)
Description was changed from ========== [Chromecast] GN - Only apply linker flags for arm BUG= internal b/25566835 ========== to ========== [Chromecast] GN - Only apply linker flags for arm BUG= internal b/25566835 ==========
bcf@chromium.org changed reviewers: + halliwell@chromium.org, slan@chromium.org, wzhong@chromium.org
https://codereview.chromium.org/1510973005/diff/1/build/config/chromecast/BUI... File build/config/chromecast/BUILD.gn (right): https://codereview.chromium.org/1510973005/diff/1/build/config/chromecast/BUI... build/config/chromecast/BUILD.gn:21: if (current_cpu == "arm") { Is it more preferred to put the conditionals in the config, or use variables outside of the config set based on the conditional?
+dpranke@ for OWNERS Dirk, these flags are not needed on clang builds for x86 - we only need them when cross-compiling for ARM. Please have a look. https://codereview.chromium.org/1510973005/diff/1/build/config/chromecast/BUI... File build/config/chromecast/BUILD.gn (right): https://codereview.chromium.org/1510973005/diff/1/build/config/chromecast/BUI... build/config/chromecast/BUILD.gn:18: ldflags = [] No need to delcare these here, just assign them directly inside the conditional.
slan@chromium.org changed reviewers: + dpranke@chromium.org
https://codereview.chromium.org/1510973005/diff/1/build/config/chromecast/BUI... File build/config/chromecast/BUILD.gn (right): https://codereview.chromium.org/1510973005/diff/1/build/config/chromecast/BUI... build/config/chromecast/BUILD.gn:18: ldflags = [] On 2015/12/10 01:25:54, slan wrote: > No need to delcare these here, just assign them directly inside the conditional. Done.
On 2015/12/10 01:46:38, bcf wrote: > https://codereview.chromium.org/1510973005/diff/1/build/config/chromecast/BUI... > File build/config/chromecast/BUILD.gn (right): > > https://codereview.chromium.org/1510973005/diff/1/build/config/chromecast/BUI... > build/config/chromecast/BUILD.gn:18: ldflags = [] > On 2015/12/10 01:25:54, slan wrote: > > No need to delcare these here, just assign them directly inside the > conditional. > > Done. Why just ARM? What about MIPS?
MIPs (gfiber) uses uclibc, so this work-around is not necessary.
lgtm
On 2015/12/10 02:34:17, wzhong wrote: > lgtm More specifically, according to https://docs.google.com/document/d/1GxxkgmXMtZMqSFWyDMXIho5SnUK0NSW4QFMn3qRSd..., MIPS build uses dynamic libstdc++.so.
lgtm
lgtm
The CQ bit was checked by bcf@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1510973005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1510973005/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by bcf@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1510973005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1510973005/20001
Message was sent while issue was closed.
Description was changed from ========== [Chromecast] GN - Only apply linker flags for arm BUG= internal b/25566835 ========== to ========== [Chromecast] GN - Only apply linker flags for arm BUG= internal b/25566835 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [Chromecast] GN - Only apply linker flags for arm BUG= internal b/25566835 ========== to ========== [Chromecast] GN - Only apply linker flags for arm BUG= internal b/25566835 Committed: https://crrev.com/1c863d917bad2dac8aed686d8220fcaae3b466b6 Cr-Commit-Position: refs/heads/master@{#364489} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/1c863d917bad2dac8aed686d8220fcaae3b466b6 Cr-Commit-Position: refs/heads/master@{#364489} |
