bazel-xcode / PodToBUILD

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

Feat: update_pods.py to use /usr/bin/env python #200

Closed luispadron closed 2 years ago

luispadron commented 2 years ago

Summary

In macOS 12.3 Apple removed the /usr/bin/python binary. This change causes failures when running pod update through update_pods.py.

This PR uses #!/usr/bin/env python which should work with version managed python better and adds some additional fixes for issues I ran into when running this via Python 3.10.2.

jerrymarino commented 2 years ago

Thanks for the PR @luispadron 🎉 ! I think this is a good improvement. This said some Monterey releases still ship with py2. Would you add this py3 treatment behind a if statement so it still works for that env? That’s the only minor change I’d suggest.

Additional / out of scope but might help: python env is now well supported in Bazel and a idiomatic/modern upgrade could be transition the rule to a py_binary / simplify the py toolchain selection: https://github.com/pinterest/PodToBUILD/blob/master/bin/BUILD#L5 we could also check people always use “bazel run .. “.

luispadron commented 2 years ago

This said some Monterey releases still ship with py2. Would you add this py3 treatment behind a if statement so it still works for that env? That’s the only minor change I’d suggest.

Would we want to do this with the comment @timsutton suggested?

Additional / out of scope but might help: python env is now well supported in Bazel and a idiomatic/modern upgrade could be transition the rule to a py_binary / simplify the py toolchain selection: https://github.com/pinterest/PodToBUILD/blob/master/bin/BUILD#L5 we could also check people always use “bazel run .. “.

This would be sweet, I could take a go at it since I don't like having to worry about which python version this repo uses as a user. Would love some pointers here though!

woshimaliang commented 2 years ago

It should be safe to assume python3 is available (via Command-Line Tools or XCode installs?) for PodToBUILD use-cases.

If py_binary works as @jerrymarino suggests, that is even better!

luispadron commented 2 years ago

Hey all, I've updated this PR and additionally made the change @timsutton recommended to use #!/usr/bin/env python3 since using just #!/usr/bin/env python suffers from the env: python: No such file or directory error on macOS 12.3.

This change requires the Python scripts to be updated to work with Python 3 which I'll try to get done as well but could use some help since I'm no Python 2 vs. Python 3 expert 🙏🏼

Will need to follow up this PR by updating Bazel to 5.0+ since Bazel 4.2 and below still have a Python 2 dependency which leads to the same error.

garrettmoon commented 2 years ago

I didn't see this PR and put up this one 🤦‍♂️ https://github.com/pinterest/PodToBUILD/pull/203/

luispadron commented 2 years ago

@garrettmoon No worries, looks like you did more to update to correctly use Python 3. More than happy to close this in favor of #203. 🚀

luispadron commented 2 years ago

Closing in favor of #203