search

googleads / googleads-mobile-ios-mediation

Apache License 2.0
120 stars 137 forks source link

Update adapters build script to include arm64-simulator slice #330

Closed sgabello closed 2 years ago

sgabello commented 3 years ago

This will make the .xcframework work correctly on Apple Silicon M1 Macs.

Please note that YANDEX, VPON and MOBFOX build scripts don't use .xcframework and therefore I haven't implemented the changes for these adapters #329

sgabello commented 2 years ago

Hello, could you review this pull request so that the adapters generated will include the slice for apple silicon MacBooks?

Thanks

ericleich commented 2 years ago

@sgabello we are actively looking into updating this for each adapter that we can for the upcoming release.

A couple of reasons that make it difficult to accept your PR directly:

  1. At this point, there are merge conflicts since we already started doing this.
  2. Just because an ad network supports XCFramework, does not mean that they added an arm64 simulator slice to their SDK. Vungle is a current example. So that will block us from releasing an arm64 simulator slice for adapters as well until this is added. We'll need to inspect each XCFramework manually.
  3. Even if this CL is submitted and the adapter is built this way, we'll also still need to make changes to the CocoaPod for those users to make sure the podspec enables the arm64 simulator slice (it's explicitly disabled right now) and those pod specs aren't open sourced on this repo.

Due to the above, we plan to work on this separately. We are trying to do this ASAP as part of each adapter's next release, which in order to use GMA SDK v9 we will need new podspecs for all adapters anyways, so this should happen soon. But keep in mind that if you do use CocoaPods, upgrading to the new adapters will require updating your GMA SDK to version 9.0.0.

sgabello commented 2 years ago

@ericleich thanks you so much for the reply.

Just a couple of notes:

  1. back in September when I first submit the PR there were not so many conflicts lol.
  2. fair point but it shouldn't be your responsibility to make sure that arm64 is supported by other ad networks SDKs. If it fails, you can just ignore it and move on. If a network doesn't provide a slice, the app build process will fail on them and will push publishers to reach out to them to update their SDKs, instead of complaining with you ;)
  3. I don't use CocoaPods - I know... I might be one of the last few - so can't comment on that. But I thought CocoaPods would build the frameworks from source hence devs on Apple Silicon machines that use CocoaPods would not be impacted by this issue. If they are, I'm very surprised I'm the only one raising the problem.

In general I'm just really surprised no one is having this issue considering that M1 Macs have been around for more than one year now and AdMob is of course very popular among app developers.

To be honest it is not a big deal for me as I now have my fork that I use every time I need to update the SDKs... it's just a little time consuming to manually build the adapters every time.

Mainly I was just trying to make life easier to other fellow devs that might get stuck on this and don't know how to do it. Maybe it would be a good idea to add a note on the official documentation to explain that arm64 support is being added and adapters can be built manually from source in the meanwhile. It took me quite a bit to figure it out. :)

Anyway... Thank you so much for taking the time to reply to my PR and explain why it can't be integrated at the moment. I really appreciated it! Thanks again!

sgabello commented 2 years ago

I guess I can close the pull request then?