Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link libdeflate for OpenEXR 3.2+ (cmake) #1274

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Razzaline
Copy link
Contributor

This just adds a line to the main CMake file, fixing #1248 for now (not compiling in Linux because of libdeflate not being explicitly linked). If there's a better fix, I don't mind others suggesting something, or adding to or changing it.

@manongjohn
Copy link
Collaborator

The build failed as I had expected since currently libdeflate is not a required dependency and therefore not loaded on current Ubuntu builds.

/usr/bin/ld: cannot find -ldeflate
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@Razzaline
Copy link
Contributor Author

Razzaline commented Nov 11, 2023

Oh, that makes sense >.< I just looked into it and it does appear that Ubuntu and a lot of other distributions don't have OpenEXR 3.2+ in their package repositories. I don't really know how to proceed or check for that, though... Maybe with a CMake module checking the OpenEXR version? Technically, OpenCV can be compiled without OpenEXR, but every distro that I looked at seems to require OpenEXR with OpenCV.

@manongjohn
Copy link
Collaborator

manongjohn commented Nov 11, 2023

As part of T2D builds, I locally compile OpenCV to adhere to an LGPL compliant license. It's possible I have exluded OpenEXR on purpose which explains why it isnt looking for it or its dependencies (i.e. libdeflate).

EDIT: I confirmed my build of OpenCV excludes OpenEXR.

@Razzaline
Copy link
Contributor Author

Razzaline commented Nov 14, 2023

Okay, that makes sense. After you mentioned that OpenEXR wasn't included, I checked and noticed that in the CI build scripts.

Sorry about the merge commit there >.< I hadn't realised it wouldn't ask me for confirmation when I told it to sync. But I've added a hopefully slightly better solution? It checks for the OpenEXR version and if it doesn't find it I think it should just ignore it. I don't have a lot of experience with CMake so I'm not entirely sure. Otherwise, it does seem to be linking libdeflate properly for me.

@Razzaline
Copy link
Contributor Author

I can probably add the same logic for Windows and Mac OS, in the unlikely event that someone's build environment happens to have OpenEXR 3.2+ on an OpenEXR-enabled build, although I guess that might happen eventually anyway so it's good to have the checks in place.

@Razzaline
Copy link
Contributor Author

Razzaline commented Nov 15, 2023

For now I just added the Apple build environment stuff. Adding the check for MSVC might involve creating a FindOpenEXR.cmake (there are no prebuilt binary releases of OpenEXR for Windows) so I thought I should leave it for later.

@Razzaline Razzaline changed the title Link libdeflate for Linux builds (cmake) Link libdeflate for OpenEXR 3.2+ (cmake) Nov 15, 2023
@Razzaline Razzaline marked this pull request as draft November 16, 2023 18:46
@Razzaline
Copy link
Contributor Author

I'm planning to finish this soon if possible (and was hoping to before the 1.4 release), but I've been stuck trying to figure out the Windows part of it (hence running a build in a virtual machine and encountering #1300). I've made a few additional changes I haven't pushed yet, but if all else fails I could just push what I have with the Windows parts commented out (though they are written in a way that under normal circumstances CMake should just skip over that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants