[vlc-devel] [PATCH] configure.ac: add option to disable Qt check

Sat Sep 28 07:24:20 CEST 2019

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190928/6a361fbc/attachment-0001.html>