| Message ID | 20190722071146.30199-1-unixmania@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | package/cups-filters: fix build without NLS | expand |
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
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.
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
On Thu, Aug 1, 2019 at 1:29 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > 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! A change solving this was accepted upstream so I will submit a follow-up with that fix: https://github.com/OpenPrinting/cups-filters/pull/155
On Mon, Sep 16, 2019 at 4:00 PM Carlos Santos <unixmania@gmail.com> wrote: > > On Thu, Aug 1, 2019 at 1:29 PM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > > > 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! > > A change solving this was accepted upstream so I will submit a > follow-up with that fix: > > https://github.com/OpenPrinting/cups-filters/pull/155 > > -- > Carlos Santos <unixmania@gmail.com> Patch updated: https://patchwork.ozlabs.org/patch/1164349/ I also sent a second patch, fixing a broken symlink: https://patchwork.ozlabs.org/patch/1164350/
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 \