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.

Sign up for GitHub

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

Jump to bottom

CMake < 18 support is broken since 0.13 #209

Open
daschuer opened this issue Jan 13, 2022 · 1 comment
Open

CMake < 18 support is broken since 0.13 #209

daschuer opened this issue Jan 13, 2022 · 1 comment

Comments

@daschuer
Copy link
Contributor

daschuer commented Jan 13, 2022 •
edited
Loading

This has happened due to 70a567a
which requires CMake >= 18.0

CMake Error at /Users/runner/buildenv/mixxx-deps-2.4-arm64-osx-min11.0-0275662/scripts/buildsystems/vcpkg.cmake:578 (_add_library):
  _add_library cannot create ALIAS target "Qt5Keychain::Qt5Keychain" because
  target "qt5keychain" is imported but not globally visible.

An easy fix would be to add a version guard around the ALIAS creation.

An alternative is proposed in the related CMake issue https://gitlab.kitware.com/cmake/cmake/-/issues/20641

-add_library(Qt@QTKEYCHAIN_VERSION_INFIX@Keychain::Qt@QTKEYCHAIN_VERSION_INFIX@Keychain ALIAS qt@QTKEYCHAIN_VERSION_INFIX@keychain)
+add_library(Qt@QTKEYCHAIN_VERSION_INFIX@Keychain::Qt@QTKEYCHAIN_VERSION_INFIX@Keychain INTERFACE IMPORTED)
+target_link_libraries(Qt@QTKEYCHAIN_VERSION_INFIX@Keychain::Qt@QTKEYCHAIN_VERSION_INFIX@Keychain INTERFACE qt@QTKEYCHAIN_VERSION_INFIX@keychain

Is this equivalent?
@frankosterfeld
Copy link
Owner

I can't judge if the latter is equivalent. I'd be fine with the version guard, as it should be the safe option. Unless someone can confirm that the latter is safe, too...

daschuer referenced this issue Jan 13, 2022
@nicolasfella @frankosterfeld
Add conventional name for exported target
70a567a 
QtKeychain defines an exported CMake target (qt5keychain), but the name
does not follow the CMake convention (Foo::Bar). This is confusing at
best, especially given the name is the same as the plain library.

It is not clear to the reader whether `target_link_libraries(foo PRIVATE
qt5keychain)` links to an imported target or a plain library.

Add an alias Qt5KeyChain::Qt5KeyChain with a more conventional name.
This preserves compatibility for users that rely on the qt5keychain
target.
daschuer added a commit to daschuer/qtkeychain that referenced this issue Jan 25, 2022
@daschuer
Define a poper exported CMake target instead of introducing an alias.
7449238 
This fixes building with cmake < version 18 a regression form
70a567a issue frankosterfeld#209
@daschuer daschuer mentioned this issue Jan 25, 2022
Merged
frankosterfeld pushed a commit that referenced this issue Jan 26, 2022
@daschuer @frankosterfeld
Define a poper exported CMake target instead of introducing an alias.
e5eeb17 
This fixes building with cmake < version 18 a regression form
70a567a issue #209
@daschuer daschuer mentioned this issue May 8, 2022
Merged
@KitsuneRal KitsuneRal mentioned this issue May 30, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@frankosterfeld @daschuer