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

Alexandre Janniaux ajanni at videolabs.io
Fri Feb 26 17:12:12 UTC 2021


Hi,

I meant, the documentation is quite confusing here, and the commit
adding the ifdef is more precise than yours, so giving more
explanation would avoid back and forth commits.

TBH, source code is not really a stable interface, especially
when it comes to Qt, so probably just an argument for «this
can't hurt currently». ;)

Regards,
--
Alexandre Janniaux
Videolabs

On Fri, Feb 26, 2021 at 04:12:18PM +0100, Steve Lhomme wrote:
> I've seen the Qt documentation, not that blog post.
> The documentation says
>
> "Note: If you are not using qmake to build your plugin you need to make sure
> that the QT_STATICPLUGIN preprocessor macro is defined."
>
> But we are *not* building a plugin. We are using them. And that's exactly
> what's in qplugin.h. QT_STATICPLUGIN has no influence on Q_IMPORT_PLUGIN
> that we use. It has an influence on QT_MOC_EXPORT_PLUGIN which is used when
> *creating* a plugin, as suggested by the documentation.
>
> I can mention that it's a revert of that commit.
>
> On 2021-02-26 15:07, Alexandre Janniaux wrote:
> > 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
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> >
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list