diff mbox series

[1/3,v4] package/qt5/qt5webengine: fix ffmpeg/codec/alsa option handling for latest

Message ID 73c8adc83c7e856136f5eacd8aab44631b0a64cc.1600290685.git.yann.morin.1998@free.fr
State Changes Requested
Headers show
Series [1/3,v4] package/qt5/qt5webengine: fix ffmpeg/codec/alsa option handling for latest | expand

Commit Message

Yann E. MORIN Sept. 16, 2020, 9:11 p.m. UTC 
From: Nathan Ford <nford@westpond.com>

As of qt 5.10, options are handled as arguments to qmake instead of
defines:

    qt5webengine-5.12.2$ vi dist/changes-5.9.0

    Important Changes
    -----------------

     - Configure options are now handled by the global configure script. This
       means options previously controlled by WEBENGINE_CONFIG options should
       now use configure flags. For instance the configure command-line option
       -proprietary-codecs replaces WEBENGINE_CONFIG+=use_proprietary_codecs.

    qt5webengine-5.12.2$ vi dist/changes-5.10.0

    Important Changes
    -----------------

       * WebEngine Features are now configured as Qt features and
         WEBENGINE_CONFIG has been removed.

Fixes: #12416 (which was an inspiration for this patch)

Signed-off-by: Nathan Ford <nford@westpond.com>
[yann.morin.1998@free.fr:
  - refresh on top of master, as we no longer have the version choice
  - extend commit log
  - mocve the new dependency to its own commit
]
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Peter Seiderer <ps.report@gmx.net>
---
 package/qt5/qt5webengine/qt5webengine.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Thomas Petazzoni Sept. 17, 2020, 12:55 p.m. UTC | #1 
Hello,

On Wed, 16 Sep 2020 23:11:32 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> diff --git a/package/qt5/qt5webengine/qt5webengine.mk b/package/qt5/qt5webengine/qt5webengine.mk
> index 9101f35937..5df6942977 100644
> --- a/package/qt5/qt5webengine/qt5webengine.mk
> +++ b/package/qt5/qt5webengine/qt5webengine.mk
> @@ -29,16 +29,16 @@ endif
>  
>  QT5WEBENGINE_DEPENDENCIES += host-libpng host-libnss libnss
>  
> -QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_system_ffmpeg
> +QT5WEBENGINE_CONF_OPTS = -webengine-ffmpeg
>  
>  ifeq ($(BR2_PACKAGE_QT5WEBENGINE_PROPRIETARY_CODECS),y)
> -QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_proprietary_codecs
> +QT5WEBENGINE_CONF_OPTS += -webengine-proprietary-codecs
>  endif
>  
>  ifeq ($(BR2_PACKAGE_QT5WEBENGINE_ALSA),y)
>  QT5WEBENGINE_DEPENDENCIES += alsa-lib
>  else
> -QT5WEBENGINE_CONF_OPTS += QT_CONFIG-=alsa
> +QT5WEBENGINE_CONF_OPTS += -no-webengine-alsa
>  endif

In the original patch, there was a "--" that was added to separate
these webengine options from the main options. You have not kept that.
Are you sure this is OK ?

Thomas
Yann E. MORIN Sept. 17, 2020, 3:39 p.m. UTC | #2 
Thomas, All,

On 2020-09-17 14:55 +0200, Thomas Petazzoni spake thusly:
> On Wed, 16 Sep 2020 23:11:32 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > diff --git a/package/qt5/qt5webengine/qt5webengine.mk b/package/qt5/qt5webengine/qt5webengine.mk
> > index 9101f35937..5df6942977 100644
> > --- a/package/qt5/qt5webengine/qt5webengine.mk
> > +++ b/package/qt5/qt5webengine/qt5webengine.mk
> > @@ -29,16 +29,16 @@ endif
> >  
> >  QT5WEBENGINE_DEPENDENCIES += host-libpng host-libnss libnss
> >  
> > -QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_system_ffmpeg
> > +QT5WEBENGINE_CONF_OPTS = -webengine-ffmpeg
> >  
> >  ifeq ($(BR2_PACKAGE_QT5WEBENGINE_PROPRIETARY_CODECS),y)
> > -QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_proprietary_codecs
> > +QT5WEBENGINE_CONF_OPTS += -webengine-proprietary-codecs
> >  endif
> >  
> >  ifeq ($(BR2_PACKAGE_QT5WEBENGINE_ALSA),y)
> >  QT5WEBENGINE_DEPENDENCIES += alsa-lib
> >  else
> > -QT5WEBENGINE_CONF_OPTS += QT_CONFIG-=alsa
> > +QT5WEBENGINE_CONF_OPTS += -no-webengine-alsa
> >  endif
> 
> In the original patch, there was a "--" that was added to separate
> these webengine options from the main options. You have not kept that.
> Are you sure this is OK ?

