Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Arnout Vandecappelle <arnout@mind.be>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 2/3] gettext: Rename BR2_PACKAGE_LIBINTL to BR2_PACKAGE_GETTEXT_NEEDS_BINARIES
Date: Sat, 12 May 2012 16:43:48 +0200	[thread overview]
Message-ID: <4FAE7724.5010608@mind.be> (raw)
In-Reply-To: <CAHXCMM+SrrkJjjv3p1ZwyOSBX_U=GWYxcsCc577rCGqx=OZrXg@mail.gmail.com>

On 05/12/12 00:45, Samuel Martin wrote:
> Hi Arnout,
>
> Thx for the review.
>
> 2012/5/11 Arnout Vandecappelle<arnout@mind.be>:
>> On 05/08/12 13:19, Samuel Martin wrote:
[snip]
>>> +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...

  I just tried with the following defconfig and without these patches,
and I do get both libgettextlib and libintl on the target.

BR2_powerpc=y
BR2_TOOLCHAIN_BUILDROOT_WCHAR=y
BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_DEVTMPFS=y
BR2_PACKAGE_GETTEXT=y
BR2_PACKAGE_LIBINTL=y
(BR2_ENABLE_LOCALE is not set)

[snip]
>>   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.

  Indeed.  Sorry I wasn't paying attention :-)

  Although most likely these two packages don't need the gettext binaries on
the target, it's safer to keep the old behaviour.

  Regards,
  Arnout

-- 
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

  reply	other threads:[~2012-05-12 14:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-04 14:46 [Buildroot] [pull request] Pull request for branch for-2012.05/packages-updates Maxime Ripard
2012-04-04 14:46 ` [Buildroot] [PATCH 1/2] Convert gettext to autotargets Maxime Ripard
2012-04-04 14:56   ` Maxime Ripard
2012-05-08 11:19     ` [Buildroot] [PATCH 1/3] " Samuel Martin
2012-05-08 11:19       ` [Buildroot] [PATCH 2/3] gettext: Rename BR2_PACKAGE_LIBINTL to BR2_PACKAGE_GETTEXT_NEEDS_BINARIES Samuel Martin
2012-05-11 21:37         ` Arnout Vandecappelle
2012-05-11 22:45           ` Samuel Martin
2012-05-12 14:43             ` Arnout Vandecappelle [this message]
2012-05-08 11:19       ` [Buildroot] [PATCH 3/3] Cleanup gettext/libintl dependencies Samuel Martin
2012-05-19 21:46       ` [Buildroot] [PATCH 1/3] Convert gettext to autotargets Peter Korsgaard
2012-05-21  7:52         ` Samuel Martin
2012-05-21  8:02           ` Peter Korsgaard
2012-05-21  8:08             ` Will Newton
2012-05-21  8:12               ` Maxime Ripard
2012-05-21  8:12               ` Peter Korsgaard
2012-05-21  8:06         ` Maxime Ripard
2012-04-15 19:09   ` [Buildroot] [PATCH 1/1] " Samuel Martin
2012-04-04 14:46 ` [Buildroot] [PATCH 2/2] Bump systemd to version 44 Maxime Ripard
2012-04-05 11:16   ` Peter Korsgaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FAE7724.5010608@mind.be \
    --to=arnout@mind.be \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox