diff mbox series

[2/2] package/graphicsmagick: remove redundant BR2_USE_WCHAR condition

Message ID 20200918172135.85009-2-grzegorz@blach.pl
State Accepted
Headers show
Series [1/2] package/graphicsmagick: enable OpenMP when supported by toolchain | expand

Commit Message

Grzegorz Blach Sept. 18, 2020, 5:21 p.m. UTC
Signed-off-by: Grzegorz Blach <grzegorz@blach.pl>
---
 package/graphicsmagick/graphicsmagick.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Petazzoni Sept. 19, 2020, 12:33 p.m. UTC | #1
On Fri, 18 Sep 2020 19:21:33 +0200
Grzegorz Blach <grzegorz@blach.pl> wrote:

> Signed-off-by: Grzegorz Blach <grzegorz@blach.pl>
> ---
>  package/graphicsmagick/graphicsmagick.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/graphicsmagick/graphicsmagick.mk b/package/graphicsmagick/graphicsmagick.mk
> index ab00ee2c9e..7e71870482 100644
> --- a/package/graphicsmagick/graphicsmagick.mk
> +++ b/package/graphicsmagick/graphicsmagick.mk
> @@ -16,7 +16,7 @@ GRAPHICSMAGICK_CONFIG_SCRIPTS = GraphicsMagick-config GraphicsMagickWand-config
>  # 0001-MNG-Fix-small-heap-overwrite-or-assertion.patch
>  GRAPHICSMAGICK_IGNORE_CVES += CVE-2020-12672
>  
> -ifeq ($(BR2_INSTALL_LIBSTDCPP)$(BR2_USE_WCHAR),yy)
> +ifeq ($(BR2_INSTALL_LIBSTDCPP),y)

In what sense is the BR2_USE_WCHAR dependency redundant ? Redundant
with what ?

Thanks!

Thomas
Grzegorz Blach Sept. 20, 2020, 9:18 a.m. UTC | #2
On 19/09/2020 14:33, Thomas Petazzoni wrote:
> On Fri, 18 Sep 2020 19:21:33 +0200
> Grzegorz Blach <grzegorz@blach.pl> wrote:
> 
>> Signed-off-by: Grzegorz Blach <grzegorz@blach.pl>
>> ---
>>   package/graphicsmagick/graphicsmagick.mk | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/package/graphicsmagick/graphicsmagick.mk b/package/graphicsmagick/graphicsmagick.mk
>> index ab00ee2c9e..7e71870482 100644
>> --- a/package/graphicsmagick/graphicsmagick.mk
>> +++ b/package/graphicsmagick/graphicsmagick.mk
>> @@ -16,7 +16,7 @@ GRAPHICSMAGICK_CONFIG_SCRIPTS = GraphicsMagick-config GraphicsMagickWand-config
>>   # 0001-MNG-Fix-small-heap-overwrite-or-assertion.patch
>>   GRAPHICSMAGICK_IGNORE_CVES += CVE-2020-12672
>>   
>> -ifeq ($(BR2_INSTALL_LIBSTDCPP)$(BR2_USE_WCHAR),yy)
>> +ifeq ($(BR2_INSTALL_LIBSTDCPP),y)
> 
> In what sense is the BR2_USE_WCHAR dependency redundant ? Redundant
> with what ?

I mean that graphicsmagick supports C++ whenever WCHAR is enabled or 
not.  So BR2_INSTALL_LIBSTDCPP should be the only condition on this line.


> Thanks!
> 
> Thomas
>
Thomas Petazzoni Sept. 20, 2020, 11:49 a.m. UTC | #3
Hello,

On Sun, 20 Sep 2020 11:18:52 +0200
Grzegorz Blach <grzegorz@blach.pl> wrote:

> > In what sense is the BR2_USE_WCHAR dependency redundant ? Redundant
> > with what ?  
> 
> I mean that graphicsmagick supports C++ whenever WCHAR is enabled or 
> not.

