[vlc-devel] [PATCH] gui: qt: don't condition plugin import based on QT_STATICPLUGIN

[Alexandre Janniaux]

Hi,

This is probably about safety when using this [1]:

> As described in How to Create Qt Plugins, it is possible to switch off
> the automatic loading of plugins with CONFIG -= import_plugins, but this
> disables the generation of the weatherinfo_plugin_import.cpp file, and
> then we would have to do everything manually: add Q_IMPORT_PLUGIN
> somewhere in our code, define the QT_STATICPLUGIN preprocessor variable
> and link the plugin (as one has to do without qmake).

Your commit seems to be a silent revert of

commit 98ada626762841b5976ca71166fd2b4b970f19ea
Author: Martell Malone <martellmalone at gmail.com>
Date:   Tue Nov 25 13:23:38 2014 +0000

    Qt: add QT_STATICPLUGIN define as per the qt spec

    The QT spec says we should use QT_STATICPLUGIN defined if building a
    static plugin.

    As we are using out own makefile and not a qt .pro project we have to
    define it ourselves.

    This is then used to decide if we should import the modules
    QWindowsIntegrationPlugin and AccessibleFactory as these are not needed
    on a shared build.

    Signed-off-by: Jean-Baptiste Kempf <jb at videolan.org>

What's the rationale behind this change?

[1]: https://www.qt.io/blog/2014/08/27/qt-weekly-18-static-linking-with-qt

On Fri, Feb 26, 2021 at 02:17:48PM +0100, Steve Lhomme wrote:
> QT_STATICPLUGIN is only used to export plugins, it has no impact on importing
> plugins (see qplugin.h).
>
> We even define it on our own when QT_STATIC is set.
> ---
>  modules/gui/qt/maininterface/mainui.cpp | 2 +-
>  modules/gui/qt/qt.cpp                   | 2 --
>  modules/gui/qt/qt.hpp                   | 4 ----
>  3 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/modules/gui/qt/maininterface/mainui.cpp b/modules/gui/qt/maininterface/mainui.cpp
> index 61bfc50e2e3..5d1cd1500f5 100644
> --- a/modules/gui/qt/maininterface/mainui.cpp
> +++ b/modules/gui/qt/maininterface/mainui.cpp
> @@ -118,7 +118,7 @@ bool MainUI::setup(QQmlEngine* engine)
>      {
>          for(auto& error: m_component->errors())
>              msg_Err(m_intf, "qml loading %s %s:%u", qtu(error.description()), qtu(error.url().toString()), error.line());
> -#ifdef QT_STATICPLUGIN
> +#ifdef QT_STATIC
>              assert( !"Missing qml modules from qt contribs." );
>  #else
>              msg_Err( m_intf, "Install missing modules using your packaging tool" );
> diff --git a/modules/gui/qt/qt.cpp b/modules/gui/qt/qt.cpp
> index 6bb0eefdb7a..a2da98b66e8 100644
> --- a/modules/gui/qt/qt.cpp
> +++ b/modules/gui/qt/qt.cpp
> @@ -72,7 +72,6 @@ extern "C" char **environ;
>   #include <QtPlugin>
>   #include <QQuickWindow>
>
> - #ifdef QT_STATICPLUGIN
>    Q_IMPORT_PLUGIN(QSvgIconPlugin)
>    Q_IMPORT_PLUGIN(QSvgPlugin)
>    Q_IMPORT_PLUGIN(QJpegPlugin)
> @@ -94,7 +93,6 @@ extern "C" char **environ;
>     Q_IMPORT_PLUGIN(QWindowsVistaStylePlugin)
>     Q_IMPORT_PLUGIN(QWindowsIntegrationPlugin)
>    #endif
> - #endif
>  #endif
>
>  #ifndef X_DISPLAY_MISSING
> diff --git a/modules/gui/qt/qt.hpp b/modules/gui/qt/qt.hpp
> index a283a02e63b..bb3447df546 100644
> --- a/modules/gui/qt/qt.hpp
> +++ b/modules/gui/qt/qt.hpp
> @@ -35,10 +35,6 @@
>
>  #include <qconfig.h>
>
> -#ifdef QT_STATIC
> -#define QT_STATICPLUGIN
> -#endif
> -
>  #define QT_NO_CAST_TO_ASCII
>  #include <QString>
>  #include <QUrl>
> --
> 2.29.2
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel