Skip to content

highgui: fix config verification for GTK#20164

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
mshabunin:fix-gtk-check
May 27, 2021
Merged

highgui: fix config verification for GTK#20164
opencv-pushbot merged 1 commit intoopencv:masterfrom
mshabunin:fix-gtk-check

Conversation

@mshabunin
Copy link
Copy Markdown
Contributor

Related #20116

There was a configuration problem with ENABLE_CONFIG_VERIFICATION=ON option because HAVE_GTK3 variable was not set.

opencv/CMakeLists.txt

Lines 275 to 277 in 830cb5c

OCV_OPTION(WITH_GTK_2_X "Use GTK version 2" OFF
VISIBLE_IF UNIX AND NOT APPLE AND NOT ANDROID
VERIFY HAVE_GTK AND NOT HAVE_GTK3)

@mshabunin mshabunin requested a review from alalek May 26, 2021 18:33
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.

We need to drop CMake scope created by add_subdirectory(modules)

@mshabunin
Copy link
Copy Markdown
Contributor Author

I think we can overcome scopes thing by using INTERNAL cache variables.

Alternatively, if result variables (HAVE_*) are used only for status printout and config verification (ideally they should not be used outside of respective module except these two cases), we can move these calculations into the module which owns them. For example:

Root cmake script:
ocv_log_section_register("other" "Other third-party libraries")
#... add modules
ocv_log_print_sections() # print whole libraries part
ocv_verify_config() # prints already evaluated results
Somewhere in the module:
if(WITH_LIB_X)
  #... detect something
  set(HAVE_LIB_X 1)
endif()
if(WITH_LIB_X OR HAVE_LIB_X)
  ocv_log_status(SECTION "other" MSG "LibraryX:" HAVE_LIB_X THEN "YES" ELSE "NO")
endif()
ocv_verify_option(WITH_LIB_X BY HAVE_LIB_X)

@opencv-pushbot opencv-pushbot merged commit 76e81df into opencv:master May 27, 2021
@mshabunin mshabunin deleted the fix-gtk-check branch May 28, 2021 09:58
This was referenced May 31, 2021
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.

3 participants