I am not sure, hence I said in the cover-letter "Note: this is not even
build-tested; it's just a refresh-n-respin."

I'll do a test-build tonight (I did not have spare CPU cycles yesterday,
but tonight I should have some.)

However, none of the qmake-package we have use that '--'.

Regards,
Yann E. MORIN.
Peter Seiderer Sept. 17, 2020, 5:15 p.m. UTC | #3 
Hello Yann,

On Thu, 17 Sep 2020 17:39:10 +0200, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Thomas, All,
>
> On 2020-09-17 14:55 +0200, Thomas Petazzoni spake thusly:
> > On Wed, 16 Sep 2020 23:11:32 +0200
> > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > > diff --git a/package/qt5/qt5webengine/qt5webengine.mk b/package/qt5/qt5webengine/qt5webengine.mk
> > > index 9101f35937..5df6942977 100644
> > > --- a/package/qt5/qt5webengine/qt5webengine.mk
> > > +++ b/package/qt5/qt5webengine/qt5webengine.mk
> > > @@ -29,16 +29,16 @@ endif
> > >
> > >  QT5WEBENGINE_DEPENDENCIES += host-libpng host-libnss libnss
> > >
> > > -QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_system_ffmpeg
> > > +QT5WEBENGINE_CONF_OPTS = -webengine-ffmpeg
> > >
> > >  ifeq ($(BR2_PACKAGE_QT5WEBENGINE_PROPRIETARY_CODECS),y)
> > > -QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_proprietary_codecs
> > > +QT5WEBENGINE_CONF_OPTS += -webengine-proprietary-codecs
> > >  endif
> > >
> > >  ifeq ($(BR2_PACKAGE_QT5WEBENGINE_ALSA),y)
> > >  QT5WEBENGINE_DEPENDENCIES += alsa-lib
> > >  else
> > > -QT5WEBENGINE_CONF_OPTS += QT_CONFIG-=alsa
> > > +QT5WEBENGINE_CONF_OPTS += -no-webengine-alsa
> > >  endif
> >
> > In the original patch, there was a "--" that was added to separate
> > these webengine options from the main options. You have not kept that.
> > Are you sure this is OK ?
>
> I am not sure, hence I said in the cover-letter "Note: this is not even
> build-tested; it's just a refresh-n-respin."
>
> I'll do a test-build tonight (I did not have spare CPU cycles yesterday,
> but tonight I should have some.)
>
> However, none of the qmake-package we have use that '--'.

