[Buildroot] [PATCH 1/2] package/libgpg-error: optionally depends on gettext

[Buildroot] [PATCH 1/2] package/libgpg-error: optionally depends on gettext

[Arnout Vandecappelle]

configure.ac has AM_GNU_GETTEXT()

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Discovered while investigating
http://autobuild.buildroot.net/results/f81/f81eabb37788aa6dcdadf4034889c84bef78b876/

I suspect there are many more packages like this... Perhaps it would
make sense to consider gettext (and therefore libiconv) to be part of
the toolchain...
---
 package/libgpg-error/libgpg-error.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/libgpg-error/libgpg-error.mk b/package/libgpg-error/libgpg-error.mk
index e081b36..dfe58e7 100644
--- a/package/libgpg-error/libgpg-error.mk
+++ b/package/libgpg-error/libgpg-error.mk
@@ -11,6 +11,10 @@ LIBGPG_ERROR_LICENSE_FILES = COPYING COPYING.LIB
 LIBGPG_ERROR_INSTALL_STAGING = YES
 LIBGPG_ERROR_CONFIG_SCRIPTS = gpg-error-config
 
+ifeq ($(BR2_NEEDS_GETTEXT),y)
+LIBGPG_ERROR_DEPENDENCIES += gettext
+endif
+
 define LIBGPG_ERROR_FIX_CROSS_COMPILATION
 	cd $(@D)/src/syscfg && \
 	ln -s lock-obj-pub.$(call qstrip, $(BR2_PACKAGE_LIBGPG_ERROR_SYSCFG)).h \
-- 
2.9.3

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Arnout Vandecappelle]

libgcrypt pulls in gpg-error which links with libintl if available.
Since iputils doesn't use libtool, -lintl has to be passed explicitly
for static builds.

Fixes:
http://autobuild.buildroot.net/results/f81/f81eabb37788aa6dcdadf4034889c84bef78b876
http://autobuild.buildroot.net/results/f29/f296e8fba1e79f96d3d119aa4c8207ed4b80694b
http://autobuild.buildroot.net/results/478/478a88cd892a119970cfca717ea7f5517bfc4cea
http://autobuild.buildroot.net/results/920/920d2d1d967691779f79fa54a38fac5adc5a8ee4

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/iputils/iputils.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
index d6398f7..47f3e28 100644
--- a/package/iputils/iputils.mk
+++ b/package/iputils/iputils.mk
@@ -26,6 +26,10 @@ endif
 IPUTILS_MAKE_OPTS = $(TARGET_CONFIGURE_OPTS) USE_SYSFS=no USE_IDN=no\
 	CFLAGS="$(TARGET_CFLAGS) -D_GNU_SOURCE"
 
+ifeq ($(BR2_NEEDS_GETTEXT)$(BR2_PACKAGE_GETTEXT),yy)
+IPUTILS_MAKE_OPTS += ADDLIB='-lintl'
+endif
+
 ifeq ($(BR2_PACKAGE_LIBCAP),y)
 IPUTILS_MAKE_OPTS += USE_CAP=yes
 IPUTILS_DEPENDENCIES += libcap
-- 
2.9.3

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Baruch Siach]

Hi Arnout,

On Thu, Aug 25, 2016 at 01:10:03AM +0200, Arnout Vandecappelle (Essensium/Mind) wrote:
> libgcrypt pulls in gpg-error which links with libintl if available.
> Since iputils doesn't use libtool, -lintl has to be passed explicitly
> for static builds.
> 
> Fixes:
> http://autobuild.buildroot.net/results/f81/f81eabb37788aa6dcdadf4034889c84bef78b876
> http://autobuild.buildroot.net/results/f29/f296e8fba1e79f96d3d119aa4c8207ed4b80694b
> http://autobuild.buildroot.net/results/478/478a88cd892a119970cfca717ea7f5517bfc4cea
> http://autobuild.buildroot.net/results/920/920d2d1d967691779f79fa54a38fac5adc5a8ee4
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  package/iputils/iputils.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk
> index d6398f7..47f3e28 100644
> --- a/package/iputils/iputils.mk
> +++ b/package/iputils/iputils.mk
> @@ -26,6 +26,10 @@ endif
>  IPUTILS_MAKE_OPTS = $(TARGET_CONFIGURE_OPTS) USE_SYSFS=no USE_IDN=no\
>  	CFLAGS="$(TARGET_CFLAGS) -D_GNU_SOURCE"
>  
> +ifeq ($(BR2_NEEDS_GETTEXT)$(BR2_PACKAGE_GETTEXT),yy)
> +IPUTILS_MAKE_OPTS += ADDLIB='-lintl'
> +endif

I think this should be limited to BR2_STATIC_LIBS. It may not change much the 
end result, but it makes the motivation clearer.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Thomas Petazzoni]

Hello,

On Thu, 25 Aug 2016 01:10:03 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:

> +ifeq ($(BR2_NEEDS_GETTEXT)$(BR2_PACKAGE_GETTEXT),yy)
> +IPUTILS_MAKE_OPTS += ADDLIB='-lintl'
> +endif

Why do you need both BR2_NEEDS_GETTEXT=y and BR2_PACKAGE_GETTEXT=y ?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Arnout Vandecappelle]

On 25-08-16 10:18, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 25 Aug 2016 01:10:03 +0200, Arnout Vandecappelle
> (Essensium/Mind) wrote:
> 
>> +ifeq ($(BR2_NEEDS_GETTEXT)$(BR2_PACKAGE_GETTEXT),yy)
>> +IPUTILS_MAKE_OPTS += ADDLIB='-lintl'
>> +endif
> 
> Why do you need both BR2_NEEDS_GETTEXT=y and BR2_PACKAGE_GETTEXT=y ?

 Historical accident: I wrote this patch before patch 1/2. Without patch 1:

 BR2_NEEDS_GETTEXT means: if a package needs intl, it needs to select gettext.

 libgpg-error, however, only links with libintl if it is available. So it
doesn't really _need_ gettext. Only if the gettext package is actually selected,
it will link with it. In other words, only if BR2_PACKAGE_GETTEXT=y, iputils
needs to link with libintl.

 In the autobuild failure, gettext was selected. Since gettext sorts
alphabetically before libgpg-error, it was already built when libgpg-error was
built, so libgpg-error links with it, so iputils needs libintl.

 Then I wrote patch 1/2 to fix that dependency. In that patch, I decided to use
BR2_NEEDS_GETTEXT to always force libgpg-error to link with libintl, because
that is what is done by all other packages. As you write in your other reply,
that's wrong though: a user who doesn't care about i18n would build libintl even
though it isn't needed in this case. I just followed the pattern that is used
everywhere else.

 Oh, and BR2_NEEDS_GETTEXT is still needed as well, because the gettext package
can also be built for glibc and musl, but in that case gpg-error doesn't link
with it (because the functionality is already available in libc).


 TBH, the more I think about it, the more I am convinced that we need a more
fundamental solution:

* Make gettext/iconv part of the toolchain; remove any explicit dependency on
gettext.

* Add a Config.in option to the toolchain (e.g. BR2_USE_LIBINTL) to enable i18n.
This option just selects the gettext package. That's just to make it easier to
the user, otherwise they'd have to go and select gettext in the target packages
menus.

* If a package really needs libintl, it will also select BR2_USE_LIBINTL if
BR2_NEEDS_GETTEXT(_IF_LOCALE).

* Perhaps also make BR2_PACKAGE_GETTEXT a blind option, is there any use for it
in glibc/musl?

* Add -lintl to link if BR2_USE_LIBINTL. Perhaps add a make variable:
ifeq ($(BR2_USE_LIBINTL),y)
TARGET_LIBINTL_IF_NEEDED = -lintl
endif

 If BR2_PACKAGE_GETTEXT is not a blind option, then there is still a possibility
that it is selected without BR2_USE_LIBINTL, so it becomes more complicated :-)

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Arnout Vandecappelle]

On 25-08-16 10:56, Arnout Vandecappelle wrote:
> * Add -lintl to link if BR2_USE_LIBINTL. Perhaps add a make variable:
> ifeq ($(BR2_USE_LIBINTL),y)

 Oh, and as per Baruch's suggestion, this could be

