[Buildroot] [PATCH 2/3] gettext: Rename BR2_PACKAGE_LIBINTL to BR2_PACKAGE_GETTEXT_NEEDS_BINARIES
Samuel Martin
s.martin49 at gmail.com
Fri May 11 22:45:40 UTC 2012
Hi Arnout,
Thx for the review.
2012/5/11 Arnout Vandecappelle <arnout at mind.be>:
> On 05/08/12 13:19, Samuel Martin wrote:
>>
>> libintl target does nothing but removing some binaries installed by
>> gettext.
>>
>> This patch renames the BR2_PACKAGE_LIBINTL symbol to
>> BR2_PACKAGE_GETTEXT_NEEDS_BINARIES and fixes the logic for packages
>> that may need those gettext binaries.
>
>
> I would call it BR2_PACKAGE_GETTEXT_BINARIES instead, or perhaps
> BR_PACKAGE_GETTEXT_INSTALL_BINARIES.
ok, why not.
>
> Also, the logic is inverted: _LIBINTL would remove the binaries, but
> now you don't remove the binaries if _GETTEXT_BINARIES is defined. So
> this should be mentioned in the commit log.
Next time, I will pay more attention to my commit messages.
>
>> It also adds a warning about libintl support depending on locale enabling
>> in the toolchain.
>>
>> Signed-off-by: Samuel Martin<s.martin49 at gmail.com>
>> ---
>> package/gettext/Config.in | 16 ++++++++--------
>> package/gettext/gettext.mk | 2 +-
>> package/libidn/Config.in | 1 +
>> package/php/Config.ext | 1 +
>> 4 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/package/gettext/Config.in b/package/gettext/Config.in
>> index 0ee4065..461652f 100644
>> --- a/package/gettext/Config.in
>> +++ b/package/gettext/Config.in
>> @@ -12,12 +12,12 @@ config BR2_PACKAGE_GETTEXT
>> comment "gettext requires a toolchain with WCHAR support"
>> depends on BR2_NEEDS_GETTEXT&& !BR2_USE_WCHAR
>>
>>
>> -config BR2_PACKAGE_LIBINTL
>> - bool "libintl"
>> - depends on BR2_NEEDS_GETTEXT
>> - depends on BR2_USE_WCHAR
>> - help
>> - Selecting this package installs all of gettext in the staging
>> - directory and the shared library for it's use in the target.
>> +comment "libintl requires a toolchain with locale/i18n support"
>> + depends on BR2_PACKAGE_GETTEXT&& !BR2_ENABLE_LOCALE
>
>
> You just removed libintl, so you shouldn't add a comment about it.
Because now libintl comes with gettext, there is no explicit target
for libintl anymore.
I add the comment about libintl because the build of packages
depending on libintl failed
if I enabled gettext, but disable locale/i18n support.
> I'm also not sure where this comes from. If gettext/libintl requires
> ENABLE_LOCALE, why did it work before? Or didn't it work before, and
> should there in fact be a 'depends on BR2_ENABLE_LOCALE' in the gettext
> package (and everything that selects it)?
When I tested these patches, with similar configs, the unique diff was
BR2_ENABLE_LOCALE enabled or not,
I got both libgettextlib and libintl installed in the staging dir.
when I enabled BR2_ENABLE_LOCALE,
whereas there was only libgettextlib installed if BR2_ENABLE_LOCALE
was disabled.
I cannot explain me either, hope there is some gettext guru around here...
>
>
>>
>> - http://www.gnu.org/software/gettext/
>> +config BR2_PACKAGE_GETTEXT_NEEDS_BINARIES
>> + bool
>> + depends on BR2_PACKAGE_GETTEXT
>> + help
>> + Force to install gettext binaries (gettext, gettext.sh and
>> gettextize)
>> + in the target.
>
>
> I think the "Force to install" is a bit strong here. Just say
> "Install gettext binaries..."
maybe...ok.
>
>
>> diff --git a/package/gettext/gettext.mk b/package/gettext/gettext.mk
>> index 7c7b26c..0902edd 100644
>> --- a/package/gettext/gettext.mk
>> +++ b/package/gettext/gettext.mk
>> @@ -17,7 +17,7 @@ define GETTEXT_REMOVE_BINARIES
>> rm -f $(TARGET_DIR)/usr/bin/gettextize
>> endef
>>
>> -ifeq ($(BR2_PACKAGE_LIBINTL),y)
>> +ifneq ($(BR2_PACKAGE_GETTEXT_NEEDS_BINARIES),y)
>> GETTEXT_POST_INSTALL_TARGET_HOOKS += GETTEXT_REMOVE_BINARIES
>> endif
>>
>> diff --git a/package/libidn/Config.in b/package/libidn/Config.in
>> index dcf9724..3e98245 100644
>> --- a/package/libidn/Config.in
>> +++ b/package/libidn/Config.in
>> @@ -1,6 +1,7 @@
>> config BR2_PACKAGE_LIBIDN
>> bool "libidn"
>> select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT_IF_LOCALE
>> + select BR2_PACKAGE_GETTEXT_NEEDS_BINARIES if
>> BR2_NEEDS_GETTEXT_IF_LOCALE
>
>
> Why does libidn need the gettext binaries? If this is just a mechanical
> update of a dependency that was in fact incorrect, why is it just for
> libidn and not for the 20-ish other packages that select
> BR2_PACKAGE_GETTEXT?
>
> In fact for this one I had a quick check in the source code, and I can't
> find a reference to the gettext binaries. So I doubt it is needed.
>
Actually, I followed some basic logic:
Before this patch series:
"a package depending on: gettext && libintl, did not need gettext binaries";
this can also be rephrase:
"a package depending on: gettext && !libintl, may need gettext binaries".
After apply this patch, because I invert (and rename) the libintl
logic, these sentences become:
"a package depending on: gettext && !need_gettext_binaries, did not
need gettext binaries";
or:
"a package depending on: gettext && need_gettext_binaries, (may) need
gettext binaries".
I was just enforcing the eventuality that a package may need gettext binaries.
Hope this makes things clearer.
>
>
>> help
>> Libidn's purpose is to encode and decode internationalized
>> domain names.
>> diff --git a/package/php/Config.ext b/package/php/Config.ext
>> index bd630ee..2ea686c 100644
>> --- a/package/php/Config.ext
>> +++ b/package/php/Config.ext
>> @@ -68,6 +68,7 @@ config BR2_PACKAGE_PHP_EXT_FTP
>> config BR2_PACKAGE_PHP_EXT_GETTEXT
>> bool "gettext"
>> select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT
>> + select BR2_PACKAGE_GETTEXT_NEEDS_BINARIES if BR2_NEEDS_GETTEXT
>
>
> Same here. I'm not altogether sure that the gettext PHP extension
> doesn't call the binary, but at first sight it looks like it's just
> using the library function.
If these packages don't need gettext binaries, I'm fine with removing
these select lines.
>
> Regards,
> Arnout
>
>
>> depends on BR2_USE_WCHAR
>> help
>> gettext support
>
>
> --
> Arnout Vandecappelle arnout at mind be
> Senior Embedded Software Architect +32-16-286540
> 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: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
Cheers,
Sam
More information about the buildroot
mailing list