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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correctly use CMake's FindX11 #1380

Merged

Conversation

shenlebantongying
Copy link
Collaborator

I was thinking about how would this line in #1369 fixes any problem.

image

The weird thing is that we already linked against X11

find_package(X11 REQUIRED)
pkg_check_modules(LIBXTST IMPORTED_TARGET xtst)
target_compile_definitions(${GOLDENDICT} PUBLIC HAVE_X11)
target_link_libraries(${GOLDENDICT} PRIVATE X11 PkgConfig::LIBXTST)

Combined with this comment #1292 (comment) , I came to realize a stupid mistake I made:

The FindX11.cmake will define X11_LIBRARIES and X11_INCLUDE_DIR but not X11

https://cmake.org/cmake/help/latest/module/FindX11.html

The working of target_link_libraries(${GOLDENDICT} PRIVATE X11 is pure accident because X11 will be translated into -lX11 馃槄

This PR thanks to @KonstantinDjairo & @Canvis-Me

Copy link

sonarcloud bot commented Jan 25, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@shenlebantongying shenlebantongying merged commit 2acf36d into xiaoyifang:staged Jan 25, 2024
13 checks passed
@KonstantinDjairo
Copy link
Contributor

tested in my freebsd machine, works perfectly
thank you for refactoring it

@shenlebantongying shenlebantongying deleted the fix-x11-cmake branch March 21, 2024 22:28
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