ifeq ($(BR2_USE_LIBINTL)$(BR2_STATIC_LIBS),yy)


 Regards,
 Arnout

> TARGET_LIBINTL_IF_NEEDED = -lintl
> endif

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Thomas Petazzoni]

Hello,

On Thu, 25 Aug 2016 10:56:12 +0200, Arnout Vandecappelle wrote:

> >> +ifeq ($(BR2_NEEDS_GETTEXT)$(BR2_PACKAGE_GETTEXT),yy)
> >> +IPUTILS_MAKE_OPTS += ADDLIB='-lintl'
> >> +endif  
> > 
> > Why do you need both BR2_NEEDS_GETTEXT=y and BR2_PACKAGE_GETTEXT=y ?  
> 
>  Historical accident: I wrote this patch before patch 1/2. Without patch 1:
> 
>  BR2_NEEDS_GETTEXT means: if a package needs intl, it needs to select gettext.

Correct.

>  libgpg-error, however, only links with libintl if it is available. So it
> doesn't really _need_ gettext. Only if the gettext package is actually selected,
> it will link with it. In other words, only if BR2_PACKAGE_GETTEXT=y, iputils
> needs to link with libintl.

In this case, as I said in my e-mail, the only thing you should do is:

ifeq ($(BR2_PACKAGE_GETTEXT),y)
<pkg>_DEPENDENCIES += gettext
endif

and not do *anything* in the Config.in.

>  In the autobuild failure, gettext was selected. Since gettext sorts
> alphabetically before libgpg-error, it was already built when libgpg-error was
> built, so libgpg-error links with it, so iputils needs libintl.
> 
>  Then I wrote patch 1/2 to fix that dependency. In that patch, I decided to use
> BR2_NEEDS_GETTEXT to always force libgpg-error to link with libintl, because
> that is what is done by all other packages. As you write in your other reply,
> that's wrong though: a user who doesn't care about i18n would build libintl even
> though it isn't needed in this case. I just followed the pattern that is used
> everywhere else.

Not correct. There are several packages that optionally depend on
gettext. For example, look at coreutils, it does exactly what I saying
above: it depends on gettext if BR2_PACKAGE_GETTEXT=y and that's it.

Same for gdbm, gptfdisk, libconfuse, popt, etc, etc.

So no, you've not followed the pattern used everywhere else :-)

>  Oh, and BR2_NEEDS_GETTEXT is still needed as well, because the gettext package
> can also be built for glibc and musl, but in that case gpg-error doesn't link
> with it (because the functionality is already available in libc).

Just use the optional dependency I mentioned above. When the gettext
package is built with a musl or glibc toolchain, it doesn't build and
install a libintl library, since the C library already provides the
functionality. So adding gettext as an optional dependency is just fine.

>  TBH, the more I think about it, the more I am convinced that we need a more
> fundamental solution:

Maybe, yes.

> * Make gettext/iconv part of the toolchain; remove any explicit dependency on
> gettext.
> 
> * Add a Config.in option to the toolchain (e.g. BR2_USE_LIBINTL) to enable i18n.
> This option just selects the gettext package. That's just to make it easier to
> the user, otherwise they'd have to go and select gettext in the target packages
> menus.
> 
> * If a package really needs libintl, it will also select BR2_USE_LIBINTL if
> BR2_NEEDS_GETTEXT(_IF_LOCALE).
> 
> * Perhaps also make BR2_PACKAGE_GETTEXT a blind option, is there any use for it
> in glibc/musl?

There is: when you need gettext programs on the target, which is the
case of the ecryptfs-utils package.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Arnout Vandecappelle]

On 25-08-16 15:07, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 25 Aug 2016 10:56:12 +0200, Arnout Vandecappelle wrote:
[snip]
>>  Then I wrote patch 1/2 to fix that dependency. In that patch, I decided to use
>> BR2_NEEDS_GETTEXT to always force libgpg-error to link with libintl, because
>> that is what is done by all other packages. As you write in your other reply,
>> that's wrong though: a user who doesn't care about i18n would build libintl even
>> though it isn't needed in this case. I just followed the pattern that is used
>> everywhere else.
> 
> Not correct. There are several packages that optionally depend on
> gettext. For example, look at coreutils, it does exactly what I saying
> above: it depends on gettext if BR2_PACKAGE_GETTEXT=y and that's it.
> 
> Same for gdbm, gptfdisk, libconfuse, popt, etc, etc.
> 
> So no, you've not followed the pattern used everywhere else :-)

 Ahem. Looking back in my console logs: I grepped for BR2_NEEDS_GETTEXT and I
