[vlc-devel] [PATCH] configure.ac: add option to disable Qt check
Alexandre Janniaux
ajanni at videolabs.io
Tue Oct 1 09:10:12 CEST 2019
Hi,
Thank you for improving the answer, it's very much appreciated.
I now agree with you, please discard this patch.
Regards,
--
Alexandre Janniaux
Videolabs
On Sat, Sep 28, 2019 at 12:29:30PM +0300, Rémi Denis-Courmont wrote:
> Well, that was bad way to put it, but still, the check has nothing to do with X11/Wayland, and everything to do with "vlc" being indistinctly CLI tool and a UI. There was already a check before Wayland.
>
> I'm all for improving the check but it cannot be removed, unless Qt priority is dropped to 0 - forcing people to request it explicitly with qvlc or -Iqt...
>
> Le 28 septembre 2019 09:43:58 GMT+03:00, "Rémi Denis-Courmont" <remi at remlab.net> a écrit :
> >You clearly don't understand what this check even does. Of course it's
> >still needed if X11 is dropped.
> >
> >We don't add flags to enable bugs, simple as that.
> >
> >Le 28 septembre 2019 08:24:20 GMT+03:00, Thomas Guillem
> ><thomas at gllm.fr> a écrit :
> >>
> >>
> >>On Fri, Sep 27, 2019, at 22:18, Rémi Denis-Courmont wrote:
> >>> OEM can, and most definitely will, patch the code however it wants.
> >>That's not our problem. There's transfinite number of bits of code
> >that
> >>you could remove because it slows things down or consumes memory, for
> >>an aspect that OEM does not care about.
> >>
> >>And what about linux distribs that will drop X11 support ? This check
> >>will be useless.
> >>
> >>I don't understand the refusal reason, it won't impact the default
> >>behavior, but it may help few mainteners (and it will be needed more
> >>and more since wl will finally replace x11 one day).
> >>>
> >>> Le 27 septembre 2019 17:48:33 GMT+03:00, Thomas Guillem
> >><thomas at gllm.fr> a écrit :
> >>>>
> >>>> On Fri, Sep 27, 2019, at 16:28, Rémi Denis-Courmont wrote:
> >>>>> Hi,
> >>>>>
> >>>>> There are no builds that *require* Qt, as dummy is always
> >>available. To force Qt, you need to pass "-Iqt,none" regardless of
> >this
> >>patch. This patch only breaks error handling effectively. And the
> >check
> >>is already compiled out on Windows, while we already have the
> >>--disable-vlc switch for embedded builds. So neither of these scenarii
> >>are valid use cases AFAICT.
> >>>>
> >>>> OEM could decide to build VLC with Qt support and wayland. In that
> >>case, they control the whole VLC/Wayland/Qt chain and they don't need
> >>that extra check. Wayland is more and more used on embedded devices.
> >>For me, this patch is totally legit, as long as it is enabled by
> >>default on default Linux distribs.
> >>>>
> >>>>>
> >>>>> Besides it's not even that slow without the sanitizers.
> >>>>>
> >>>>> Le 27 septembre 2019 11:18:46 GMT+03:00, Alexandre Janniaux
> >><ajanni at videolabs.io> a écrit :
> >>>>>> Qt check is useless for builds that require the Qt interface to
> >be
> >>used
> >>>>>> like embedded systems targets or that cannot fail to start like
> >>for
> >>>>>> Windows static builds. As it takes time to start the
> >vlc-qt-check,
> >>this
> >>>>>> patch provides a way to disable this.
> >>>>>>
> >>>>>> This shouldn't be used for any other build or test. configure.ac
> >
> >> | 5 +++++
> >>>>>> modules/gui/qt/Makefile.am | 4 ++++
> >>>>>> modules/gui/qt/qt.cpp | 18 +++++++++++-------
> >>>>>> 3 files changed, 20 insertions(+), 7 deletions(-)
> >>>>>>
> >>>>>> diff --git a/configure.ac b/configure.ac
> >>>>>> index 210b4ca5371..b864ab2e483 100644
> >>>>>> --- a/configure.ac
> >>>>>> +++ b/configure.ac
> >>>>>> @@ -3932,6 +3932,11 @@ AC_ARG_ENABLE([qt-qml-debug],
> >>>>>> AS_HELP_STRING([--enable-qt-qml-debug], [enable qml debuger]))
> >>>>>> AM_CONDITIONAL(QT_QML_DEBUG, [test "${enable_qt_qml_debug}" =
> >>"yes"])
> >>>>>>
> >>>>>> +AC_ARG_ENABLE([qt-check],
> >>>>>> + AS_HELP_STRING([--disable-qt-check],
> >>>>>> + [Disable Qt check before starting the interface. This will
> >>prevent Qt module from determining if it can start correctly, and
> >crash
> >>if it cannot.]))
> >>>>>> +AM_CONDITIONAL([HAVE_QT_CHECK], [test "${enable_qt_check}" !=
> >>"no"])
> >>>>>> +
> >>>>>> AS_IF([test "${enable_qt}" != "no"], [
> >>>>>> ALIASES="${ALIASES} qvlc"
> >>>>>> ])
> >>>>>> diff --git a/modules/gui/qt/Makefile.am
> >>b/modules/gui/qt/Makefile.am
> >>>>>> index af4a5d05cc8..06c372d6cbb 100644
> >>>>>> --- a/modules/gui/qt/Makefile.am
> >>>>>> +++ b/modules/gui/qt/Makefile.am
> >>>>>> @@ -681,16 +681,20 @@ gui/qt/resources.cpp: gui/qt/vlc.qrc
> >>$(libqt_plugin_la_RES) $(libqt_plugin_la_QM
> >>>>>>
> >>>>>> endif
> >>>>>>
> >>>>>> +if HAVE_QT_CHECK
> >>>>>> vlc_qt_check_SOURCES = gui/qt/vlc-qt-check.cpp
> >>>>>> vlc_qt_check_CXXFLAGS = $(AM_CXXFLAGS) $(QT_CFLAGS) -fPIC
> >>>>>> vlc_qt_check_LDADD = $(QT_LIBS)
> >>>>>> +endif
> >>>>>>
> >>>>>> if ENABLE_QT
> >>>>>> gui_LTLIBRARIES += libqt_plugin.la
> >>>>>> BUILT_SOURCES += $(nodist_libqt_plugin_la_SOURCES)
> >>>>>> if !HAVE_WIN32
> >>>>>> if !HAVE_OS2
> >>>>>> +if HAVE_QT_CHECK
> >>>>>> pkglibexec_PROGRAMS += vlc-qt-check
> >>>>>> endif
> >>>>>> endif
> >>>>>> endif
> >>>>>> +endif
> >>>>>> diff --git a/modules/gui/qt/qt.cpp b/modules/gui/qt/qt.cpp
> >>>>>> index 798f1c41e82..a5796fbd535 100644
> >>>>>> --- a/modules/gui/qt/qt.cpp
> >>>>>> +++ b/modules/gui/qt/qt.cpp
> >>>>>> @@ -29,14 +29,16 @@
> >>>>>>
> >>>>>> #include <stdlib.h>
> >>>>>> #include <unistd.h>
> >>>>>> -#ifndef _POSIX_SPAWN
> >>>>>> -# define _POSIX_SPAWN (-1)
> >>>>>> -#endif
> >>>>>> -#if (_POSIX_SPAWN >= 0)
> >>>>>> -# include <spawn.h>
> >>>>>> -# include <sys/wait.h>
> >>>>>>
> >>>>>> +#ifdef HAVE_QT_CHECK
> >>>>>> +# ifndef _POSIX_SPAWN
> >>>>>> +# define _POSIX_SPAWN (-1)
> >>>>>> +# endif
> >>>>>> +# if (_POSIX_SPAWN >= 0)
> >>>>>> +# include <spawn.h>
> >>>>>> +# include <sys/wait.h>
> >>>>>> extern "C" char **environ;
> >>>>>> +# endif
> >>>>>> #endif
> >>>>>>
> >>>>>> #include <QApplication>
> >>>>>> @@ -411,7 +413,8 @@ static int Open( vlc_object_t *p_this, bool
> >>isDialogProvider )
> >>>>>> return VLC_EGENERIC;
> >>>>>> #endif
> >>>>>>
> >>>>>> -#if (_POSIX_SPAWN >= 0)
> >>>>>> +#ifdef HAVE_QT_CHECK
> >>>>>> +# if (_POSIX_SPAWN >= 0)
> >>>>>> /* Check if QApplication works */
> >>>>>> char *path = config_GetSysPath(VLC_PKG_LIBEXEC_DIR,
> >>"vlc-qt-check");
> >>>>>> if (unlikely(path == NULL))
> >>>>>> @@ -433,6 +436,7 @@ static int Open( vlc_object_t *p_this, bool
> >>isDialogProvider )
> >>>>>> msg_Dbg(p_this, "Qt check failed (%d). Skipping.",
> >>status);
> >>>>>> return VLC_EGENERIC;
> >>>>>> }
> >>>>>> +# endif
> >>>>>> #endif
> >>>>>>
> >>>>>> /* Get the playlist before the lock to avoid a
> >>lock-order-inversion */
> >>>>>> --
> >>>>>> 2.23.0vlc-devel mailing list
> >>>>>> To unsubscribe or modify your subscription options:
> >>>>>> https://mailman.videolan.org/listinfo/vlc-devel
> >>>>>
> >>>>> --
> >>>>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
> >>excuser ma brièveté.
> >>>>> _______________________________________________
> >>>>> vlc-devel mailing list
> >>>>> To unsubscribe or modify your subscription options:
> >>>>> https://mailman.videolan.org/listinfo/vlc-devel
> >>>>
> >>>
> >>> --
> >>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
> >>excuser ma brièveté.
> >>> _______________________________________________
> >>> vlc-devel mailing list
> >>> To unsubscribe or modify your subscription options:
> >>> https://mailman.videolan.org/listinfo/vlc-devel
> >
> >--
> >Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser
> >ma brièveté.
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> _______________________________________________
> 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