package/cups-filters: fix build without NLS
diff mbox series

Message ID 20190722071146.30199-1-unixmania@gmail.com
State New
Headers show
Series
  • package/cups-filters: fix build without NLS
Related show

Commit Message

Carlos Santos July 22, 2019, 7:11 a.m. UTC
From: Carlos Santos <unixmania@gmail.com>

texttotext must be linked to libiconv if !BR2_ENABLE_LOCALE so patch
Makefile.am to add the required -liconv argument.

We could patch configure.ac to use AM_ICONV but it would require more
change adding AM_GNU_GETTEXT_VERSION([x.y.z]) to bring iconv.m4.

Fixes: https://bugs.busybox.net/show_bug.cgi?id=12031

Signed-off-by: Carlos Santos <unixmania@gmail.com>
---
 package/cups-filters/cups-filters.mk | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Vadim Kochan July 22, 2019, 7:22 a.m. UTC | #1
Hi Carlos,


On Mon, Jul 22, 2019 at 10:12 AM <unixmania@gmail.com> wrote:
>
> From: Carlos Santos <unixmania@gmail.com>
>
> texttotext must be linked to libiconv if !BR2_ENABLE_LOCALE so patch
> Makefile.am to add the required -liconv argument.
>
> We could patch configure.ac to use AM_ICONV but it would require more
> change adding AM_GNU_GETTEXT_VERSION([x.y.z]) to bring iconv.m4.
>
> Fixes: https://bugs.busybox.net/show_bug.cgi?id=12031
>
> Signed-off-by: Carlos Santos <unixmania@gmail.com>
> ---
>  package/cups-filters/cups-filters.mk | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/package/cups-filters/cups-filters.mk b/package/cups-filters/cups-filters.mk
> index 214647a90e..8368be5e4b 100644
> --- a/package/cups-filters/cups-filters.mk
> +++ b/package/cups-filters/cups-filters.mk
> @@ -13,6 +13,17 @@ CUPS_FILTERS_AUTORECONF = YES
>
>  CUPS_FILTERS_DEPENDENCIES = cups libglib2 lcms2 qpdf fontconfig freetype jpeg
>
> +# texttotext must be linked to libiconv if !BR2_ENABLE_LOCALE. We could patch
> +# configure.ac to use AM_ICONV but it would require iconv.m4, increasing the
> +# patch size by at least 130 lines. Let's keep it simple and just patch
> +# Makefile.am to append -liconv to the link command line.
> +ifeq ($(BR2_PACKAGE_LIBICONV),y)
> +define CUPS_FILTERS_LINK_LIBICONV
> +       $(SED) '/texttotext_LDADD =/s/(CUPS_LIBS)$$/(CUPS_LIBS) -liconv/' $(@D)/Makefile.am
> +endef
> +CUPS_FILTERS_PRE_CONFIGURE_HOOKS += CUPS_FILTERS_LINK_LIBICONV
> +endif
> +
>  CUPS_FILTERS_CONF_OPTS = --disable-imagefilters \
>         --disable-mutool \
>         --disable-foomatic \
> --
> 2.18.1
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

I already sent patch to fix this problem (but I did not check the
LOCALE config so looks like your version is better):
    https://patchwork.ozlabs.org/patch/1134318

but in case of your version is it not enough to just add:

    CUPS_FILTERS_CONF_ENV += LIBS=-liconv

?