I believe Thomas is right, all other qmake-packages set
an option <name>=|-=|+=<value> as the old behavior before
the patch and not the 'real' option one after the patch
(see above 'QT_CONFIG-=alsa' vs. '-no-webengine-alsa' or
in case for the first one '-- -webengine-ffmpeg'...

Regards,
Peter

>
> Regards,
> Yann E. MORIN.
>
Nathan Ford Sept. 17, 2020, 7:23 p.m. UTC | #4 
Hi All,

I can't speak for newer qt5 versions, but for 5.14.2 the -- is
required. I don't understand the qt build system well enough to
explain why.

--Nate

On Thu, Sep 17, 2020 at 1:16 PM Peter Seiderer <ps.report@gmx.net> wrote:
>
> Hello Yann,
>
> On Thu, 17 Sep 2020 17:39:10 +0200, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
>
> > Thomas, All,
> >
> > On 2020-09-17 14:55 +0200, Thomas Petazzoni spake thusly:
> > > On Wed, 16 Sep 2020 23:11:32 +0200
> > > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > > > diff --git a/package/qt5/qt5webengine/qt5webengine.mk b/package/qt5/qt5webengine/qt5webengine.mk
> > > > index 9101f35937..5df6942977 100644
> > > > --- a/package/qt5/qt5webengine/qt5webengine.mk
> > > > +++ b/package/qt5/qt5webengine/qt5webengine.mk
> > > > @@ -29,16 +29,16 @@ endif
> > > >
> > > >  QT5WEBENGINE_DEPENDENCIES += host-libpng host-libnss libnss
> > > >
> > > > -QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_system_ffmpeg
> > > > +QT5WEBENGINE_CONF_OPTS = -webengine-ffmpeg
> > > >
> > > >  ifeq ($(BR2_PACKAGE_QT5WEBENGINE_PROPRIETARY_CODECS),y)
> > > > -QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_proprietary_codecs
> > > > +QT5WEBENGINE_CONF_OPTS += -webengine-proprietary-codecs
> > > >  endif
> > > >
> > > >  ifeq ($(BR2_PACKAGE_QT5WEBENGINE_ALSA),y)
> > > >  QT5WEBENGINE_DEPENDENCIES += alsa-lib
> > > >  else
> > > > -QT5WEBENGINE_CONF_OPTS += QT_CONFIG-=alsa
> > > > +QT5WEBENGINE_CONF_OPTS += -no-webengine-alsa
> > > >  endif
> > >
> > > In the original patch, there was a "--" that was added to separate
> > > these webengine options from the main options. You have not kept that.
> > > Are you sure this is OK ?
> >
> > I am not sure, hence I said in the cover-letter "Note: this is not even
> > build-tested; it's just a refresh-n-respin."
> >
> > I'll do a test-build tonight (I did not have spare CPU cycles yesterday,
> > but tonight I should have some.)
> >
> > However, none of the qmake-package we have use that '--'.
>
> I believe Thomas is right, all other qmake-packages set
> an option <name>=|-=|+=<value> as the old behavior before
> the patch and not the 'real' option one after the patch
> (see above 'QT_CONFIG-=alsa' vs. '-no-webengine-alsa' or
> in case for the first one '-- -webengine-ffmpeg'...
>
> Regards,
> Peter
>
> >
> > Regards,
> > Yann E. MORIN.
> >
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Yann E. MORIN Sept. 17, 2020, 9:08 p.m. UTC | #5 
Nathan, Peter, All,

On 2020-09-17 15:23 -0400, Nathan Ford spake thusly:
> I can't speak for newer qt5 versions, but for 5.14.2 the -- is
> required. I don't understand the qt build system well enough to
> explain why.

As said on the cover letter, I did not even build-tested it, and just
refreshed the series, and I semi-voluntarily removed the --. I now did a
build, and they are indeed needed.

Thanks for the sharp eyes!

Regards,
Yann E. MORIN.
diff mbox series

Patch

diff --git a/package/qt5/qt5webengine/qt5webengine.mk b/package/qt5/qt5webengine/qt5webengine.mk
index 9101f35937..5df6942977 100644
--- a/package/qt5/qt5webengine/qt5webengine.mk
+++ b/package/qt5/qt5webengine/qt5webengine.mk
@@ -29,16 +29,16 @@  endif
 
 QT5WEBENGINE_DEPENDENCIES += host-libpng host-libnss libnss
 
-QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_system_ffmpeg
+QT5WEBENGINE_CONF_OPTS = -webengine-ffmpeg
 
 ifeq ($(BR2_PACKAGE_QT5WEBENGINE_PROPRIETARY_CODECS),y)
-QT5WEBENGINE_CONF_OPTS += WEBENGINE_CONFIG+=use_proprietary_codecs
+QT5WEBENGINE_CONF_OPTS += -webengine-proprietary-codecs
 endif
 
 ifeq ($(BR2_PACKAGE_QT5WEBENGINE_ALSA),y)
 QT5WEBENGINE_DEPENDENCIES += alsa-lib
 else
-QT5WEBENGINE_CONF_OPTS += QT_CONFIG-=alsa
+QT5WEBENGINE_CONF_OPTS += -no-webengine-alsa
 endif
 
 # QtWebengine's build system uses python, but only supports python2. We work