Conversation
… namespace conflict
|
@chrisballinger Please take a look on CI status. BuildBot report trailing white spaces issue: https://pullrequest.opencv.org/buildbot/builders/precommit_docs/builds/27965 |
|
|
||
| import PackageDescription | ||
|
|
||
| let package = Package( |
There was a problem hiding this comment.
Is it really necessary to put this file into repository root?
There was a problem hiding this comment.
Packages are Git repositories, tagged with semantic versions, containing a Package.swift file at their root.
From here: https://github.com/apple/swift-package-manager/blob/main/Documentation/Usage.md
@alalek Unfortunately it's not possible to put the Package.swift file elsewhere. Alternatively, we could create another repository like opencv/opencv-swift or opencv/opencv-swift-package for the Swift Package that is periodically kept up to date with the parent repo. That might also solve the "chicken and the egg" xcframework checksum problem as well.
There was a problem hiding this comment.
We usually do not store package-specific information in the root of this repository.
we could create another repository like opencv/opencv-swift or opencv/opencv-swift-package for the Swift Package
I believe this should work.
There is an example for "opencv-python" PyPi packages: https://github.com/skvark/opencv-python
It is not the same (it is not the package itself), just contains build scripts / dependencies settings / other package manager requirements.
BTW, Do you have a simple demo example "project" which uses the package? (besides of builtin tests). It is possible to put it into samples/swift?
OpenCV release binaries are built from release tag
The chicken and egg problem ... @chrisballinger Could you please add into PR description some links on documentation about package manager requirements? |
|
@asmorkalov I took another look at the diff and don't see any obvious whitespace issues. Is there a way to view the logs for the whitespace issue? When I click into the CI job, it says "No logs" so I'm not exactly sure where to look: https://pullrequest.opencv.org/buildbot/builders/precommit_docs/builds/27965/steps/whitespace%20opencv @alalek If we split off the |
|
We lost log details (due to builds artifacts limit). I restarted the Docs builder. Logs: |
| .testTarget( | ||
| name: "OpenCVTests", | ||
| dependencies: ["OpenCV"], | ||
| path: "modules/core/misc/objc/test/swift", |
There was a problem hiding this comment.
There are several tests besides of "core" module which are not mentioned here.
Full set of tests is prepared during the build.
If we are moving to dedicated repository for the package it make sense to adapt build scripts to prepare additional folder with:
Package.swiftfile based on template fromplatforms/apple/Package.swift.template(or.insuffix) with correct checksum and adjusted tests path- copy of test files from build directory
- misc files, like readme.md (copy content from
platforms/apple/opencv-swift)
This folder should be uploaded as artifact (.zip archive should be OK).
This artifact can be used later to prepare an update (commit/PR) in dedicated "opencv-swift" repository.
Note1: All names/paths above are just examples for now.
Note2: Prefer to keep code in Python scripts instead of workflows (it is very hard to debug "workflows" or other CI's scripts)
There was a problem hiding this comment.
Ahh I wasn't aware of how the OpenCV test process works, I'll take a closer look. I'm going to break this PR into smaller functional components, because it seems like the right path forward.
- GitHub Actions for building xcframework artifacts
- Fix Size/Point/Rect symbol conflict issue
- Package.swift in separate repository
|
|
||
| #import <XCTest/XCTest.h> | ||
| #import <OpenCV/OpenCV.h> | ||
| #import <opencv2/opencv2.h> |
There was a problem hiding this comment.
There is an idea to rename "opencv2" => "OpenCV" framework.
Currently it is postponed for OpenCV 4.x. Should be changed with next major release OpenCV 5.0+ .
Need to check if we can change this for Swift in OpenCV 4.5.x (do we break existed user Swift code?)
BTW, this name change is handled by this code in "build" directory.
There was a problem hiding this comment.
Yeah I was aware but when integrated via SPM, I can't use #import <OpenCV/OpenCV.h> because it doesn't exist in Obj-C land. I saw there's a script to do a find/replace for this, but it doesn't execute before swift test. I'll see if there's another workaround.
As an aside - I do like the idea to rename to OpenCV in 5.x. But that raises an interesting point about what will happen in the Package.swift after the transition, as underlying binaryTarget name (currently opencv2) might conflict with the parent/wrapper package name OpenCV.
|
Alrighty, I made a couple smaller PRs #18976, #18977, and #18978 based on this PR, and started work on a standalone Swift Package repo here: https://github.com/Rightpoint/opencv-swift Right now it contains the Package.swift manifest and opencv added as a git submodule, currently pointed to this branch. Once things get merged and other issues ironed out, it would be ideal if the long term plan would be to move that repo under the OpenCV GitHub org to facilitate ongoing maintenance and automated release processes. |
|
I looked on content in proposed Swift package repository ( https://github.com/Rightpoint/opencv-swift ).
Meantime it would be nice to have some simple "hello-world" sample which uses OpenCV Swift package and some "how to" instructions. It will be used for validation, so command-line instructions is preferable to perform automatic validation. |
Here is preliminary support for Swift Package Manager (resolves #18398) utilizing the new
binaryTargetfeature in conjunction with xcframeworks, building upon @colejd's work in #18826. Additionally this PR includes support for GitHub Actions to build and store both static and dynamic xcframework artifacts, as seen here in the Rightpoint fork: https://github.com/Rightpoint/opencv/actions/runs/383938322There are still some additional problems to solve, namely:
swift testusing a locally compiled binary xcframeworkThis PR also changes a few things to get around a symbol conflict issue for
Rect,SizeandPoint, as they conflict with existing symbols present in theDarwinpackage. I provided typealiases for backward compatibility. I may have also broken the existing tests during my process of migrating them to working viaswift test, so I expect some follow up work there before this is mergeable.I think the trickiest bit will be figuring out a workflow that helps keep the binaryTarget URL and checksum in
Package.swiftup to date. Although you can grab the artifacts and checksum from the GitHub Actions jobs, there isn't a way to keep these URLs up to date automatically through the CI system, so every time there is a release someone will need to update these items:Where this becomes extra tricky, is that attaching a release to a tag creates sort of a chicken and the egg scenario. A tag needs to be created for a binary xcframework build to be made and checksum computed, but although the URL can be known in advance the checksum cannot. So when creating a new release tag, someone first should ensure they are using the latest xcframework and have recomputed the checksum and committed it to
Package.swiftbefore tagging. I've attempted to make this process slightly less annoying by computing the checksum on CI, so you can grab the latest checksum and xcframework binaries frommaster. Then you just need to make surePackage.swiftis updated with the latest checksum and URL before creating the new release tag.The other part that is a bit tricky, is that running the tests via
swift testrequires uncommenting the local xcframework section of thePackage.swiftfile, and then commenting out the remote xcframework:// Recompute checksum via `swift package --package-path /path/to/opencv compute-checksum /path/to/opencv2.xcframework.zip` // .binaryTarget( // name: "opencv2", // url: "https://github.com/Rightpoint/opencv/releases/download/4.5.1/opencv2-4.5.1-dynamic.xcframework.zip", // checksum: "48273710fe03eb6d6a77ca57f96ef7917db395e6c3bc62e2b495df3dc8f1a0a9" // ), // If you are compiling OpenCV locally, you can uncomment the below block to use a custom copy // e.g. `$ python platforms/apple/build_xcframework.py --dynamic build/dynamic-xcframework` .binaryTarget( name: "opencv2", path: "build/dynamic-xcframework/opencv2.xcframework" ),There may be a way to modify Rocket which has support for
hide_dev_dependencies/unhide_dev_dependenciesto include something likehide_prod_dependenciesso we can temporarily comment out the remote binaryTarget when running tests on CI.I'll probably not respond much over the next few days, but wanted to get this out there for people to start testing.
Cheers! 🦃
Pull Request Readiness Checklist
Patch to opencv_extra has the same branch name.