Are you sure about this ? graphicsmagick.mk is copy/pasted from
imagemagick.mk, and in imagemagick.mk, this BR2_USE_WCHAR also exists,
and was explicitly added by:

commit b339bb5641c59ad2e19aeb95c1387584c7c2aeab
Author: Gustavo Zacarias <gustavo@zacarias.com.ar>
Date:   Mon Aug 12 09:06:58 2013 -0300

    imagemagick: fix magick++-config fixup error
    
    Magic++ bindings are built only with C++ and WCHAR toolchains.
    Add a WCHAR toolchain check for the magick++ config fixup.
    Looking into the future the fixup shouldn't bail on a missing file so we
    can avoid awkward kludges for packages that have many options and config
    files.
    Fixes:
    http://autobuild.buildroot.net/results/33a/33ac4b17866a64379b7bab3c0549f6e075c98dde/
    
    Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
    Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Of course, perhaps graphicsmagick is different. Have you done a test
build with C++ support enabled, but wchar disabled, and confirmed that
Magick++-config was there ?

Thanks!

Thomas
Grzegorz Blach Sept. 20, 2020, 12:31 p.m. UTC | #4
On 20/09/2020 13:49, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 20 Sep 2020 11:18:52 +0200
> Grzegorz Blach <grzegorz@blach.pl> wrote:
> 
>>> In what sense is the BR2_USE_WCHAR dependency redundant ? Redundant
>>> with what ?
>>
>> I mean that graphicsmagick supports C++ whenever WCHAR is enabled or
>> not.
> 
> Are you sure about this ? graphicsmagick.mk is copy/pasted from
> imagemagick.mk, and in imagemagick.mk, this BR2_USE_WCHAR also exists,
> and was explicitly added by:
> 
> commit b339bb5641c59ad2e19aeb95c1387584c7c2aeab
> Author: Gustavo Zacarias <gustavo@zacarias.com.ar>
> Date:   Mon Aug 12 09:06:58 2013 -0300
> 
>      imagemagick: fix magick++-config fixup error
>      
>      Magic++ bindings are built only with C++ and WCHAR toolchains.
>      Add a WCHAR toolchain check for the magick++ config fixup.
>      Looking into the future the fixup shouldn't bail on a missing file so we
>      can avoid awkward kludges for packages that have many options and config
>      files.
>      Fixes:
>      http://autobuild.buildroot.net/results/33a/33ac4b17866a64379b7bab3c0549f6e075c98dde/
>      
>      Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
>      Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> Of course, perhaps graphicsmagick is different. Have you done a test
> build with C++ support enabled, but wchar disabled, and confirmed that
> Magick++-config was there ?

Yes, I've done this test and GraphicsMagick++-config exists.

Also I am sure that 
https://github.com/hzeller/rpi-rgb-led-matrix/blob/master/utils/led-image-viewer.cc 
works correctly with WCHAR disabled.

> Thanks!
> 
> Thomas
>
diff mbox series

Patch

diff --git a/package/graphicsmagick/graphicsmagick.mk b/package/graphicsmagick/graphicsmagick.mk
index ab00ee2c9e..7e71870482 100644
--- a/package/graphicsmagick/graphicsmagick.mk
+++ b/package/graphicsmagick/graphicsmagick.mk
@@ -16,7 +16,7 @@  GRAPHICSMAGICK_CONFIG_SCRIPTS = GraphicsMagick-config GraphicsMagickWand-config
 # 0001-MNG-Fix-small-heap-overwrite-or-assertion.patch
 GRAPHICSMAGICK_IGNORE_CVES += CVE-2020-12672
 
-ifeq ($(BR2_INSTALL_LIBSTDCPP)$(BR2_USE_WCHAR),yy)
+ifeq ($(BR2_INSTALL_LIBSTDCPP),y)
 GRAPHICSMAGICK_CONFIG_SCRIPTS += GraphicsMagick++-config
 endif