Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2682223004:
Support selecting architectures in iOS build script (Closed)
|
Created:
3 years, 10 months ago by oprypin_webrtc Modified:
3 years, 10 months ago CC:
webrtc-reviews_webrtc.org Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionSupport selecting architectures in iOS build script
BUG=webrtc:7127
Review-Url: https://codereview.webrtc.org/2682223004
Cr-Commit-Position: refs/heads/master@{#16587}
Committed: https://chromium.googlesource.com/external/webrtc/+/9b0dbd443888852065e8584646d65460435d0196
Patch Set 1 #
Total comments: 5
Patch Set 2 : Unrelated comment fix #Patch Set 3 : Validate architecture selections #Patch Set 4 : Rebase #Messages
Total messages: 28 (17 generated)
The CQ bit was checked by oprypin@webrtc.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.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Support selecting architectures in iOS build script BUG=webrtc:7127 ========== to ========== Support selecting architectures in iOS build script BUG=webrtc:7127 ==========
oprypin@webrtc.org changed reviewers: + kjellander@webrtc.org, kthelgason@webrtc.org
lgtm!
https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... File tools-webrtc/ios/build_ios_libs.py (right): https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... tools-webrtc/ios/build_ios_libs.py:140: architectures = list(args.arch) Maybe make sure that the list only contains architectures that make sense?
https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... File tools-webrtc/ios/build_ios_libs.py (right): https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... tools-webrtc/ios/build_ios_libs.py:140: architectures = list(args.arch) On 2017/02/09 12:27:53, kthelgason wrote: > Maybe make sure that the list only contains architectures that make sense? How can I determine what makes sense? Limit it only to supported architectures (are they currently the same as ENABLED_ARCHITECTURES/DEFAULT_ARCHS)?
lgtm, thanks for improving the readability of the script as well. https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... File tools-webrtc/ios/build_ios_libs.py (right): https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... tools-webrtc/ios/build_ios_libs.py:140: architectures = list(args.arch) On 2017/02/09 12:39:47, oprypin_webrtc wrote: > On 2017/02/09 12:27:53, kthelgason wrote: > > Maybe make sure that the list only contains architectures that make sense? > > How can I determine what makes sense? Limit it only to supported architectures > (are they currently the same as ENABLED_ARCHITECTURES/DEFAULT_ARCHS)? There are only those four today, so you could validate against DEFAULT_ARCHS. Another option is to add another constant: ENABLED_ARCHITECTURES = DEFAULT_ARCHS[:] and use it to be explicit in case it might change in the future.
https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... File tools-webrtc/ios/build_ios_libs.py (right): https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... tools-webrtc/ios/build_ios_libs.py:140: architectures = list(args.arch) On 2017/02/09 12:39:47, oprypin_webrtc wrote: > On 2017/02/09 12:27:53, kthelgason wrote: > > Maybe make sure that the list only contains architectures that make sense? > > How can I determine what makes sense? Limit it only to supported architectures > (are they currently the same as ENABLED_ARCHITECTURES/DEFAULT_ARCHS)? Yeah, I was thinking something like: architectures = list(filter(lambda arch: arch in ENABLED_ARCHITECTURES, args.arch))
The CQ bit was checked by oprypin@webrtc.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.webrtc.org/...
https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... File tools-webrtc/ios/build_ios_libs.py (right): https://codereview.webrtc.org/2682223004/diff/1/tools-webrtc/ios/build_ios_li... tools-webrtc/ios/build_ios_libs.py:140: architectures = list(args.arch) On 2017/02/09 12:27:53, kthelgason wrote: > Maybe make sure that the list only contains architectures that make sense? Done.
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 oprypin@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from kthelgason@webrtc.org, kjellander@webrtc.org Link to the patchset: https://codereview.webrtc.org/2682223004/#ps40001 (title: "Validate architecture selections")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_api_framework on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_api_framework/build...) ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/17445) ios_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/22701) mac_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/22713)
The CQ bit was checked by oprypin@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from kthelgason@webrtc.org, kjellander@webrtc.org Link to the patchset: https://codereview.webrtc.org/2682223004/#ps60001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486999339279630,
"parent_rev": "bcd88dbc01ee75f9c15a44dd392ee7cc68704021", "commit_rev":
"9b0dbd443888852065e8584646d65460435d0196"}
Message was sent while issue was closed.
Description was changed from ========== Support selecting architectures in iOS build script BUG=webrtc:7127 ========== to ========== Support selecting architectures in iOS build script BUG=webrtc:7127 Review-Url: https://codereview.webrtc.org/2682223004 Cr-Commit-Position: refs/heads/master@{#16587} Committed: https://chromium.googlesource.com/external/webrtc/+/9b0dbd443888852065e858464... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/external/webrtc/+/9b0dbd443888852065e858464... |
