Skip to content

Xcode 12 and Python 2/3 fixes#18771

Merged
alalek merged 8 commits intoopencv:masterfrom
chrisballinger:xcode-12-fixes
Nov 13, 2020
Merged

Xcode 12 and Python 2/3 fixes#18771
alalek merged 8 commits intoopencv:masterfrom
chrisballinger:xcode-12-fixes

Conversation

@chrisballinger
Copy link
Copy Markdown
Contributor

@chrisballinger chrisballinger commented Nov 10, 2020

Pull Request Readiness Checklist

This pull request resolves a couple issues when building OpenCV using Xcode 12 on macOS Catalina:

  • IPHONEOS_DEPLOYMENT_TARGET now defaults to 9.0 because that is the minimum version allowed by Xcode 12
  • modules/objc/generator/gen_objc.py and platform build scripts have been ported to Python 2/3 using futurize as well as a few manual fixes regarding utf8/bytes handling, because it would fail if your #!/usr/bin/env python check evaluated to a python3 binary. Another alternative I considered was using #!/usr/bin/env python2, but I'm not sure how much longer Python 2 will be supported.
  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@alalek
Copy link
Copy Markdown
Member

alalek commented Nov 11, 2020

@chrisballinger Thank you for contribution!

/cc @komakai Could you please check gen_objc.py updates?

@asmorkalov asmorkalov requested a review from vpisarev November 12, 2020 09:36
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 12, 2020

@vpisarev Please take a look.

@komakai
Copy link
Copy Markdown
Contributor

komakai commented Nov 12, 2020

@chrisballinger thanks for this. I don't have much insight into Python2 -> Python3 porting, but I have run builds with and without your changes and can confirm that the generated Objective-C sources was the same in both cases.

(Note: had to run pip install future to fix "No module named builtins" build error - I guess that we need to do the same thing on the OpenCV build server)

@chrisballinger
Copy link
Copy Markdown
Contributor Author

chrisballinger commented Nov 12, 2020

@komakai Ah ideally I'd want to remove a dependency on any pip packages, I didn't realize builtins was part of future, I must have that installed globally and didn't realize. My goal was to make the script run on a stock Python 2 or Python 3 install. I'll see if it's possible to remove that dependency.

alalek
alalek reviewed Nov 12, 2020
View reviewed changes
@chrisballinger
Copy link
Copy Markdown
Contributor Author

chrisballinger commented Nov 12, 2020

Alrighty I think I was able to remove the builtins dependency.

@komakai Can you share with me an example manual test invocation of the gen_objc.py script? I'd like to double check that it runs on both Python 2 and Python 3 after my recent changes.

@alalek
Copy link
Copy Markdown
Member

alalek commented Nov 12, 2020

BTW, Used parameters of this script are visible in build logs on CI:

cd /Volumes/build-storage/build/precommit_ios/build/build_ios/build/build-armv7-iphoneos/modules/objc && /Volumes/build-storage/build/precommit_ios/opencv/modules/objc/generator/../generator/gen_objc.py -p /Volumes/build-storage/build/precommit_ios/opencv/modules/objc/generator/../../python/src2/gen2.py -c /Volumes/build-storage/build/precommit_ios/build/build_ios/build/build-armv7-iphoneos/modules/objc/gen_objc.json -t ios -f opencv2

Objective-C: Processing OpenCV modules: 11
duplicated: CLASS cv::.Algorithm : 
Traceback (most recent call last):
  File "/Volumes/build-storage/build/precommit_ios/opencv/modules/objc/generator/../generator/gen_objc.py", line 1608, in <module>
    generator.gen(srcfiles, module, dstdir, objc_base_path, common_headers, manual_classes)
  File "/Volumes/build-storage/build/precommit_ios/opencv/modules/objc/generator/../generator/gen_objc.py", line 879, in gen
    self.gen_class(ci, self.module, extension_implementations, extension_signatures)
  File "/Volumes/build-storage/build/precommit_ios/opencv/modules/objc/generator/../generator/gen_objc.py", line 1224, in gen_class
    {0}\n}};\n\n""".format(",\n    ".join(["%s = %s" % (c.name, c.value) for c in consts]), typeName, typeName)
TypeError: unicode argument expected, got 'str'

@chrisballinger
Copy link
Copy Markdown
Contributor Author

chrisballinger commented Nov 12, 2020

Thanks! I reverted the last unicode_literals commit since it turns out it actually was needed, and confirmed that things seem to be working with both both Python 2 and Python 3 with the following test:

$ python3 ./platforms/ios/build_framework.py --iphoneos_archs arm64 --iphonesimulator_archs x86_64 ../opencv-build/
$ python3 modules/objc/generator/../generator/gen_objc.py -p modules/objc/generator/../../python/src2/gen2.py -c ../opencv-build/build/build-arm64-iphoneos/modules/objc/gen_objc.json -t ios -f opencv2
$ python2 ./platforms/ios/build_framework.py --iphoneos_archs arm64 --iphonesimulator_archs x86_64 ../opencv-build/
$ python2 modules/objc/generator/../generator/gen_objc.py -p modules/objc/generator/../../python/src2/gen2.py -c ../opencv-build/build/build-arm64-iphoneos/modules/objc/gen_objc.json -t ios -f opencv2

alalek
alalek approved these changes Nov 13, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@alalek alalek merged commit 4d00ed8 into opencv:master Nov 13, 2020
@alalek alalek mentioned this pull request Nov 16, 2020
Open
@chrisballinger chrisballinger mentioned this pull request Nov 16, 2020
Merged
6 tasks
@alalek alalek mentioned this pull request Nov 27, 2020
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
@chrisballinger
Merge pull request opencv#18771 from chrisballinger:xcode-12-fixes
a4e9c42 
Xcode 12 and Python 2/3 fixes

* Fix compilation issues using Xcode 12 on macOS Catalina

* Fix macOS scripts to work on Python 2 or 3

* Fix additional issues with Python 3

* Fix additional Python 2/3 issue

* Fix another Python 2/3 issue

* Remove dependency on builtins module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek approved these changes

@vpisarev vpisarev Awaiting requested review from vpisarev

+1 more reviewer

@komakai komakai komakai approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alalek alalek

Labels

category: build/install platform: ios/osx

Projects

None yet

Milestone

4.5.1

Development

Successfully merging this pull request may close these issues.

4 participants

@chrisballinger @alalek @asmorkalov @komakai