|
|
DescriptionFix BoringSSL build on iOS
In order to get BoringSSL to build on iOS we need to make a couple of initial changes:
* We disable unittests due to the current inability to run new processes on iOS.
* We disable the use of custom assembly since we are unable to build the assembly for the iOS architecture and would otherwise be unable to test it.
BUG=591545
Committed: https://crrev.com/2e34051065f7e6bd139780273148d7d0c643485f
Cr-Commit-Position: refs/heads/master@{#381812}
Patch Set 1 #
Messages
Total messages: 17 (7 generated)
svaldez@chromium.org changed reviewers: + davidben@chromium.org
lgtm. Probably want to tweak the description to mention why boringssl_unittests can't be built and the assembly issue. (I think it's because target_arch is weird on iOS? Something about arch-specific bits being handled in XCode.) May as well staple it to the bug too.
Description was changed from ========== Removing iOS from testing in BoringSSL BUG= ========== to ========== Fix BoringSSL build on iOS In order to get BoringSSL to build on iOS we need to make a couple of initial changes: * We disable unittests due to the current inability to run new processes on iOS. * We disable the use of custom assembly since we are unable to build the assembly for the iOS architecture and would otherwise be unable to test it. BUG=591545 ==========
The CQ bit was checked by svaldez@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1809153002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1809153002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
The CQ bit was checked by svaldez@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1809153002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1809153002/1
Message was sent while issue was closed.
Description was changed from ========== Fix BoringSSL build on iOS In order to get BoringSSL to build on iOS we need to make a couple of initial changes: * We disable unittests due to the current inability to run new processes on iOS. * We disable the use of custom assembly since we are unable to build the assembly for the iOS architecture and would otherwise be unable to test it. BUG=591545 ========== to ========== Fix BoringSSL build on iOS In order to get BoringSSL to build on iOS we need to make a couple of initial changes: * We disable unittests due to the current inability to run new processes on iOS. * We disable the use of custom assembly since we are unable to build the assembly for the iOS architecture and would otherwise be unable to test it. BUG=591545 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Fix BoringSSL build on iOS In order to get BoringSSL to build on iOS we need to make a couple of initial changes: * We disable unittests due to the current inability to run new processes on iOS. * We disable the use of custom assembly since we are unable to build the assembly for the iOS architecture and would otherwise be unable to test it. BUG=591545 ========== to ========== Fix BoringSSL build on iOS In order to get BoringSSL to build on iOS we need to make a couple of initial changes: * We disable unittests due to the current inability to run new processes on iOS. * We disable the use of custom assembly since we are unable to build the assembly for the iOS architecture and would otherwise be unable to test it. BUG=591545 Committed: https://crrev.com/2e34051065f7e6bd139780273148d7d0c643485f Cr-Commit-Position: refs/heads/master@{#381812} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/2e34051065f7e6bd139780273148d7d0c643485f Cr-Commit-Position: refs/heads/master@{#381812}
Message was sent while issue was closed.
On 2016/03/17 17:31:45, davidben (OOO until 5-31) wrote: > lgtm. > > Probably want to tweak the description to mention why boringssl_unittests can't > be built and the assembly issue. (I think it's because target_arch is weird on > iOS? Something about arch-specific bits being handled in XCode.) May as well > staple it to the bug too. Hi! There is a problem, which was introduced in https://codereview.chromium.org/1126663002 and was not fixed here. "OS==ios" conditions are added inside "target_arch=..." conditions, but all iOS configurations have the same "target_arch" which is "ia32". This leads to some issues: 1. "OS==ios" conditions inside "target_arch=x64" conditions are not used at all. 2. All iOS configurations(including x64 and both arm-s) actually include "boringssl_mac_x86_sources" sources. 3. Starting from this commit, all iOS configurations also have "OPENSSL_NO_ASM" key, that means they include all "generic" implementations. This leads to duplicate symbols on "i386" arch.
Message was sent while issue was closed.
Do you know if there's some better way of getting the iOS configuration arch? We can probably just move the NO_ASM check outside of the target_arch for now since we aren't shipping iOS assembly yet, but it would be nice to have the correct architecture sources for when we actually enable iOS assembly. -Steven On Mon, Jun 6, 2016 at 9:34 AM <efimovmichael@yandex-team.ru> wrote: > On 2016/03/17 17:31:45, davidben (OOO until 5-31) wrote: > > lgtm. > > > > Probably want to tweak the description to mention why boringssl_unittests > can't > > be built and the assembly issue. (I think it's because target_arch is > weird on > > iOS? Something about arch-specific bits being handled in XCode.) May as > well > > staple it to the bug too. > > Hi! There is a problem, which was introduced in > https://codereview.chromium.org/1126663002 and was not fixed here. > "OS==ios" conditions are added inside "target_arch=..." conditions, but > all iOS > configurations have the same "target_arch" which is "ia32". > This leads to some issues: > 1. "OS==ios" conditions inside "target_arch=x64" conditions are not used > at all. > 2. All iOS configurations(including x64 and both arm-s) actually include > "boringssl_mac_x86_sources" sources. > 3. Starting from this commit, all iOS configurations also have > "OPENSSL_NO_ASM" > key, that means they include all "generic" implementations. This leads to > duplicate symbols on "i386" arch. > > https://codereview.chromium.org/1809153002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Message was sent while issue was closed.
Huh, I thought we took iOS out of the assembly picture completely. That some iOS mentions are still there were an oversight. We definitely need to fix that. On 2016/06/06 13:53:31, chromium-reviews wrote: > Do you know if there's some better way of getting the iOS configuration > arch? We can probably just move the NO_ASM check outside of the target_arch > for now since we aren't shipping iOS assembly yet, but it would be nice to > have the correct architecture sources for when we actually enable iOS > assembly. As I understand, we can't. The iOS build story is such that XCode handles the architecture switch, not gyp. Which means we can't do anything conditioned on architecture in gyp at all. We'd have to have a completely different iOS story (maybe wrap the assembly in ifdefs or something). > -Steven > > On Mon, Jun 6, 2016 at 9:34 AM <mailto:efimovmichael@yandex-team.ru> wrote: > > > On 2016/03/17 17:31:45, davidben (OOO until 5-31) wrote: > > > lgtm. > > > > > > Probably want to tweak the description to mention why boringssl_unittests > > can't > > > be built and the assembly issue. (I think it's because target_arch is > > weird on > > > iOS? Something about arch-specific bits being handled in XCode.) May as > > well > > > staple it to the bug too. > > > > Hi! There is a problem, which was introduced in > > https://codereview.chromium.org/1126663002 and was not fixed here. > > "OS==ios" conditions are added inside "target_arch=..." conditions, but > > all iOS > > configurations have the same "target_arch" which is "ia32". > > This leads to some issues: > > 1. "OS==ios" conditions inside "target_arch=x64" conditions are not used > > at all. > > 2. All iOS configurations(including x64 and both arm-s) actually include > > "boringssl_mac_x86_sources" sources. > > 3. Starting from this commit, all iOS configurations also have > > "OPENSSL_NO_ASM" > > key, that means they include all "generic" implementations. This leads to > > duplicate symbols on "i386" arch. > > > > https://codereview.chromium.org/1809153002/ > > > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org. |