diff mbox

[v6,8/9] python-pyqt5: Depend directly on Qt5 rather than its toolchain requirements

Message ID 1485849551-20469-9-git-send-email-mr.zoltan.gyarmati@gmail.com
State Accepted
Headers show

Commit Message

Zoltan Gyarmati Jan. 31, 2017, 7:59 a.m. UTC 
From: Naumann Andreas <ANaumann@ultratronik.de>

 When pyqt5 is used it's obvious that qt5 needs to be selected and configured
by the user, hence we enforce it by making pyqt5 depending on qt5, rather than
selecting it and depending only on the qt5 requirements.

Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
Signed-off-by: Zoltan Gyarmati <mr.zoltan.gyarmati@gmail.com>
---
 package/python-pyqt5/Config.in | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

Comments

Thomas Petazzoni Feb. 1, 2017, 9:29 p.m. UTC | #1 
Hello,

On Tue, 31 Jan 2017 08:59:10 +0100, Zoltan Gyarmati wrote:
> From: Naumann Andreas <ANaumann@ultratronik.de>
> 
>  When pyqt5 is used it's obvious that qt5 needs to be selected and configured
> by the user, hence we enforce it by making pyqt5 depending on qt5, rather than
> selecting it and depending only on the qt5 requirements.
> 
> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
> Signed-off-by: Zoltan Gyarmati <mr.zoltan.gyarmati@gmail.com>

The previous version of this patch series from Andreas was doing this
change for both python-pyqt and python-pyqt5. But in your new version,
you have removed Andreas patch making the change to python-pyqt, and
only kept the python-pyqt5 change. Why? Due to this, the handling of
python-pyqt would no longer be consistent with the one of python-pyqt5.

Thanks,

Thomas
Zoltan Gyarmati Feb. 2, 2017, 8:57 a.m. UTC | #2 
On 02/01/2017 10:29 PM, Thomas Petazzoni wrote:

> Hello,
>
> On Tue, 31 Jan 2017 08:59:10 +0100, Zoltan Gyarmati wrote:
>> From: Naumann Andreas <ANaumann@ultratronik.de>
>>
>>  When pyqt5 is used it's obvious that qt5 needs to be selected and configured
>> by the user, hence we enforce it by making pyqt5 depending on qt5, rather than
>> selecting it and depending only on the qt5 requirements.
>>
>> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
>> Signed-off-by: Zoltan Gyarmati <mr.zoltan.gyarmati@gmail.com>
> The previous version of this patch series from Andreas was doing this
> change for both python-pyqt and python-pyqt5. But in your new version,
> you have removed Andreas patch making the change to python-pyqt, and
> only kept the python-pyqt5 change. Why? Due to this, the handling of
> python-pyqt would no longer be consistent with the one of python-pyqt5.

Ah, i was focusing on the qt5 related things, but you are right, it
should be consistent, i'll add it back, thanks for noting this. Did you
find any other flaw in the series?
>
> Thanks,
>
> Thomas
Thanks,

Zoltan Gyarmati
https://zgyarmati.de
Thomas Petazzoni Feb. 2, 2017, 11 a.m. UTC | #3 
Hello,

On Thu, 2 Feb 2017 09:57:57 +0100, Zoltan Gyarmati wrote:

> Ah, i was focusing on the qt5 related things, but you are right, it
> should be consistent, i'll add it back, thanks for noting this. Did you
> find any other flaw in the series?

I haven't looked very precisely at the rest of the series yet, so I
don't have other comments. I think the review from Peter Seiderer is
actually the most important one, so I'll wait for Peter to have a
look :)

Thomas
Arnout Vandecappelle Feb. 2, 2017, 11:06 p.m. UTC | #4 
On 31-01-17 08:59, Zoltan Gyarmati wrote:
> From: Naumann Andreas <ANaumann@ultratronik.de>
> 
>  When pyqt5 is used it's obvious that qt5 needs to be selected and configured
> by the user, hence we enforce it by making pyqt5 depending on qt5, rather than
> selecting it and depending only on the qt5 requirements.

 I'm not so sure that this really is an improvement. Andreas, can you explain
why this is needed?

 Regards,
 Arnout