found the pattern I mentioned. Self-fulfilling prophecy :-)


>>  Oh, and BR2_NEEDS_GETTEXT is still needed as well, because the gettext package
>> can also be built for glibc and musl, but in that case gpg-error doesn't link
>> with it (because the functionality is already available in libc).
> 
> Just use the optional dependency I mentioned above. When the gettext
> package is built with a musl or glibc toolchain, it doesn't build and
> install a libintl library, since the C library already provides the
> functionality. So adding gettext as an optional dependency is just fine.

 I mean it is needed (or rather, makes sense) for iputils. If
BR2_PACKAGE_GETTEXT && !BR2_NEEDS_GETTEXT, then libgpg-error doesn't link with
libintl, so iputils doesn't need -lintl. And since libintl doesn't even exist,
adding -lintl will give an error. No?


> 
>>  TBH, the more I think about it, the more I am convinced that we need a more
>> fundamental solution:
> 
> Maybe, yes.

 But that's for post-2016.08 for sure :-)


 Regards,
 Arnout

> 
>> * Make gettext/iconv part of the toolchain; remove any explicit dependency on
>> gettext.
>>
>> * Add a Config.in option to the toolchain (e.g. BR2_USE_LIBINTL) to enable i18n.
>> This option just selects the gettext package. That's just to make it easier to
>> the user, otherwise they'd have to go and select gettext in the target packages
>> menus.
>>
>> * If a package really needs libintl, it will also select BR2_USE_LIBINTL if
>> BR2_NEEDS_GETTEXT(_IF_LOCALE).
>>
>> * Perhaps also make BR2_PACKAGE_GETTEXT a blind option, is there any use for it
>> in glibc/musl?
> 
> There is: when you need gettext programs on the target, which is the
> case of the ecryptfs-utils package.
> 
> Thomas
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Thomas Petazzoni]

Hello,

On Thu, 25 Aug 2016 15:17:37 +0200, Arnout Vandecappelle wrote:

> > Not correct. There are several packages that optionally depend on
> > gettext. For example, look at coreutils, it does exactly what I saying
> > above: it depends on gettext if BR2_PACKAGE_GETTEXT=y and that's it.
> > 
> > Same for gdbm, gptfdisk, libconfuse, popt, etc, etc.
> > 
> > So no, you've not followed the pattern used everywhere else :-)  
> 
>  Ahem. Looking back in my console logs: I grepped for BR2_NEEDS_GETTEXT and I
> found the pattern I mentioned. Self-fulfilling prophecy :-)

Hehe :-)

> >>  Oh, and BR2_NEEDS_GETTEXT is still needed as well, because the gettext package
> >> can also be built for glibc and musl, but in that case gpg-error doesn't link
> >> with it (because the functionality is already available in libc).  
> > 
> > Just use the optional dependency I mentioned above. When the gettext
> > package is built with a musl or glibc toolchain, it doesn't build and
> > install a libintl library, since the C library already provides the
> > functionality. So adding gettext as an optional dependency is just fine.  
> 
>  I mean it is needed (or rather, makes sense) for iputils. If
> BR2_PACKAGE_GETTEXT && !BR2_NEEDS_GETTEXT, then libgpg-error doesn't link with
> libintl, so iputils doesn't need -lintl. And since libintl doesn't even exist,
> adding -lintl will give an error. No?

Hum, yes, I guess so. So, to sum up:

 * Your PATCH 1 needs to be adjusted to:

   ifeq ($(BR2_PACKAGE_GETTEXT),y)
   LIBGPG_ERROR_DEPENDENCIES += gettext
   endif

 * Your PATCH 2 needs to be adjusted to:

   # When gettext is enabled (BR2_PACKAGE_GETTEXT=y), and provides
   # libintl (BR2_NEEDS_GETTEXT=y), we need to link with libintl
   # explicitly when static linking, since our dependencies might use it.
   ifeq ($(BR2_NEEDS_GETTEXT)$(BR2_PACKAGE_GETTEXT)$(BR2_STATIC_LIBS),yyy)
   IPUTILS_MAKE_OPTS += ADDLIB='-lintl'
   endif

Is this correct?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] package/iputils: link with -lintl

[Arnout Vandecappelle]

On 25-08-16 15:37, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 25 Aug 2016 15:17:37 +0200, Arnout Vandecappelle wrote:
[snip]
>>>>  Oh, and BR2_NEEDS_GETTEXT is still needed as well, because the gettext package
>>>> can also be built for glibc and musl, but in that case gpg-error doesn't link
>>>> with it (because the functionality is already available in libc).  
>>>
>>> Just use the optional dependency I mentioned above. When the gettext
>>> package is built with a musl or glibc toolchain, it doesn't build and
>>> install a libintl library, since the C library already provides the
>>> functionality. So adding gettext as an optional dependency is just fine.  
>>
>>  I mean it is needed (or rather, makes sense) for iputils. If
>> BR2_PACKAGE_GETTEXT && !BR2_NEEDS_GETTEXT, then libgpg-error doesn't link with
>> libintl, so iputils doesn't need -lintl. And since libintl doesn't even exist,
>> adding -lintl will give an error. No?
> 
> Hum, yes, I guess so. So, to sum up:
> 
>  * Your PATCH 1 needs to be adjusted to:
> 
>    ifeq ($(BR2_PACKAGE_GETTEXT),y)
>    LIBGPG_ERROR_DEPENDENCIES += gettext
>    endif
> 
>  * Your PATCH 2 needs to be adjusted to:
> 
>    # When gettext is enabled (BR2_PACKAGE_GETTEXT=y), and provides
>    # libintl (BR2_NEEDS_GETTEXT=y), we need to link with libintl
>    # explicitly when static linking, since our dependencies might use it.

 Small improvement:

# When gettext is enabled (BR2_PACKAGE_GETTEXT=y), and provides
# libintl (BR2_NEEDS_GETTEXT=y), libgpg-error will link with libintl,
# and libgpg-error is pulled in by libgcrypt. Since iputils doesn't use
# libtool, we have to link with libintl explicitly for static linking,

 And the whole thing should go under the libgcrypt condition.

 I'll fix up, test and resubmit this evening.

 Regards,
 Arnout

>    ifeq ($(BR2_NEEDS_GETTEXT)$(BR2_PACKAGE_GETTEXT)$(BR2_STATIC_LIBS),yyy)
>    IPUTILS_MAKE_OPTS += ADDLIB='-lintl'
>    endif
> 
> Is this correct?
> 
> Thomas
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/2] package/libgpg-error: optionally depends on gettext

[Thomas Petazzoni]

Hello,

On Thu, 25 Aug 2016 01:10:02 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:

> +ifeq ($(BR2_NEEDS_GETTEXT),y)
> +LIBGPG_ERROR_DEPENDENCIES += gettext
> +endif

If you add gettext as a dependency, then the Config.in should be adjust
accordingly, otherwise the mechanism that checks when a package gets
built that it is really selected in the configuration will abort the
build.

The manual at
https://buildroot.org/downloads/manual/manual.html#_gettext_integration_and_interaction_with_packages
has some details on the gettext interaction:

"""

Packages that need gettext only when locale support is enabled should:

 - use select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT_IF_LOCALE in
   the Config.in file;

 - use $(if $(BR2_NEEDS_GETTEXT_IF_LOCALE),gettext) in the package
   DEPENDENCIES variable in the .mk file. 

Packages that unconditionally need gettext (which should be very rare)
should:

 - use select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT in the
   Config.in file;

 - use $(if $(BR2_NEEDS_GETTEXT),gettext) in the package
   DEPENDENCIES variable in the .mk file.

"""

Then I believe there's possibly a *third* case, which the manual
doesn't describe, but that we should document: packages that *may* use
gettext if available, but which can work without it. In this case:

ifeq ($(BR2_PACKAGE_GETTEXT),y)
<pkg>_DEPENDENCIES += gettext
endif

would be the correct solution.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com