Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2858723002:
allow_posix_link_time_opt and is_cfi are clang features (Closed)
|
Created:
3 years, 7 months ago by Mostyn Bramley-Moore Modified:
3 years, 7 months ago CC:
chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Descriptionallow_posix_link_time_opt and is_cfi are clang features
This is required for GCC builds with is_official_build=true.
BUG=464797
Review-Url: https://codereview.chromium.org/2858723002
Cr-Commit-Position: refs/heads/master@{#469188}
Committed: https://chromium.googlesource.com/chromium/src/+/9dd38d20453e05cecaa0f883f0865da3837a380c
Patch Set 1 #
Total comments: 2
Patch Set 2 : move is_clang to allow_posix_link_time_opt def #Messages
Total messages: 21 (13 generated)
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mostynb@opera.com changed reviewers: + dpranke@chromium.org, krasin@google.com
@krasin, dpranke: please take a look at this fixup for GCC builds with is_official_build=true.
... I guess the TODO note might need modification, eg "replace this flag with is_clang when ..." ?
krasin@chromium.org changed reviewers: + krasin@chromium.org, pcc@chromium.org - krasin@google.com
Hi Mostyn, i might lose the access to krasin@chromium.org account today, adding Peter (pcc@chromium.org) for a proper review. Also, one comment. https://codereview.chromium.org/2858723002/diff/1/build/config/sanitizers/san... File build/config/sanitizers/sanitizers.gni (right): https://codereview.chromium.org/2858723002/diff/1/build/config/sanitizers/san... build/config/sanitizers/sanitizers.gni:58: is_official_build && allow_posix_link_time_opt && is_clang allow_posix_link_time is a clang feature. Can you please move is_clang into allow_posix_link_time_opt definition?
https://codereview.chromium.org/2858723002/diff/1/build/config/sanitizers/san... File build/config/sanitizers/sanitizers.gni (right): https://codereview.chromium.org/2858723002/diff/1/build/config/sanitizers/san... build/config/sanitizers/sanitizers.gni:58: is_official_build && allow_posix_link_time_opt && is_clang On 2017/05/02 19:48:23, krasin1 wrote: > allow_posix_link_time is a clang feature. Can you please move is_clang into > allow_posix_link_time_opt definition? Thanks- I was testing that change simultaneously in another workspace, didn't spot that they were inter-related.
Description was changed from ========== CFI is a clang feature BUG=464797 ========== to ========== allow_posix_link_time_opt and is_cfi are clang features BUG=464797 ==========
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM, over to dpranke@ for OWNERS review.
Description was changed from ========== allow_posix_link_time_opt and is_cfi are clang features BUG=464797 ========== to ========== allow_posix_link_time_opt and is_cfi are clang features This is required for GCC builds with is_official_build=true. BUG=464797 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
lgtm
The CQ bit was checked by mostynb@opera.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493844366665890,
"parent_rev": "5a2d8707dff6a410c0511c423ed837651559ce92", "commit_rev":
"9dd38d20453e05cecaa0f883f0865da3837a380c"}
Message was sent while issue was closed.
Description was changed from ========== allow_posix_link_time_opt and is_cfi are clang features This is required for GCC builds with is_official_build=true. BUG=464797 ========== to ========== allow_posix_link_time_opt and is_cfi are clang features This is required for GCC builds with is_official_build=true. BUG=464797 Review-Url: https://codereview.chromium.org/2858723002 Cr-Commit-Position: refs/heads/master@{#469188} Committed: https://chromium.googlesource.com/chromium/src/+/9dd38d20453e05cecaa0f883f086... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/9dd38d20453e05cecaa0f883f086... |
