Skip to content

cmake: fix Linux system OpenEXR detection#16294

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
h6197627:master
Jan 17, 2020
Merged

cmake: fix Linux system OpenEXR detection#16294
opencv-pushbot merged 1 commit intoopencv:3.4from
h6197627:master

Conversation

@h6197627
Copy link
Copy Markdown
Contributor

@h6197627 h6197627 commented Jan 6, 2020

resolves #16293

@asmorkalov asmorkalov self-assigned this Jan 10, 2020
@asmorkalov
Copy link
Copy Markdown
Contributor

The solution works for me at least with Ubuntu 16.04.

@asmorkalov asmorkalov added pr: needs rebase Rebase patch (and squash fixup commits) on the top of target branch category: build/install labels Jan 10, 2020
@asmorkalov
Copy link
Copy Markdown
Contributor

This patch should go into 3.4 branch first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly).

So, please:

  • change "base" branch of this PR: master => 3.4 (use "Edit" button near PR title)
  • rebase your commits from master onto 3.4 branch. For example:
    git rebase -i --onto upstream/3.4 upstream/master
    (check list of your commits, save and quit (Esc + "wq" + Enter)
    where upstream is configured by following this GitHub guide and fetched (git fetch upstream).
  • push rebased commits into source branch of your fork (with --force option)

Note: no needs to re-open PR, apply changes "inplace".

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.

ignores empty suffix ("") while iterating in FOREACH loop

Yes, it is known CMake behavior.
So lets "iterate" through macro helper (something like this).

-    SET(LIBRARY_SUFFIXES
-        "-${OPENEXR_VERSION}"	        "-${OPENEXR_VERSION}"
-        "-${OPENEXR_VERSION}_s"	        "-${OPENEXR_VERSION}_s"
-        "-${OPENEXR_VERSION}_d"	        "-${OPENEXR_VERSION}_d"
-        "-${OPEXEXR_VERSION}_s_d"	        "-${OPEXEXR_VERSION}_s_d"
-        ""	
-        "_s"	        "_s"
-        "_d"	        "_d"
-        "_s_d")	        "_s_d")
-    FOREACH(LIBRARY_SUFFIX ${LIBRARY_SUFFIXES})	    FOREACH(LIBRARY_SUFFIX ${LIBRARY_SUFFIXES})
+    macro(ocv_find_openexr LIBRARY_SUFFIX)
         ... current loop code, remove/replace "break()" ...
-    endforeach()
+    endmacro()

+    endmacro()
+    macro(ocv_try_openexr LIBRARY_SUFFIX)
+        if(NOT OPENEXR_FOUND)  # find first guard
+            ocv_find_openexr(${LIBRARY_SUFFIX})
+        endif()
+    endmacro()
+
+    ocv_try_openexr("-${OPENEXR_VERSION}"
+    ocv_try_openexr("-${OPENEXR_VERSION}_s")
+    ocv_try_openexr("-${OPENEXR_VERSION}_d")
+    ocv_try_openexr("-${OPEXEXR_VERSION}_s_d")
+    ocv_try_openexr("")
+    ocv_try_openexr("_s")
+    ocv_try_openexr("_d")
+    ocv_try_openexr("_s_d")

if(X86_64)
SET(OPENEXR_LIBSEARCH_SUFFIXES x86_64-linux-gnu)
elseif(X86)
SET(OPENEXR_LIBSEARCH_SUFFIXES i386-linux-gnu)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider using CMAKE_LIBRARY_ARCHITECTURE variable instead.

string(TOUPPER "${LIBRARY_NAME}" LIBRARY_NAME_UPPER)
FIND_LIBRARY(OPENEXR_${LIBRARY_NAME_UPPER}_LIBRARY
NAMES ${LIBRARY_NAME}${LIBRARY_SUFFIX}
NAMES ${LIBRARY_NAME} ${LIBRARY_NAME}${LIBRARY_SUFFIX}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This would re-introduce mess with names again.

All libraries for found components must have the same suffix. Different subsets must be forbidden.

@asmorkalov
Copy link
Copy Markdown
Contributor

@h6197627 friendly reminder

@h6197627 h6197627 changed the base branch from master to 3.4 January 16, 2020 12:40
@h6197627
Copy link
Copy Markdown
Contributor Author

@asmorkalov, I rebased and tried to address @alalek comments

@asmorkalov
Copy link
Copy Markdown
Contributor

@h6197627 Looks good to me. Please squash the commits and I'll merge the patch.

@h6197627
Copy link
Copy Markdown
Contributor Author

@asmorkalov, done

@asmorkalov asmorkalov added category: build/install and removed category: build/install pr: needs rebase Rebase patch (and squash fixup commits) on the top of target branch labels Jan 17, 2020
@asmorkalov
Copy link
Copy Markdown
Contributor

👍

@asmorkalov asmorkalov self-requested a review January 17, 2020 07:04
opencv-pushbot pushed a commit that referenced this pull request Jan 17, 2020
@opencv-pushbot opencv-pushbot merged commit fb3a334 into opencv:3.4 Jan 17, 2020
@alalek alalek mentioned this pull request Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants