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

[qt5-base] deploy all platform plugins on Windows #37898

Conversation

tsondergaard
Copy link
Contributor

This is necessary as consuming applications may use e.g the offscreen plugin for unit tests. There is no real downside to deploying all platform plugins - they are small and qtdeploy.ps1 already deploys all other plugin types than platforms fully. The additional deployed plugins are these:

  • direct2d.dll
  • minimal.dll
  • offscreen.dll

Fixes #37897

  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

This is necessary as consuming applications may use e.g the offscreen
plugin for unit tests. There is no real downside to deploying all
platform plugins - they are small and qtdeploy.ps1 already deploys all
other plugin types than platforms fully. The additional deployed
plugins are these:

* direct2d.dll
* minimal.dll
* offscreen.dll

Fixes microsoft#37897
@JonLiu1993 JonLiu1993 self-assigned this Apr 1, 2024
@JonLiu1993 JonLiu1993 added the category:port-bug The issue is with a library, which is something the port should already support label Apr 1, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Apr 1, 2024

I'm mildly against this change. The plugin story is broken anyways, execpt for very few package families.

There is no real downside to deploying all platform plugins

We can expect them to be copied into all packages which deploy Qt5Gui.dll. At the moment there are app. 15.

qminimal is not related to Gui at all - that's why it is useful for tests.

@tsondergaard
Copy link
Contributor Author

tsondergaard commented Apr 1, 2024

I'm mildly against this change.

My application maintains a qt5-base overlay with local patches anyway, so it is not a huge inconvenience if this PR is rejected. Looking at the changes, though, the PR just makes the handling of the plugins simpler and more consistent.

[..]. The plugin story is broken anyways, execpt for very few package families.

Can you elaborate on this? In cmake what is the recommended/idiomatic way to copy additional plugins to the build directory?

is it just something a la this or should it be done at build time via add_custom_command or add_custom_target?

if (WIN32)
    file(COPY ${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}$<$<CONFIG:Debug>:/debug>/plugins/platforms/qoffscreen$<$<CONFIG:Debug>:d>.dll DESTINATION ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/bin/plugins/platforms)
endif()

We can expect them to be copied into all packages which deploy Qt5Gui.dll. At the moment there are app. 15.

That's a fair point. These are the files and their sizes in debug and release:

31/03/2024  12.21         4.065.280 qdirect2dd.dll
31/03/2024  12.21           691.712 qminimald.dll
31/03/2024  12.21           353.280 qoffscreend.dll
31/03/2024  12.21         3.803.648 qwindowsd.dll

31/03/2024  12.22           932.864 qdirect2d.dll
31/03/2024  12.22           206.336 qminimal.dll
31/03/2024  12.22           110.080 qoffscreen.dll
31/03/2024  12.22           863.232 qwindows.dll

The three extra plugins in debug and release versions together are about 6 MB. I guess that means this change costs an extra 90 MB space when building all 15 packages.

@dg0yt
Copy link
Contributor

dg0yt commented Apr 1, 2024

It is not just ports. This change also affects install commands for all consumers. Which means they are also entering CPack artifacts.

[..]. The plugin story is broken anyways, execpt for very few package families.

Can you elaborate on this?

Look at applocal.ps1: It has hooks only for Qt, OpenNI2, Magnum and AzureKinectSensorSDK. But there are many more packages with plugins.

In cmake what is the recommended/idiomatic way to copy additional plugins to the build directory?

I think there is no such thing.
What my Qt5 app does in CMake: inspect the CMake packages for plugins, inspect the target properties and deploy the runtime.
In fact, your project is the only place which knows plugins you are actually using.
"Simply deploy everything" is the easiest strategy to apply in projects without any help from vcpkg.

@tsondergaard
Copy link
Contributor Author

It is not just ports. This change also affects install commands for all consumers.

It does? How so? I have added the following my top-level CMakeLists.txt to install dependencies. Are you telling me I don't have to do that?

if (VCPKG_TOOLCHAIN)
  set(MY_VCPKG_INSTALLED_TRIPLET_DIR ${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET})
  set(MY_VCPKG_INSTALLED_BUILD_TYPE_DIR ${MY_VCPKG_INSTALLED_TRIPLET_DIR}$<$<CONFIG:Debug>:/debug>)
  if (UNIX)
    install(DIRECTORY ${MY_VCPKG_INSTALLED_BUILD_TYPE_DIR}/lib DESTINATION .
            FILES_MATCHING
            PATTERN *.so.*
            PATTERN liblz4*.so
            PATTERN pkgconfig EXCLUDE
            PATTERN metatypes EXCLUDE
            PATTERN ossl-modules EXCLUDE)
    install(PROGRAMS
            ${MY_VCPKG_INSTALLED_BUILD_TYPE_DIR}/bin/dbus-daemon
            ${MY_VCPKG_INSTALLED_BUILD_TYPE_DIR}/bin/dbus-launch
            ${MY_VCPKG_INSTALLED_TRIPLET_DIR}/tools/qt5$<$<CONFIG:Debug>:/debug>/QtWebEngineProcess
            DESTINATION bin)
  endif()
  if (WIN32)
    install(DIRECTORY ${MY_VCPKG_INSTALLED_BUILD_TYPE_DIR}/bin DESTINATION .
            FILES_MATCHING
            PATTERN *.dll
            PATTERN pkgconfig EXCLUDE
            PATTERN metatypes EXCLUDE
            PATTERN ossl-modules EXCLUDE)
    install(PROGRAMS
            ${MY_VCPKG_INSTALLED_BUILD_TYPE_DIR}/bin/dbus-daemon.exe
            ${MY_VCPKG_INSTALLED_BUILD_TYPE_DIR}/bin/dbus-launch.exe
            ${MY_VCPKG_INSTALLED_TRIPLET_DIR}/tools/qt5$<$<CONFIG:Debug>:/debug>/QtWebEngineProcess$<$<CONFIG:Debug>:d>.exe
            DESTINATION bin)
  endif()
  install(DIRECTORY ${MY_VCPKG_INSTALLED_BUILD_TYPE_DIR}/plugins DESTINATION .)
  install(DIRECTORY ${MY_VCPKG_INSTALLED_BUILD_TYPE_DIR}/qml DESTINATION .)
  install(FILES ${MY_VCPKG_INSTALLED_TRIPLET_DIR}/share/dbus-1/session.conf DESTINATION share/dbus-1)
  install(FILES qt.conf DESTINATION bin)
