JuliaPackaging
Updated OpenCV compatibility for libcxxwrap_julia_jll to version 0.11
Good morning,
As we are working with our Professor @marcoxa, I am opening this pull request in order to fix the incompatibility problem for Julia 1.10 of OpenCV package. I'm also going to open a PR on OpenCV.jl to update the Project.toml with the CxxWrap version 0.14. I hope this will fix the incompatibility.
Hi
my students are doing a great job tracking down these issues.
Let me say that all of this should be ... easier.
You also need to change the version number of the package itself:
https://github.com/JuliaPackaging/Yggdrasil/blob/cc90a42a42a00e8a353a5fba8831aacd720ab5fd/O/OpenCV/build_tarballs.jl#L11
which basically means upgrade to a newer version
We are not the maintainers of the OpenCV Julia binding; it should be up to them to decide on a new version number.
it should be up to them to decide on a new version number.
It looks like there have been already four releases after 4.6.0: https://opencv.org/releases/
it should be up to them to decide on a new version number.
It looks like there have been already four releases after 4.6.0: https://opencv.org/releases/
Good point. But this is the OpenCV release(s).
The bigger issue here is how to bring the Julia binding of OpenCV up to speed with Julia latest incarnations.
It looks like the actual Julia wrappers are in opencv_contrib, which like OpenCV itself has a tag for version 4.9.0. Unfortunately the code in the Julia module is 3 years old, so it may need some adaptation.
The reason the OpenCV version needs to be incremented is because the compat requirements of this JLL will change by moving to a newer libcxxwrap_julia. The most elegant way to do that is by moving to a more recent OpenCV upstream version. There are workarounds by modifying the version number only here, but this is messy and best avoided.
The main reason for me to ask to change the version number was the change of the compat bound for julia, which appears unjustified. Without that, we may not need to change the version number of the package, but someone still needs to fix the wrapper somehow.
The main reason for me to ask to change the version number was the change of the compat bound for julia, which appears unjustified. Without that, we may not need to change the version number of the package, but someone still needs to fix the wrapper somehow.
That "someone still needs to fix the wrapper somehow" seems reasonable.
Now: where should this wrapper be "fixed"? In the OpenCV main repository? Meaning a PR to the OpenCV community? And what about the libcxxwrap library? How to coordinate these changes?
Now: where should this wrapper be "fixed"? In the OpenCV main repository? Meaning a PR to the OpenCV community? And what about the
libcxxwraplibrary? How to coordinate these changes?
The wrapper lives here: https://github.com/opencv/opencv_contrib/tree/4.x/modules/julia
Unfortunately, it is a little more complicated than I anticipated, since the wrapper is actually auto-generated using a Python script, so modifying that would probably be the best way forward. Looking at the git commits in that directory it seems the wrappers were added as part of a GSOC project.
Ahhhhh. A Python script. People never learn, do they?
Marco Antoniotti Somewhere over the Rainbow
On Sat, 1 Jun 2024 at 00:14, Bart Janssens @.***> wrote:
Now: where should this wrapper be "fixed"? In the OpenCV main repository? Meaning a PR to the OpenCV community? And what about the libcxxwrap library? How to coordinate these changes?
The wrapper lives here: https://github.com/opencv/opencv_contrib/tree/4.x/modules/julia
Unfortunately, it is a little more complicated than I anticipated, since the wrapper is actually auto-generated using a Python script, so modifying that would probably be the best way forward. Looking at the git commits in that directory it seems the wrappers were added as part of a GSOC project.
— Reply to this email directly, view it on GitHub https://github.com/JuliaPackaging/Yggdrasil/pull/8820#issuecomment-2143045557, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5SWTJ45MYQF2KP6S5CGDZFDY3PAVCNFSM6AAAAABISP4XOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTGA2DKNJVG4 . You are receiving this because you were mentioned.Message ID: @.***>
Getting this to work for Julia 1.10 may actually be easier than I initially thought, all that is needed is to modify bundled/patches/opencv-julia.patch to replace
@wrapmodule(OpenCV_jll.libopencv_julia_path, :cv_wrap)
with
@wrapmodule(OpenCV_jll.get_libopencv_julia_path, :cv_wrap)
The Julia 1.11 and 1.12 errors can probably also be fixed by adding a patch to opencv_contrib/modules/julia/gen/cpp_files/jlcxx/array.hpp, backporting some of the changes that were made in libcxxwrap_julia.
@barche Is there an update to this problem? And where this code should be changed? Thanks.
And where this code should be changed? Thanks.
https://github.com/JuliaPackaging/Yggdrasil/pull/8820#issuecomment-2143045557
@marcoxa The easiest thing for us to do is to apply the patches we need for opencv_contrib in Yggrdasil and build opencv 4.10 first. We can then submit them upstream to opencv_contrib so that we don't need to keep carrying them.
It seems that once we can get this over the hump, future updates should hopefully be straightforward since a lot of the machinery on the Julia side has stabilized a fair bit (fingers crossed).
Note that some of the bundled patches against opencv 4.6 will need to be reviewed and updated (or removed if those changes have been incorporated upstream).
I added some changes that will hopefully make this build against the latest libcxxwrap-julia
@barche That is an insane patch - thank you! I hope going forward it won't need this much work. Should the opencv-contrib patches be upstreamed?
Before upstreaming the opencv_contrib changes I'd like to see what can be changed in libcxxwrap-julia, since some of the changes (e.g. the container_has_less_than_operator specializations) are only needed because of a poor design in libcxxwrap-julia, that causes also problems elsewhere (such as https://github.com/JuliaInterop/CxxWrap.jl/issues/455)
Also, before merging this PR the binaries (downloadable from buildkite) should be tested with OpenCV.jl
@marcoxa @Moeasy64 Would it be possible for you to test the binaries from this PR? In the build pipeline in the CI, each of the jobs has artifacts that can be tested.
The binaries are not working because the OpenCV build system adds the flags -fvisibility=hidden -fvisibility-inlines-hidden to the compilation. I don't immediately see where, so clues welcome :)
Provided all builds still pass the binaries should work after the build is complete. At least the OpenCV tests start here in Julia 1.11 locally, with no output shown yet at the time of writing this.
So if all builds pass this is good to merge for me.
Going over the PR one more time before hitting the big green button I see this still uses Qt5Base. Any idea if this is needed? It will conflict with the Qt6Base requirement of GR (and thus Plots).
I went ahead and tried directly by just changing to Qt6Base, let's hope this recent OpenCV just picks it up. It's best to do this change now, since updating the Qt dep would require a JLL version change otherwise.