Regards,
Vadim Kochan
Carlos Santos July 22, 2019, 11:02 a.m. UTC | #2
On Mon, Jul 22, 2019 at 4:24 AM Vadim Kochan <vadim4j@gmail.com> wrote:
>
> Hi Carlos,
>
>
> On Mon, Jul 22, 2019 at 10:12 AM <unixmania@gmail.com> wrote:
> >
> > From: Carlos Santos <unixmania@gmail.com>
> >
> > texttotext must be linked to libiconv if !BR2_ENABLE_LOCALE so patch
> > Makefile.am to add the required -liconv argument.
> >
> > We could patch configure.ac to use AM_ICONV but it would require more
> > change adding AM_GNU_GETTEXT_VERSION([x.y.z]) to bring iconv.m4.
> >
> > Fixes: https://bugs.busybox.net/show_bug.cgi?id=12031
> >
> > Signed-off-by: Carlos Santos <unixmania@gmail.com>
> > ---
> >  package/cups-filters/cups-filters.mk | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/package/cups-filters/cups-filters.mk b/package/cups-filters/cups-filters.mk
> > index 214647a90e..8368be5e4b 100644
> > --- a/package/cups-filters/cups-filters.mk
> > +++ b/package/cups-filters/cups-filters.mk
> > @@ -13,6 +13,17 @@ CUPS_FILTERS_AUTORECONF = YES
> >
> >  CUPS_FILTERS_DEPENDENCIES = cups libglib2 lcms2 qpdf fontconfig freetype jpeg
> >
> > +# texttotext must be linked to libiconv if !BR2_ENABLE_LOCALE. We could patch
> > +# configure.ac to use AM_ICONV but it would require iconv.m4, increasing the
> > +# patch size by at least 130 lines. Let's keep it simple and just patch
> > +# Makefile.am to append -liconv to the link command line.
> > +ifeq ($(BR2_PACKAGE_LIBICONV),y)
> > +define CUPS_FILTERS_LINK_LIBICONV
> > +       $(SED) '/texttotext_LDADD =/s/(CUPS_LIBS)$$/(CUPS_LIBS) -liconv/' $(@D)/Makefile.am
> > +endef
> > +CUPS_FILTERS_PRE_CONFIGURE_HOOKS += CUPS_FILTERS_LINK_LIBICONV
> > +endif
> > +
> >  CUPS_FILTERS_CONF_OPTS = --disable-imagefilters \
> >         --disable-mutool \
> >         --disable-foomatic \
> > --
> > 2.18.1
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
>
> I already sent patch to fix this problem (but I did not check the
> LOCALE config so looks like your version is better):
>     https://patchwork.ozlabs.org/patch/1134318
>
> but in case of your version is it not enough to just add:
>
>     CUPS_FILTERS_CONF_ENV += LIBS=-liconv
>
> ?

This would link all programs to libiconv, not just texttotext.
Thomas Petazzoni Aug. 1, 2019, 4:29 p.m. UTC | #3
On Mon, 22 Jul 2019 04:11:46 -0300
unixmania@gmail.com wrote:

> +# texttotext must be linked to libiconv if !BR2_ENABLE_LOCALE. We could patch
> +# configure.ac to use AM_ICONV but it would require iconv.m4, increasing the
> +# patch size by at least 130 lines. Let's keep it simple and just patch
> +# Makefile.am to append -liconv to the link command line.

Why is this AM_ICONV solution taking at least 130 lines ? This would be
upstreamable, and looks a lot nicer. Could you propose a patch with an
AM_ICONV base solution instead ?

Thanks!

Thomas

Patch
diff mbox series

diff --git a/package/cups-filters/cups-filters.mk b/package/cups-filters/cups-filters.mk
index 214647a90e..8368be5e4b 100644
--- a/package/cups-filters/cups-filters.mk
+++ b/package/cups-filters/cups-filters.mk
@@ -13,6 +13,17 @@  CUPS_FILTERS_AUTORECONF = YES
 
 CUPS_FILTERS_DEPENDENCIES = cups libglib2 lcms2 qpdf fontconfig freetype jpeg
 
+# texttotext must be linked to libiconv if !BR2_ENABLE_LOCALE. We could patch
+# configure.ac to use AM_ICONV but it would require iconv.m4, increasing the
+# patch size by at least 130 lines. Let's keep it simple and just patch
+# Makefile.am to append -liconv to the link command line.
+ifeq ($(BR2_PACKAGE_LIBICONV),y)
+define CUPS_FILTERS_LINK_LIBICONV
+	$(SED) '/texttotext_LDADD =/s/(CUPS_LIBS)$$/(CUPS_LIBS) -liconv/' $(@D)/Makefile.am
+endef
+CUPS_FILTERS_PRE_CONFIGURE_HOOKS += CUPS_FILTERS_LINK_LIBICONV
+endif
+
 CUPS_FILTERS_CONF_OPTS = --disable-imagefilters \
 	--disable-mutool \
 	--disable-foomatic \