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

Preliminary Qt 6.5 support #26

Merged
merged 1 commit into from
Sep 30, 2023
Merged

Preliminary Qt 6.5 support #26

merged 1 commit into from
Sep 30, 2023

Conversation

ntadej
Copy link
Collaborator

@ntadej ntadej commented Apr 15, 2023

Preliminary Qt 6.5 support. This temporarily removes custom parameters feature as it has been removed from upstream Qt release.

Some additional API fixes are done based on the feedback from the Qt community.

A disclaimer is added to README as things will be moving around a bit in the next weeks.

@ntadej ntadej force-pushed the qt-65 branch 2 times, most recently from 0f0fb05 to d083405 Compare April 15, 2023 13:14
@ntadej
Copy link
Collaborator Author

ntadej commented Apr 15, 2023

I will also disable Qt6 to not spend too much time on build system changes during the reorganisation and iOS+Windows CI temporarily as they are slow.

@kebekus
Copy link

kebekus commented Apr 16, 2023

@ntadej Thank you very much for your work! I am super happy that this wonderful project makes the step toward Qt6.5.

@ntadej
Copy link
Collaborator Author

ntadej commented Apr 19, 2023

Ping @louwers again 😊

@ntadej ntadej marked this pull request as draft April 23, 2023 09:54
@kiibimees
Copy link
Collaborator

kiibimees commented Apr 28, 2023

Using QtCreator + Qt 6.5.0 on Windows give the following error:

  get_target_property() called with non-existent target
  "Qt6::QGeoServiceProviderFactoryMapLibreGLPlugin".
Call Stack (most recent call first):
  CMakeLists.txt:38 (qt_internal_add_plugin)

Perhaps somebody knows already what is wrong and saves me a lot of time? :)

@ntadej
Copy link
Collaborator Author

ntadej commented Apr 28, 2023

There's a Qt bug where they hardcode some things in their internal CMake. My goal is to try to move away from internal Qt CMake functions, but it will make the library slightly harder to either develop or use 😊

I don't have my installation at hand, but the function is called something like "qt target aliases", where they hardcode Qt6 somewhere, which should be changed to something like Qt${PROJECT_VERSION_MAJOR}.

@kiibimees
Copy link
Collaborator

Hmmm.... but in my case, it shouldn't matter. I use Qt 6 :)

And besides that, it doesn't seem to be hard-coded. At least not anymore:

# Add Qt::target and Qt6::target as aliases for the target
function(qt_internal_add_target_aliases target)
    set(versionless_alias "Qt::${target}")
    set(versionfull_alias "Qt${PROJECT_VERSION_MAJOR}::${target}")
    set_target_properties("${target}" PROPERTIES _qt_versionless_alias "${versionless_alias}")
    set_target_properties("${target}" PROPERTIES _qt_versionfull_alias "${versionfull_alias}")

    get_target_property(type "${target}" TYPE)
    if (type STREQUAL EXECUTABLE)
        add_executable("${versionless_alias}" ALIAS "${target}")
        add_executable("${versionfull_alias}" ALIAS "${target}")
    else()
        add_library("${versionless_alias}" ALIAS "${target}")
        add_library("${versionfull_alias}" ALIAS "${target}")
    endif()
endfunction()

@ntadej
Copy link
Collaborator Author

ntadej commented Apr 28, 2023

I remember now, PROJECT_VERSION_MAJOR is the version of the project that uses it (which is not 6 in our case). They also have something like QT_VERSION_MAJOR that they use elsewhere.

@kiibimees
Copy link
Collaborator

A, yes, of course! Didn't spot that. Using QT_VERSION_MAJOR doesn't work because it's evaluated only when using find_package(QT NAMES Qt6 Qt5 and not find_package(Qt6. Just hard-coded with 6 right now.

Copy link
Collaborator

@kiibimees kiibimees left a comment

Choose a reason for hiding this comment

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

Well, yes. It compiles and runs correctly and this change looks good.

I see that you have removed parameter support totally. So it's not even needed for Qt 5?

@ntadej
Copy link
Collaborator Author

ntadej commented Apr 28, 2023

My idea is to have a consistent API between Qt5 and Qt6 (as much as Qt allows). My app is still Qt5 on some platforms (Windows for example).

@ntadej ntadej marked this pull request as ready for review September 30, 2023 21:16
@ntadej ntadej merged commit 86e4dc6 into maplibre:main Sep 30, 2023
6 checks passed
@ntadej ntadej deleted the qt-65 branch September 30, 2023 21:16
@ntadej ntadej added this to the 3.0 milestone Oct 11, 2024
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.

3 participants