Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2573253002:
[Cronet] Enforce building Cronet with use_platform_icu_alternatives == true (Closed)
|
Created:
4 years ago by xunjieli Modified:
4 years ago Reviewers:
pauljensen CC:
chromium-reviews, cbentzel+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Cronet] Enforce building Cronet with use_platform_icu_alternatives == true
This CL adds an condition on "cronet_package" target to enforce that
Cronet is built with use_platform_icu_alternatives == true.
BUG=611621
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
Committed: https://crrev.com/86aa1ab301820780c3eef9fb179e0e016777d47c
Cr-Commit-Position: refs/heads/master@{#438917}
Patch Set 1 #Patch Set 2 : Address comment #Messages
Total messages: 19 (12 generated)
Description was changed from ========== [Cronet] Enforce building Cronet with use_platform_icu_alternatives == true This CL adds a compile time assert to enforce that Cronet is built with use_platform_icu_alternatives == true. BUG=611621 ========== to ========== [Cronet] Enforce building Cronet with use_platform_icu_alternatives == true This CL adds a compile time assert to enforce that Cronet is built with use_platform_icu_alternatives == true. BUG=611621 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ==========
The CQ bit was checked by xunjieli@chromium.org 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...
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...)
xunjieli@chromium.org changed reviewers: + pauljensen@chromium.org
Paul, do you know what might be a good approach here? I was thinking of adding a compile-time check (which I think was suggested during triage meeting). But it looks like other bots are also trying to build cronet..
On 2016/12/15 02:36:23, xunjieli wrote: > Paul, do you know what might be a good approach here? > I was thinking of adding a compile-time check (which I think was suggested > during triage meeting). But it looks like other bots are also trying to build > cronet.. Ya, asserts don't work because Cronet is pulled in (meaning at least the ninja files are generated from the GN files) for configurations where we don't want Cronet to work, hence the trybot failures you see on this CL. I ran smack into this for ARM Neon. The best solution I could come up with is to make cronet_package an empty target when the build configuration is setup to values where we don't want to support Cronet, see: https://cs.chromium.org/chromium/src/components/cronet/android/BUILD.gn?rcl=0... You can see more on the CL if you want...but it's mostly just the round-about discussion where I came to the conclusion of what I ended up implementing: https://codereview.chromium.org/2060333002
Description was changed from ========== [Cronet] Enforce building Cronet with use_platform_icu_alternatives == true This CL adds a compile time assert to enforce that Cronet is built with use_platform_icu_alternatives == true. BUG=611621 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ========== to ========== [Cronet] Enforce building Cronet with use_platform_icu_alternatives == true This CL adds an condition on "cronet_package" target to enforce that Cronet is built with use_platform_icu_alternatives == true. BUG=611621 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ==========
Patchset #2 (id:20001) has been deleted
Done. Thanks for the suggestion! PTAL.
lgtm
The CQ bit was checked by xunjieli@chromium.org
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": 40001, "attempt_start_ts": 1481831413355630,
"parent_rev": "959b4cd2181e189acaec8067d008e55fd4e6d356", "commit_rev":
"a7560dddaaa3bc718fb113e5a7fe0fd6e9af3b23"}
Message was sent while issue was closed.
Description was changed from ========== [Cronet] Enforce building Cronet with use_platform_icu_alternatives == true This CL adds an condition on "cronet_package" target to enforce that Cronet is built with use_platform_icu_alternatives == true. BUG=611621 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ========== to ========== [Cronet] Enforce building Cronet with use_platform_icu_alternatives == true This CL adds an condition on "cronet_package" target to enforce that Cronet is built with use_platform_icu_alternatives == true. BUG=611621 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester Review-Url: https://codereview.chromium.org/2573253002 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [Cronet] Enforce building Cronet with use_platform_icu_alternatives == true This CL adds an condition on "cronet_package" target to enforce that Cronet is built with use_platform_icu_alternatives == true. BUG=611621 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester Review-Url: https://codereview.chromium.org/2573253002 ========== to ========== [Cronet] Enforce building Cronet with use_platform_icu_alternatives == true This CL adds an condition on "cronet_package" target to enforce that Cronet is built with use_platform_icu_alternatives == true. BUG=611621 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester Committed: https://crrev.com/86aa1ab301820780c3eef9fb179e0e016777d47c Cr-Commit-Position: refs/heads/master@{#438917} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/86aa1ab301820780c3eef9fb179e0e016777d47c Cr-Commit-Position: refs/heads/master@{#438917} |
