search

bazel-xcode / PodToBUILD

An easy way to integrate CocoaPods into Bazel
Apache License 2.0
322 stars 69 forks source link

Add a podspec_file attribute #83

Closed FuegoFro closed 4 years ago

FuegoFro commented 5 years ago

This allows a podspec to be checked into the repo, rather than only allowing an external URL as a podspec.

FuegoFro commented 5 years ago

Oh that makes sense and seems like a better, more consistent solution. Good call!

FuegoFro commented 5 years ago

Hm, looks like I was a bit hasty with my refactor. This is actually fairly tricky to get working as it does for the Python code, mainly because it in this context (creating a remote repository) we don't have access to the main workspace root. I wasn't able to figure out a way to get that, and things like repository_ctx.path just give you a path relative to the remote repository. The one way I was able to find to get around that seems to be to have a label attribute, which resolves the path before we get into the _impl function. Given that, I'm going to go back to having the additional podspec_file attribute.

FuegoFro commented 5 years ago

@jerrymarino I've cleaned this up a bit more in its previous form (adding the new attribute). If you're able to find a way to access the workspace root from this context, I'd be happy to re-try the strategy of re-using podspec_url, but otherwise I think this should be ready to go 🙂

FuegoFro commented 5 years ago

Quick ping on this, would love to see it merged in 😀

rahul-malik commented 5 years ago

@FuegoFro - Hey thanks for following up. We've been heads down on iOS 13 / Xcode 11 work and just haven't had a chance to approve / merge.

@jerrymarino - Any remaining things you think need to be done before merging?

FuegoFro commented 5 years ago

That's fair! Separately, I was extremely pleased when I saw that the Xcode 11 issues I ran into when testing it were already fixed here 😀 Keep up the great work!

FuegoFro commented 5 years ago

I hope the iOS 13 / Xcode 11 work hasn't been too bad! Let me know if there's any changes you'd like to see here 😀 We've been using this PR for a few months now, so I'm fairly confident in it's stability haha 🙂

FuegoFro commented 4 years ago

Is there anything else you'd like to see changed here before merging this in? 🙂