endif()

You can see I am taking the plugins directly from vcpkg_installed, so I am not affected by qtdeploy.ps1 for my install step.

@dg0yt
Copy link
Contributor

dg0yt commented Apr 1, 2024

It does? How so?

I see, it is off by default, and it is "X"perimental:

if(X_VCPKG_APPLOCAL_DEPS_INSTALL)
function(install)
z_vcpkg_function_arguments(ARGS)
_install(${ARGS})
if(ARGV0 STREQUAL "TARGETS")
# Will contain the list of targets
set(parsed_targets "")
# Destination - [RUNTIME] DESTINATION argument overrides this
set(destination "bin")
set(component_param "")
# Parse arguments given to the install function to find targets and (runtime) destination
set(modifier "") # Modifier for the command in the argument
set(last_command "") # Last command we found to process
foreach(arg IN LISTS ARGS)
if(arg MATCHES "^(ARCHIVE|LIBRARY|RUNTIME|OBJECTS|FRAMEWORK|BUNDLE|PRIVATE_HEADER|PUBLIC_HEADER|RESOURCE|INCLUDES)$")
set(modifier "${arg}")
continue()
endif()
if(arg MATCHES "^(TARGETS|DESTINATION|PERMISSIONS|CONFIGURATIONS|COMPONENT|NAMELINK_COMPONENT|OPTIONAL|EXCLUDE_FROM_ALL|NAMELINK_ONLY|NAMELINK_SKIP|EXPORT)$")
set(last_command "${arg}")
continue()
endif()
if(last_command STREQUAL "TARGETS")
list(APPEND parsed_targets "${arg}")
endif()
if(last_command STREQUAL "DESTINATION" AND (modifier STREQUAL "" OR modifier STREQUAL "RUNTIME"))
set(destination "${arg}")
endif()
if(last_command STREQUAL "COMPONENT" AND (modifier STREQUAL "" OR modifier STREQUAL "RUNTIME"))
set(component_param "COMPONENT" "${arg}")
endif()
endforeach()
x_vcpkg_install_local_dependencies(
TARGETS ${parsed_targets}
DESTINATION "${destination}"
${component_param}
)
endif()
endfunction()
endif()

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Apr 2, 2024
@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Apr 2, 2024
@BillyONeal
Copy link
Member

I need subject matter experts in Qt's help to call this one

@Osyotr
Copy link
Contributor

Osyotr commented Apr 2, 2024

add_custom_command(TARGET ${target_name}
    POST_BUILD
    COMMAND ${CMAKE_COMMAND} -E make_directory "$<TARGET_FILE_DIR:${target_name}>/platforms"
    COMMAND ${CMAKE_COMMAND} -E copy_if_different $<TARGET_FILE:Qt5::QWindowsDirect2DIntegrationPlugin> "$<TARGET_FILE_DIR:${target_name}>/platforms/"
    COMMAND ${CMAKE_COMMAND} -E copy_if_different $<TARGET_FILE:Qt5::QMinimalIntegrationPlugin> "$<TARGET_FILE_DIR:${target_name}>/platforms/"
    COMMAND ${CMAKE_COMMAND} -E copy_if_different $<TARGET_FILE:Qt5::QOffscreenIntegrationPlugin> "$<TARGET_FILE_DIR:${target_name}>/platforms/"
)

Install is similar.

@tsondergaard
Copy link
Contributor Author

@BillyONeal I'm okay with this PR being declined. I have more problems in the same category with qt5-webengine and dbus where VCPKG_APPLOCAL_DEPS=ON is insufficient to deploy all the needed runtime dependencies to the output directory. I'll work around it using something along the lines of what @Osyotr has provided in a comment higher up.

@BillyONeal
Copy link
Member

@BillyONeal I'm okay with this PR being declined.

To be clear, I wasn't trying to say it's declined. I just wanted help from people who have edited this file before as I don't have enough experience enough with it to be making judgement calls

@tsondergaard
Copy link
Contributor Author

@BillyONeal, I know. I just decided that since I have to hand-deploy a handful of other resources for qt5-webengine and dbus and the handling of tools/plugins and other runtime resources seem a little underdeveloped I thought I’d not waste your time with this relatively trivial issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[qt5-base] does not deploy all platform plugins on Windows
5 participants