bugsnag / cocoapods-bugsnag

A CocoaPods plugin to integrate bugsnag into your project workspace
https://docs.bugsnag.com/platforms/ios/symbolication-guide/
MIT License
7 stars 3 forks source link

Get Info.plist path from the right location #15

Closed sethfri closed 4 years ago

sethfri commented 4 years ago

Goal

The glob the plugin currently uses to find the Info.plist doesn't work for all projects

Design

This approach uses environment variables that Xcode sets, guaranteeing that the plugin will find and use the correct Info.plist

Changeset

Changed where the build phase script was looking for the Info.plist

Testing

Pointed my own project to the fork, ran bundle install && pod install. Build succeeded instead of failing because it couldn't find the Info.plist.

johnkiely1 commented 4 years ago

Hi @sethfri,

Thanks for this. I think we may want to make this behave more like the dsym uploader plugin and include a fallback to current behaviour. We will review in due course.

sethfri commented 4 years ago

Hi @johnkiely1,

Do you mind explaining why you'd still want to use a glob? It's brittle in that it doesn't find the Info.plist in all projects. Using the env variables that Xcode sets like I'm doing in this PR is much more robust.

johnkiely1 commented 4 years ago

Hey @sethfri.

I'd agree with you that environment variables are a good idea, what I meant was we would just need to ensure that it is backwards compatible for existing users and also be consistent with our other tools.

sethfri commented 4 years ago

@johnkiely1 I believe using env variables makes this functionally a superset of the existing behavior. An explicit fallback shouldn't be necessary; I'd assume it would find the previously specified path and more.

Do you have a specific configuration you'd like me to test?

xljones commented 4 years ago

Hey @sethfri, we need to just check that it'll work with all of our supported Xcode versions (which I suspect it will), but we like this idea and will be merging it into the next release, ideally along with https://github.com/bugsnag/cocoapods-bugsnag/pull/16 subject to testing :)

sethfri commented 4 years ago

Thanks for the clarity, @xander-jones! Looking forward to it!

sethfri commented 4 years ago

Hi @xander-jones @johnkiely1, do you think you folks will be able to get this merged soon? My team is currently relying on a fork with this bug fix and would love to be able to switch back to upstream.

xljones commented 4 years ago

Hey @sethfri, I hear you. Sorry about the delay. I aim to get this merged and released next week.

sethfri commented 4 years ago

Thanks for the update @xander-jones! Looking forward to getting this merged

xljones commented 4 years ago

Hey @sethfri, this has been released in https://github.com/bugsnag/cocoapods-bugsnag/releases/tag/v2.2.0 –– Thank you for your contribution!