[Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency

[Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency

[Alexey Brodkin]

Looks like a copy-paste issue in
commit ea1e7ee60682 ("package/gettext: turn into virtual package").

This creates a funny circular dependency in minimalistic configs
with uClibc:
------------------------->8---------------------
make libiconv-graph-depends
Getting dependency tree...

Recursion detected for  : toolchain
which is a dependency of: host-gettext-tiny
which is a dependency of: host-gettext
which is a dependency of: host-flex
which is a dependency of: host-binutils
which is a dependency of: host-gcc-final
which is a dependency of: toolchain-buildroot
which is a dependency of: toolchain
make: *** [libiconv-graph-depends] Error 1
------------------------->8---------------------

But real problem in build failure because we try to build
libiconv for target before toolchain is built.

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Vadim Kochan <vadim4j@gmail.com>
---
 package/gettext-tiny/gettext-tiny.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/gettext-tiny/gettext-tiny.mk b/package/gettext-tiny/gettext-tiny.mk
index 4fa014e8dd..f07903d203 100644
--- a/package/gettext-tiny/gettext-tiny.mk
+++ b/package/gettext-tiny/gettext-tiny.mk
@@ -50,7 +50,7 @@ endef
 HOST_GETTEXT_TINY_POST_PATCH_HOOKS += HOST_GETTEXT_TINY_COPY_EXTRA_FILES
 
 ifeq ($(BR2_ENABLE_LOCALE),)
-HOST_GETTEXT_TINY_DEPENDENCIES = libiconv
+GETTEXT_TINY_DEPENDENCIES = libiconv
 endif
 
 define HOST_GETTEXT_TINY_BUILD_CMDS
-- 
2.16.2

[Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency

[Thomas Petazzoni]

Hello,

On Mon,  8 Apr 2019 17:04:42 +0300
Alexey Brodkin <alexey.brodkin@synopsys.com> wrote:

> diff --git a/package/gettext-tiny/gettext-tiny.mk b/package/gettext-tiny/gettext-tiny.mk
> index 4fa014e8dd..f07903d203 100644
> --- a/package/gettext-tiny/gettext-tiny.mk
> +++ b/package/gettext-tiny/gettext-tiny.mk
> @@ -50,7 +50,7 @@ endef
>  HOST_GETTEXT_TINY_POST_PATCH_HOOKS += HOST_GETTEXT_TINY_COPY_EXTRA_FILES
>  
>  ifeq ($(BR2_ENABLE_LOCALE),)
> -HOST_GETTEXT_TINY_DEPENDENCIES = libiconv
> +GETTEXT_TINY_DEPENDENCIES = libiconv
>  endif

Hm, the issue I see is that gettext-tiny/Config.in does not select
BR2_PACKAGE_LIBICONV, so we would run into the error that libiconv is
used as a dependency but not selected.

While looking at this, I realize that gettext-gnu does have the
conditional dependency on libiconv, but does not select it. libiconv
seems to no longer be needed since commit
187b4d68e0ad9e92492345c5863da40e32863eda, but we forgot to remove the
libiconv conditional dependency in gettext-gnu.mk.

Vadim, do you confirm ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency

[Vadim Kochan]

Hi Thomas, Alexey, All

On Mon, Apr 08, 2019 at 11:05:30PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon,  8 Apr 2019 17:04:42 +0300
> Alexey Brodkin <alexey.brodkin@synopsys.com> wrote:
> 
> > diff --git a/package/gettext-tiny/gettext-tiny.mk b/package/gettext-tiny/gettext-tiny.mk
> > index 4fa014e8dd..f07903d203 100644
> > --- a/package/gettext-tiny/gettext-tiny.mk
> > +++ b/package/gettext-tiny/gettext-tiny.mk
> > @@ -50,7 +50,7 @@ endef
> >  HOST_GETTEXT_TINY_POST_PATCH_HOOKS += HOST_GETTEXT_TINY_COPY_EXTRA_FILES
> >  
> >  ifeq ($(BR2_ENABLE_LOCALE),)
> > -HOST_GETTEXT_TINY_DEPENDENCIES = libiconv
> > +GETTEXT_TINY_DEPENDENCIES = libiconv
> >  endif
> 
> Hm, the issue I see is that gettext-tiny/Config.in does not select
> BR2_PACKAGE_LIBICONV, so we would run into the error that libiconv is
> used as a dependency but not selected.
> 
> While looking at this, I realize that gettext-gnu does have the
> conditional dependency on libiconv, but does not select it. libiconv
> seems to no longer be needed since commit
> 187b4d68e0ad9e92492345c5863da40e32863eda, but we forgot to remove the
> libiconv conditional dependency in gettext-gnu.mk.
> 
> Vadim, do you confirm ?
> 

libiconv is needed by msgfmt and msgmerge during "translation" on the
host so I think host-gettext-tiny should depend on host-libiconv.

Sorry for this issue!

Regards,
Vadim Kochan

[Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency

[Thomas Petazzoni]

On Tue, 9 Apr 2019 00:54:10 +0300
Vadim Kochan <vadim4j@gmail.com> wrote:

> libiconv is needed by msgfmt and msgmerge during "translation" on the
> host so I think host-gettext-tiny should depend on host-libiconv.

There is no such thing as "host-libiconv". We do not need it, because
we assume the host system is iconv-capable.

The only thing gettext-tiny does for the target is install the wrapper
gettext program, so it certainly does not need libiconv.

The right fix is to remove this conditional libiconv dependency
entirely, and probably we should do the same for the gettext-gnu
package.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency

[Vadim Kochan]

Hi Thomas, Alexey, All

On Tue, Apr 09, 2019 at 08:02:07AM +0200, Thomas Petazzoni wrote:
> On Tue, 9 Apr 2019 00:54:10 +0300
> Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > libiconv is needed by msgfmt and msgmerge during "translation" on the
> > host so I think host-gettext-tiny should depend on host-libiconv.
> 
> There is no such thing as "host-libiconv". We do not need it, because
> we assume the host system is iconv-capable.
> 
> The only thing gettext-tiny does for the target is install the wrapper
> gettext program, so it certainly does not need libiconv.
> 
> The right fix is to remove this conditional libiconv dependency
> entirely, and probably we should do the same for the gettext-gnu
> package.
> 

Thanks for the explanation!

Regards,
Vadim Kochan

[Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency

[Alexey Brodkin]

Hi Vadim,

> -----Original Message-----
> From: Vadim Kochan <vadim4j@gmail.com>
> Sent: Tuesday, April 9, 2019 9:40 AM
> To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Alexey Brodkin <alexey.brodkin@synopsys.com>; buildroot at busybox.net; ARC Buildroot mailing list
> <arc-buildroot@synopsys.com>
> Subject: Re: [Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency
> 
> Hi Thomas, Alexey, All
> 
> On Tue, Apr 09, 2019 at 08:02:07AM +0200, Thomas Petazzoni wrote:
> > On Tue, 9 Apr 2019 00:54:10 +0300
> > Vadim Kochan <vadim4j@gmail.com> wrote:
> >
> > > libiconv is needed by msgfmt and msgmerge during "translation" on the
> > > host so I think host-gettext-tiny should depend on host-libiconv.
> >
> > There is no such thing as "host-libiconv". We do not need it, because
> > we assume the host system is iconv-capable.
> >
> > The only thing gettext-tiny does for the target is install the wrapper
> > gettext program, so it certainly does not need libiconv.
> >
> > The right fix is to remove this conditional libiconv dependency
> > entirely, and probably we should do the same for the gettext-gnu
> > package.
> >
> 
> Thanks for the explanation!

Are you going to prepare a fix mentioned by Thomas or should I do it myself?

-Alexey

[Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency

[Vadim Kochan]

Hi Alexey,

On Tue, Apr 09, 2019 at 07:11:30AM +0000, Alexey Brodkin wrote:
> Hi Vadim,
> 
> > -----Original Message-----
> > From: Vadim Kochan <vadim4j@gmail.com>
> > Sent: Tuesday, April 9, 2019 9:40 AM
> > To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Cc: Alexey Brodkin <alexey.brodkin@synopsys.com>; buildroot at busybox.net; ARC Buildroot mailing list
> > <arc-buildroot@synopsys.com>
> > Subject: Re: [Buildroot] [PATCH] gettext-tiny: Make libiconv only target dependency
> > 
> > Hi Thomas, Alexey, All
> > 
> > On Tue, Apr 09, 2019 at 08:02:07AM +0200, Thomas Petazzoni wrote:
> > > On Tue, 9 Apr 2019 00:54:10 +0300
> > > Vadim Kochan <vadim4j@gmail.com> wrote:
> > >
> > > > libiconv is needed by msgfmt and msgmerge during "translation" on the
> > > > host so I think host-gettext-tiny should depend on host-libiconv.
> > >
> > > There is no such thing as "host-libiconv". We do not need it, because
> > > we assume the host system is iconv-capable.
> > >
> > > The only thing gettext-tiny does for the target is install the wrapper
> > > gettext program, so it certainly does not need libiconv.
> > >
> > > The right fix is to remove this conditional libiconv dependency
> > > entirely, and probably we should do the same for the gettext-gnu
> > > package.
> > >
> > 
> > Thanks for the explanation!
> 
> Are you going to prepare a fix mentioned by Thomas or should I do it myself?
> 
> -Alexey

Sure I will do it, but a bit later today.

Regards,
Vadim Kochan