> 
> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
> Signed-off-by: Zoltan Gyarmati <mr.zoltan.gyarmati@gmail.com>
> ---
>  package/python-pyqt5/Config.in | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/package/python-pyqt5/Config.in b/package/python-pyqt5/Config.in
> index 4eca68a..20ac988 100644
> --- a/package/python-pyqt5/Config.in
> +++ b/package/python-pyqt5/Config.in
> @@ -1,16 +1,9 @@
> -comment "python-pyqt5 needs a toolchain w/ wchar, NPTL, C++, dynamic library"
> -	depends on !BR2_PACKAGE_QT
> -	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
> -		!BR2_TOOLCHAIN_HAS_THREADS_NPTL || BR2_STATIC_LIBS
> +comment "python-pyqt5 needs Qt5"
> +	depends on !BR2_PACKAGE_QT5
>  
>  config BR2_PACKAGE_PYTHON_PYQT5
>  	bool "python-pyqt5"
> -	depends on BR2_INSTALL_LIBSTDCPP
> -	depends on BR2_USE_WCHAR # qt5
> -	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # qt5
> -	depends on !BR2_STATIC_LIBS # qt5
> -	depends on !BR2_PACKAGE_QT # qt5
> -	select BR2_PACKAGE_QT5
> +	depends on BR2_PACKAGE_QT5
>  	select BR2_PACKAGE_PYTHON_SIP
>  	help
>  	  Python bindings for Qt 5
>
Andreas Naumann Feb. 7, 2017, 8:22 a.m. UTC | #5 
Hi,

Am 03.02.2017 um 00:06 schrieb Arnout Vandecappelle:
>
>
> On 31-01-17 08:59, Zoltan Gyarmati wrote:
>> From: Naumann Andreas <ANaumann@ultratronik.de>
>>
>>  When pyqt5 is used it's obvious that qt5 needs to be selected and configured
>> by the user, hence we enforce it by making pyqt5 depending on qt5, rather than
>> selecting it and depending only on the qt5 requirements.
>
>  I'm not so sure that this really is an improvement. Andreas, can you explain
> why this is needed?

Since Qt 5.7 the toolchain needs at least C++11 support so theoretically 
we need to add that everywhere we select Qt5.
Thomas suggested that depending on rather than selecting Qt5 would 
simplify this. Maybe that fact should also go into the commit message.

regards,
Andreas

>
>  Regards,
>  Arnout
>
>>
>> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
>> Signed-off-by: Zoltan Gyarmati <mr.zoltan.gyarmati@gmail.com>
>> ---
>>  package/python-pyqt5/Config.in | 13 +++----------
>>  1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> diff --git a/package/python-pyqt5/Config.in b/package/python-pyqt5/Config.in
>> index 4eca68a..20ac988 100644
>> --- a/package/python-pyqt5/Config.in
>> +++ b/package/python-pyqt5/Config.in
>> @@ -1,16 +1,9 @@
>> -comment "python-pyqt5 needs a toolchain w/ wchar, NPTL, C++, dynamic library"
>> -	depends on !BR2_PACKAGE_QT
>> -	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
>> -		!BR2_TOOLCHAIN_HAS_THREADS_NPTL || BR2_STATIC_LIBS
>> +comment "python-pyqt5 needs Qt5"
>> +	depends on !BR2_PACKAGE_QT5
>>
>>  config BR2_PACKAGE_PYTHON_PYQT5
>>  	bool "python-pyqt5"
>> -	depends on BR2_INSTALL_LIBSTDCPP
>> -	depends on BR2_USE_WCHAR # qt5
>> -	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # qt5
>> -	depends on !BR2_STATIC_LIBS # qt5
>> -	depends on !BR2_PACKAGE_QT # qt5
>> -	select BR2_PACKAGE_QT5
>> +	depends on BR2_PACKAGE_QT5
>>  	select BR2_PACKAGE_PYTHON_SIP
>>  	help
>>  	  Python bindings for Qt 5
>>
>
diff mbox

Patch

diff --git a/package/python-pyqt5/Config.in b/package/python-pyqt5/Config.in
index 4eca68a..20ac988 100644
--- a/package/python-pyqt5/Config.in
+++ b/package/python-pyqt5/Config.in
@@ -1,16 +1,9 @@ 
-comment "python-pyqt5 needs a toolchain w/ wchar, NPTL, C++, dynamic library"
-	depends on !BR2_PACKAGE_QT
-	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || \
-		!BR2_TOOLCHAIN_HAS_THREADS_NPTL || BR2_STATIC_LIBS
+comment "python-pyqt5 needs Qt5"
+	depends on !BR2_PACKAGE_QT5
 
 config BR2_PACKAGE_PYTHON_PYQT5
 	bool "python-pyqt5"
-	depends on BR2_INSTALL_LIBSTDCPP
-	depends on BR2_USE_WCHAR # qt5
-	depends on BR2_TOOLCHAIN_HAS_THREADS_NPTL # qt5
-	depends on !BR2_STATIC_LIBS # qt5
-	depends on !BR2_PACKAGE_QT # qt5
-	select BR2_PACKAGE_QT5
+	depends on BR2_PACKAGE_QT5
 	select BR2_PACKAGE_PYTHON_SIP
 	help
 	  Python bindings for Qt 5