[Buildroot] Patchwork cleanup, 10 patches to look at

[Buildroot] Patchwork cleanup, 10 patches to look at

[Thomas Petazzoni]

Hello,

I'd like to again restart the patchwork cleanup effort. There are
numerous patches in patchwork that I'm not sure what we want to do
with.

Here is a list of 10 patches that we need to take a decision on. If no
decision is taken in the next 2 weeks, I'll mark them as Rejected.

 * avahi: link with libintl if libglib2 is enabled
   https://patchwork.ozlabs.org/patch/683732/

   My plan is to enable the stub libintl in uClibc-ng and stop having
   all those linking issues with libintl from gettext. The only
   drawback would be that there would no longer be "translation"
   support with uClibc: the libintl implementation in uClibc-ng is
   really just a stub.

   Thoughts ?

 * qemu: allow to build statically
   https://patchwork.ozlabs.org/patch/692667/

   We normally don't want to have special options for each package to
   build them statically. Should we make an exception for Qemu?

 * infra: fix 'packages-file-list.txt' with TLP
   https://patchwork.ozlabs.org/patch/694519/

   Adds a lock around the installation step of packages, to make
   top-level parallel build still generate a correct list of files per
   package.

   Gustavo, what is your plan regarding this?

   Others: should we merge this?

 * e2fsprogs: keep util-linux's fsck if chosen
   https://patchwork.ozlabs.org/patch/696634/

   This fixes a real bug, but even Maxime who submitted the patch is
   not too happy with the proposal. Could someone look into this?

 * linux-headers: Account for LINUX_OVERRIDE_SRCDIR
   https://patchwork.ozlabs.org/patch/713927/

   I think I am against this patch, because if the user sets
   LINUX_OVERRIDE_SRCDIR, then suddenly he will see his kernel headers
   being rebuilt as well.

   Arnout ?

 * python-lxml: allow build as host package
   https://patchwork.ozlabs.org/patch/722644/

   Host package not used anywhere. Discussed at length during the
   previous BR meeting. What do we do?

 * fakedate: simplify logic
   https://patchwork.ozlabs.org/patch/725396/

 * linux-tools/perf: fix build for MIPS by using the right emulation on
   LD
   https://patchwork.ozlabs.org/patch/729154/

 * package/pkg-download.mk: add gitlab helper
   https://patchwork.ozlabs.org/patch/736382/

 * linux: Add CIP SLTS easy selection option
   https://patchwork.ozlabs.org/patch/736383/

Please help taking decisions about those patches.

Thanks!

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

[Buildroot] Patchwork cleanup, 10 patches to look at

[Arnout Vandecappelle]

On 26-03-17 23:53, Thomas Petazzoni wrote:
> Hello,
> 
> I'd like to again restart the patchwork cleanup effort. There are
> numerous patches in patchwork that I'm not sure what we want to do
> with.

 Great initiative!

> 
> Here is a list of 10 patches that we need to take a decision on. If no
> decision is taken in the next 2 weeks, I'll mark them as Rejected.
> 
>  * avahi: link with libintl if libglib2 is enabled
>    https://patchwork.ozlabs.org/patch/683732/
> 
>    My plan is to enable the stub libintl in uClibc-ng and stop having
>    all those linking issues with libintl from gettext. The only
>    drawback would be that there would no longer be "translation"
>    support with uClibc: the libintl implementation in uClibc-ng is
>    really just a stub.
> 
>    Thoughts ?

 I think having internationalization support can still be useful in a
Buildroot/uClibc based system, e.g. to support translation of a custom CLI UI.
So using the stub implementation unconditionally sounds like a bad idea.

 How about instead making gettext/intl support depend on !static? That's also a
catch-all solution, but a lot less aggressive.


>  * qemu: allow to build statically
>    https://patchwork.ozlabs.org/patch/692667/
> 
>    We normally don't want to have special options for each package to
>    build them statically. Should we make an exception for Qemu?

 I see something like this like having package options that do little more than
selecting other packages, like BR2_PACKAGE_PHP_EXT_BZIP2. It's something we want
to avoid, but sometimes there are good reasons to do it.

 So for Qemu and also for Docker I do think it makes sense to have static as a
per-package option.


>  * infra: fix 'packages-file-list.txt' with TLP
>    https://patchwork.ozlabs.org/patch/694519/
> 
>    Adds a lock around the installation step of packages, to make
>    top-level parallel build still generate a correct list of files per
>    package.
> 
>    Gustavo, what is your plan regarding this?
> 
>    Others: should we merge this?

 I don't think this is the proper approach. The proper approach is map/reduce,
i.e. generate the pieces in per-package files and collect them together in a
final gathering stage. Obviously, this requires a per-package target first, but
that's anyway the way we want to go.

 I thought someone already made that comment but I can't find it in that thread...

>  * e2fsprogs: keep util-linux's fsck if chosen
>    https://patchwork.ozlabs.org/patch/696634/
> 
>    This fixes a real bug, but even Maxime who submitted the patch is
>    not too happy with the proposal. Could someone look into this?
> 
>  * linux-headers: Account for LINUX_OVERRIDE_SRCDIR
>    https://patchwork.ozlabs.org/patch/713927/
> 
>    I think I am against this patch, because if the user sets
>    LINUX_OVERRIDE_SRCDIR, then suddenly he will see his kernel headers
>    being rebuilt as well.
> 
>    Arnout ?

 Well, as you can see in the discussion thread, I argued against it, asked for a
stronger argument for it, but none was forthcoming.

 However, my conclusion was that we should either apply this, or fix the
documentation so that it explains that HEADERS_SAME_AS_KERNEL doesn't apply if
LINUX_OVERRIDE_SRCDIR (or LINUX_SITE_METHOD = local) is set.


>  * python-lxml: allow build as host package
>    https://patchwork.ozlabs.org/patch/722644/
> 
>    Host package not used anywhere. Discussed at length during the
>    previous BR meeting. What do we do?

 The conclusion at BR meeting was unfortunately not clear, but we definitely
said that we would be more lenient towards this kind of addition. So I'd say we
accept it (with a Config.in.host entry, though).


>  * fakedate: simplify logic
>    https://patchwork.ozlabs.org/patch/725396/
> 
>  * linux-tools/perf: fix build for MIPS by using the right emulation on
>    LD
>    https://patchwork.ozlabs.org/patch/729154/
> 
>  * package/pkg-download.mk: add gitlab helper
>    https://patchwork.ozlabs.org/patch/736382/

 I'm not a fan of adding helpers (in fact, I think the github helper should
die). For gitlab, however, the URL construction is indeed a bit peculiar and it
differs a lot from the URL a user normally sees (adding that &filename= thing),
so it does make sense to have it. Since I couldn't decide, I didn't reply :-)


 Regards,
 Arnout

>  * linux: Add CIP SLTS easy selection option
>    https://patchwork.ozlabs.org/patch/736383/
> 
> Please help taking decisions about those patches.
> 
> Thanks!
> 
> 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] Patchwork cleanup, 10 patches to look at

[Andrey Smirnov]

On Mon, Mar 27, 2017 at 5:35 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
> On 26-03-17 23:53, Thomas Petazzoni wrote:
>> Hello,
>>
>> I'd like to again restart the patchwork cleanup effort. There are
>> numerous patches in patchwork that I'm not sure what we want to do
>> with.
>
>  Great initiative!
>
>>
>> Here is a list of 10 patches that we need to take a decision on. If no
>> decision is taken in the next 2 weeks, I'll mark them as Rejected.
>>
>>  * avahi: link with libintl if libglib2 is enabled
>>    https://patchwork.ozlabs.org/patch/683732/
>>
>>    My plan is to enable the stub libintl in uClibc-ng and stop having
>>    all those linking issues with libintl from gettext. The only
>>    drawback would be that there would no longer be "translation"
>>    support with uClibc: the libintl implementation in uClibc-ng is
>>    really just a stub.
>>
>>    Thoughts ?
>
>  I think having internationalization support can still be useful in a
> Buildroot/uClibc based system, e.g. to support translation of a custom CLI UI.
> So using the stub implementation unconditionally sounds like a bad idea.
>
>  How about instead making gettext/intl support depend on !static? That's also a
> catch-all solution, but a lot less aggressive.
>
>
>>  * qemu: allow to build statically
>>    https://patchwork.ozlabs.org/patch/692667/
>>
>>    We normally don't want to have special options for each package to
>>    build them statically. Should we make an exception for Qemu?
>
>  I see something like this like having package options that do little more than
> selecting other packages, like BR2_PACKAGE_PHP_EXT_BZIP2. It's something we want
> to avoid, but sometimes there are good reasons to do it.
>
>  So for Qemu and also for Docker I do think it makes sense to have static as a
> per-package option.
>
>
>>  * infra: fix 'packages-file-list.txt' with TLP
>>    https://patchwork.ozlabs.org/patch/694519/
>>
>>    Adds a lock around the installation step of packages, to make
>>    top-level parallel build still generate a correct list of files per
>>    package.
>>
>>    Gustavo, what is your plan regarding this?
>>
>>    Others: should we merge this?
>
>  I don't think this is the proper approach. The proper approach is map/reduce,
> i.e. generate the pieces in per-package files and collect them together in a
> final gathering stage. Obviously, this requires a per-package target first, but
> that's anyway the way we want to go.
>
>  I thought someone already made that comment but I can't find it in that thread...
>
>>  * e2fsprogs: keep util-linux's fsck if chosen
>>    https://patchwork.ozlabs.org/patch/696634/
>>
>>    This fixes a real bug, but even Maxime who submitted the patch is
>>    not too happy with the proposal. Could someone look into this?
>>
>>  * linux-headers: Account for LINUX_OVERRIDE_SRCDIR
>>    https://patchwork.ozlabs.org/patch/713927/
>>
>>    I think I am against this patch, because if the user sets
>>    LINUX_OVERRIDE_SRCDIR, then suddenly he will see his kernel headers
>>    being rebuilt as well.
>>
>>    Arnout ?
>
>  Well, as you can see in the discussion thread, I argued against it, asked for a
> stronger argument for it, but none was forthcoming.

The patch in question is trivial, so asking for stronger argument for
it is as good as saying "no". I don't have any plans on working on
that patch further, and given the state of discussion, IMHO, marking
it as "Rejected" would be the right thing to do.

Thanks,
Andrey Smirnov

[Buildroot] Patchwork cleanup, 10 patches to look at

[Thomas Petazzoni]

Hello,

On Mon, 27 Mar 2017 14:35:46 +0200, Arnout Vandecappelle wrote:

> >  * avahi: link with libintl if libglib2 is enabled
> >    https://patchwork.ozlabs.org/patch/683732/
> > 
> >    My plan is to enable the stub libintl in uClibc-ng and stop having
> >    all those linking issues with libintl from gettext. The only
> >    drawback would be that there would no longer be "translation"
> >    support with uClibc: the libintl implementation in uClibc-ng is
> >    really just a stub.
> > 
> >    Thoughts ?  
> 
>  I think having internationalization support can still be useful in a
> Buildroot/uClibc based system, e.g. to support translation of a custom CLI UI.
> So using the stub implementation unconditionally sounds like a bad idea.
> 
>  How about instead making gettext/intl support depend on !static? That's also a
> catch-all solution, but a lot less aggressive.

Indeed, that's a solution. This solves the static linking issues, but
by getting rid of libintl from gettext entirely, I was hoping to remove
all the BR2_NEEDS_GETTEXT/BR2_NEEDS_GETTEXT_IF_LOCALE stuff.

But yes, I can understand that i18n support can be useful in a uClibc
based system.

However, the !static dependency then needs to be propagated to all
places where libintl is used, but only if uClibc is used. It's a bit of
a nightmare :-/

> >  * qemu: allow to build statically
> >    https://patchwork.ozlabs.org/patch/692667/
> > 
> >    We normally don't want to have special options for each package to
> >    build them statically. Should we make an exception for Qemu?  
> 
>  I see something like this like having package options that do little more than
> selecting other packages, like BR2_PACKAGE_PHP_EXT_BZIP2. It's something we want
> to avoid, but sometimes there are good reasons to do it.
> 
>  So for Qemu and also for Docker I do think it makes sense to have static as a
> per-package option.

ACK, so I'll apply the patch from J?r?me.

> >  * infra: fix 'packages-file-list.txt' with TLP
> >    https://patchwork.ozlabs.org/patch/694519/
> > 
> >    Adds a lock around the installation step of packages, to make
> >    top-level parallel build still generate a correct list of files per
> >    package.
> > 
> >    Gustavo, what is your plan regarding this?
> > 
> >    Others: should we merge this?  
> 
>  I don't think this is the proper approach. The proper approach is map/reduce,
> i.e. generate the pieces in per-package files and collect them together in a
> final gathering stage. Obviously, this requires a per-package target first, but
> that's anyway the way we want to go.
> 
>  I thought someone already made that comment but I can't find it in that thread...

OK, so I'll mark the patch as Rejected.

> >  * linux-headers: Account for LINUX_OVERRIDE_SRCDIR
> >    https://patchwork.ozlabs.org/patch/713927/
> > 
> >    I think I am against this patch, because if the user sets
> >    LINUX_OVERRIDE_SRCDIR, then suddenly he will see his kernel headers
> >    being rebuilt as well.
> > 
> >    Arnout ?  
> 
>  Well, as you can see in the discussion thread, I argued against it, asked for a
> stronger argument for it, but none was forthcoming.
> 
>  However, my conclusion was that we should either apply this, or fix the
> documentation so that it explains that HEADERS_SAME_AS_KERNEL doesn't apply if
> LINUX_OVERRIDE_SRCDIR (or LINUX_SITE_METHOD = local) is set.

I'm also not a big fan of this patch. I prefer if LINUX_OVERRIDE_SRCDIR
really only affects the linux package, to me it's just applying the
principle of least surprise.

> >  * python-lxml: allow build as host package
> >    https://patchwork.ozlabs.org/patch/722644/
> > 
> >    Host package not used anywhere. Discussed at length during the
> >    previous BR meeting. What do we do?  
> 
>  The conclusion at BR meeting was unfortunately not clear, but we definitely
> said that we would be more lenient towards this kind of addition. So I'd say we
> accept it (with a Config.in.host entry, though).

ACK.

> >  * package/pkg-download.mk: add gitlab helper
> >    https://patchwork.ozlabs.org/patch/736382/  
> 
>  I'm not a fan of adding helpers (in fact, I think the github helper should
> die). For gitlab, however, the URL construction is indeed a bit peculiar and it
> differs a lot from the URL a user normally sees (adding that &filename= thing),
> so it does make sense to have it. Since I couldn't decide, I didn't reply :-)

Nothing in Buildroot currently fetches its source code from Gitlab.
This change was proposed for the CIP long-term kernel version, but
according to
https://wiki.linuxfoundation.org/civilinfrastructureplatform/start:

"""Check the 4.4 CIP kernel repository, mirrored in Gitlab"""

So in fact their primary kernel repository is
https://git.kernel.org/pub/scm/linux/kernel/git/bwh/linux-cip.git/, not
the one on gitlab. And additionally, their gitlab mirror is apparently
not working as expected:
https://lists.cip-project.org/pipermail/cip-dev/2017-March/000167.html.
So I guess we should use the git.kernel.org repository instead.

And I still think the BR2_LINUX_KERNEL_LATEST_CIP_VERSION_BRANCH option
is not good, as it uses the "latest" version available, which makes the
build non-reproducible. I argued about this with Angelo, but I was a
bit alone against Angelo on the matter. Could some other folks give
their opinion so that we can resolve the debate?

Thanks!

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

[Buildroot] Patchwork cleanup, 10 patches to look at

[Thomas Petazzoni]

Hello,

Quick update on where we stand.

On Sun, 26 Mar 2017 23:53:20 +0200, Thomas Petazzoni wrote:

>  * avahi: link with libintl if libglib2 is enabled
>    https://patchwork.ozlabs.org/patch/683732/
> 
>    My plan is to enable the stub libintl in uClibc-ng and stop having
>    all those linking issues with libintl from gettext. The only
>    drawback would be that there would no longer be "translation"
>    support with uClibc: the libintl implementation in uClibc-ng is
>    really just a stub.
> 
>    Thoughts ?

No real solution provided. A discussion started with Arnout, but no
conclusion at this point.

>  * qemu: allow to build statically
>    https://patchwork.ozlabs.org/patch/692667/
> 
>    We normally don't want to have special options for each package to
>    build them statically. Should we make an exception for Qemu?

Following Arnout's suggestion, this patch was accepted and merged.

>  * infra: fix 'packages-file-list.txt' with TLP
>    https://patchwork.ozlabs.org/patch/694519/
>
>    Adds a lock around the installation step of packages, to make
>    top-level parallel build still generate a correct list of files per
>    package.
> 
>    Gustavo, what is your plan regarding this?
> 
>    Others: should we merge this?

Following discussion with Arnout, this patch was rejected.

>  * e2fsprogs: keep util-linux's fsck if chosen
>    https://patchwork.ozlabs.org/patch/696634/
> 
>    This fixes a real bug, but even Maxime who submitted the patch is
>    not too happy with the proposal. Could someone look into this?

No progress.

>  * linux-headers: Account for LINUX_OVERRIDE_SRCDIR
>    https://patchwork.ozlabs.org/patch/713927/
> 
>    I think I am against this patch, because if the user sets
>    LINUX_OVERRIDE_SRCDIR, then suddenly he will see his kernel headers
>    being rebuilt as well.
> 
>    Arnout ?

Following discussion with Arnout, this patch was rejected.

>  * python-lxml: allow build as host package
>    https://patchwork.ozlabs.org/patch/722644/
> 
>    Host package not used anywhere. Discussed at length during the
>    previous BR meeting. What do we do?

Following discussion with Arnout, this patch was accepted after adding
a Config.in.host file.

>  * fakedate: simplify logic
>    https://patchwork.ozlabs.org/patch/725396/

No progress.

>  * linux-tools/perf: fix build for MIPS by using the right emulation on
>    LD
>    https://patchwork.ozlabs.org/patch/729154/

No progress.

>  * package/pkg-download.mk: add gitlab helper
>    https://patchwork.ozlabs.org/patch/736382/
> 
>  * linux: Add CIP SLTS easy selection option
>    https://patchwork.ozlabs.org/patch/736383/

Patches marked as Changes Requested, with feedback given to the
submitter.

Temporary results:

 - 6 patches on which a decision was taken, and therefore the patches
   are no longer in patchwork

 - 4 patches still pending a decision

Thanks,

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

[Buildroot] Patchwork cleanup, 10 patches to look at

[Carlos Santos]

> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: buildroot at uclibc.org, "Gustavo Zacarias" <gustavo@zacarias.com.ar>, "Arnout Vandecappelle" <arnout@mind.be>, "Johan
> Oudinet" <johan.oudinet@gmail.com>, "J?r?me Pouiller" <jezz@sysmic.org>, "Maxime Hadjinlian"
> <maxime.hadjinlian@gmail.com>, "Andrey Smirnov" <andrew.smirnov@gmail.com>, "Carlos Santos" <casantos@datacom.ind.br>,
> "Peter Korsgaard" <peter@korsgaard.com>, "Vicente Olivert Riera" <Vincent.Riera@imgtec.com>, "Angelo Compagnucci"
> <angelo.compagnucci@gmail.com>
> Sent: Saturday, April 1, 2017 1:52:04 PM
> Subject: Re: [Buildroot] Patchwork cleanup, 10 patches to look at

[...]
>>  * e2fsprogs: keep util-linux's fsck if chosen
>>    https://patchwork.ozlabs.org/patch/696634/
>> 
>>    This fixes a real bug, but even Maxime who submitted the patch is
>>    not too happy with the proposal. Could someone look into this?
> 
> No progress.

Patch submitted: http://patchwork.ozlabs.org/patch/746106/

I attempted to keep the change as small as possible, since a better
solution, following Arnout's suggestions, would require an extreme
overhauling of the recipe. I will do this in the scope of fixing bug
9436 (https://bugs.busybox.net/show_bug.cgi?id=9436)

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens

[Buildroot] libintl static linking issues - yet another spin

[Arnout Vandecappelle]

 [Following up on the libintl static linking issues which have been discussed in
several threads but no conclusion as of yet.]

On 28-03-17 22:25, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 27 Mar 2017 14:35:46 +0200, Arnout Vandecappelle wrote:
> 
>>>  * avahi: link with libintl if libglib2 is enabled
>>>    https://patchwork.ozlabs.org/patch/683732/
>>>
>>>    My plan is to enable the stub libintl in uClibc-ng and stop having
>>>    all those linking issues with libintl from gettext. The only
>>>    drawback would be that there would no longer be "translation"
>>>    support with uClibc: the libintl implementation in uClibc-ng is
>>>    really just a stub.
>>>
>>>    Thoughts ?  
>>  I think having internationalization support can still be useful in a
>> Buildroot/uClibc based system, e.g. to support translation of a custom CLI UI.
>> So using the stub implementation unconditionally sounds like a bad idea.
>>
>>  How about instead making gettext/intl support depend on !static? That's also a
>> catch-all solution, but a lot less aggressive.
> Indeed, that's a solution. This solves the static linking issues, but
> by getting rid of libintl from gettext entirely, I was hoping to remove
> all the BR2_NEEDS_GETTEXT/BR2_NEEDS_GETTEXT_IF_LOCALE stuff.
> 
> But yes, I can understand that i18n support can be useful in a uClibc
> based system.
> 
> However, the !static dependency then needs to be propagated to all
> places where libintl is used, but only if uClibc is used. It's a bit of
> a nightmare :-/


 Here's a fresh idea, extended version of the "let gettext depend on !static" idea:

- We make LOCALE support depend on !static.
- We make the gettext package depend on !static.
- We need to propage a !static dependency to the 14 packages that have select
BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT; probably should be double-checked if
they really depend on gettext.
- The dozens of packages that have select BR2_PACKAGE_GETTEXT if
BR2_NEEDS_GETTEXT_IF_LOCALE don't need to depend on !static.


 However, thinking a bit more about it: if libintl support disappears from
uClibc-ng, does that mean that the BR2_TOOLCHAIN_BUILDROOT_LOCALE is gone? If
so, then the entire BR2_NEEDS_GETTEXT_IF_LOCALE becomes redundant. Actually,
that option is currently probably incorrect, since for musl it will never be
enabled, which means that those packages may not be built with intl support when
using musl, even if the gettext package is enabled.


 So perhaps instead we should introduce a new toolchain option BR2_ENABLE_INTL
or something similar. It would depend on BR2_NEEDS_GETTEXT and on !static, it
selects the gettext package (and nothing more). We could then move convert the
gettext package into a toolchain package so it is always there, and remove all
the gettext-related dependencies from Config.in and *.mk. In terms of patches it
would be a little bit complicated, but the end result would be a whole lot simpler.


 Thoughts?

 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] libintl static linking issues - yet another spin

[Thomas Petazzoni]

Hello,

[ Not read/thought about the entire e-mail yet, but just one point ]

On Mon, 3 Apr 2017 12:42:21 +0200, Arnout Vandecappelle wrote:

>  However, thinking a bit more about it: if libintl support disappears from
> uClibc-ng

What makes you think libintl disappears from uClibc-ng? uClibc-ng has
recently *added* libintl support.

Basically, today we have the following situation:

 * glibc comes with its own libintl implementation, which is
   full-featured

 * uClibc-ng until recently did not have any libintl implementation, so
   we are building the libintl from gettext to get a libintl
   implementation. uClibc-ng has recently added a stub libintl
   implementation, which does not support translation, i.e it's really
   just a stub.

 * musl has a libintl implementation. It is not a stub, and seems to
   support translation (i.e it seems to read .mo files for
   translations, etc.).

Best regards,

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

[Buildroot] libintl static linking issues - yet another spin

[Arnout Vandecappelle]

On 03-04-17 13:01, Thomas Petazzoni wrote:
> Hello,
> 
> [ Not read/thought about the entire e-mail yet, but just one point ]
> 
> On Mon, 3 Apr 2017 12:42:21 +0200, Arnout Vandecappelle wrote:
> 
>>  However, thinking a bit more about it: if libintl support disappears from
>> uClibc-ng
> 
> What makes you think libintl disappears from uClibc-ng? uClibc-ng has
> recently *added* libintl support.

 Crap, I was confusing with RPC, due to your statement "just a stub implementation".

> 
> Basically, today we have the following situation:
> 
>  * glibc comes with its own libintl implementation, which is
>    full-featured
> 
>  * uClibc-ng until recently did not have any libintl implementation, so
>    we are building the libintl from gettext to get a libintl
>    implementation. uClibc-ng has recently added a stub libintl
>    implementation, which does not support translation, i.e it's really
>    just a stub.
> 
>  * musl has a libintl implementation. It is not a stub, and seems to
>    support translation (i.e it seems to read .mo files for
>    translations, etc.).

 Ah yes, I didn't see a mention of locales anywhere in the musl related code,
but it's just in the default of BR2_NEEDS_GETTEXT that we can see that it's only
relevant for uClibc.

 Would the libintl from uClibc make it easier to do static linking, i.e. is it
in libc or in a separate libintl library?

 Regards,
 Arnout

> 
> Best regards,
> 
> 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] libintl static linking issues - yet another spin

[Thomas Petazzoni]

Hello,

On Mon, 3 Apr 2017 13:25:11 +0200, Arnout Vandecappelle wrote:

>  Would the libintl from uClibc make it easier to do static linking, i.e. is it
> in libc or in a separate libintl library?

It is in libc I believe, since uClibc now has everything in libc. So it
does make things a *lot* easier for static linking.

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

[Buildroot] libintl static linking issues - yet another spin

[Waldemar Brodkorb]

Hi,
Thomas Petazzoni wrote,

> Hello,
> 
> On Mon, 3 Apr 2017 13:25:11 +0200, Arnout Vandecappelle wrote:
> 
> >  Would the libintl from uClibc make it easier to do static linking, i.e. is it
> > in libc or in a separate libintl library?
> 
> It is in libc I believe, since uClibc now has everything in libc. So it
> does make things a *lot* easier for static linking.

Yes, it is included in libc when activated.
It was added to allow packages which use libintl to at least
be usable in a limited way (as stub.)

It might help in some cases, but surely not in all cases.

gru?
 Waldemar

[Buildroot] libintl static linking issues - yet another spin

[Arnout Vandecappelle]

On 03-04-17 14:11, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 3 Apr 2017 13:25:11 +0200, Arnout Vandecappelle wrote:
> 
>>  Would the libintl from uClibc make it easier to do static linking, i.e. is it
>> in libc or in a separate libintl library?
> 
> It is in libc I believe, since uClibc now has everything in libc. So it
> does make things a *lot* easier for static linking.

 And I guess musl also has it in a single libc? Yeah, I don't see a libintl
there so I guess it's bundled.

 If we do enable the stubs in uClibc, and a packages links with -lintl, then in
fact the full gettext will be used and the stubs are not even linked in
(statically or dynamically), correct? In that case, we could indeed enable the
stubs in uClibc. As a result, in static linking, any package that links with
gettext but forgets an explicit -lintl will use the stubs instead of the full
implementation. So the package still works, but without translations.

 So that would be an option. We should then still accept patches that add
appropriate -lintl even though it's not a build failure, but we don't have to
fix build failures due to -lintl if nobody complains about it. It's a kind of
pragmatical, imperfect solution.


 As a side note, pretty much independent of this issue: I wonder if our
BR2_NEEDS_GETTEXT_IF_LOCALE approach really is the right thing to do. It
basically says: if a package is able to use translations, we force translations
(by enabling gettext). Wouldn't it be much more appropriate to remove all those
selects from Config.in, and instead just add to _DEPENDENCIES:
$(if $(BR2_PACKAGE_GETTEXT),gettext) ? That allows the user to choose whether or
not they want translations.


 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] libintl static linking issues - yet another spin

[Thomas Petazzoni]

Hello,

On Mon, 3 Apr 2017 14:45:12 +0200, Arnout Vandecappelle wrote:

>  And I guess musl also has it in a single libc? Yeah, I don't see a libintl
> there so I guess it's bundled.
> 
>  If we do enable the stubs in uClibc, and a packages links with -lintl, then in
> fact the full gettext will be used and the stubs are not even linked in
> (statically or dynamically), correct? In that case, we could indeed enable the
> stubs in uClibc. As a result, in static linking, any package that links with
> gettext but forgets an explicit -lintl will use the stubs instead of the full
> implementation. So the package still works, but without translations.

Does this actually work? Won't the linker complain about conflicting
symbols between libintl and libc? Are weak symbols in libc also working
in static linking scenarios?

>  As a side note, pretty much independent of this issue: I wonder if our
> BR2_NEEDS_GETTEXT_IF_LOCALE approach really is the right thing to do. It
> basically says: if a package is able to use translations, we force translations
> (by enabling gettext). Wouldn't it be much more appropriate to remove all those
> selects from Config.in, and instead just add to _DEPENDENCIES:
> $(if $(BR2_PACKAGE_GETTEXT),gettext) ? That allows the user to choose whether or
> not they want translations.

No, that's not what BR2_NEEDS_GETTEXT_IF_LOCALE says. What
BR2_NEEDS_GETTEXT_IF_LOCALE is really what it means: the package really
absolutely NEEDS gettext to be available if locale support is available
in the toolchain. I.e, with locale support enabled, the package would
*NOT* build if gettext is not available.

Of course, there might be incorrect uses of BR2_NEEDS_GETTEXT_IF_LOCALE
that don't follow this rule, and they should be fixed as you suggest,
but BR2_NEEDS_GETTEXT_IF_LOCALE remains needed for the case I explained
above. See the documentation:

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

Best regards,

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

[Buildroot] libintl static linking issues - yet another spin

[Arnout Vandecappelle]

On 03-04-17 15:02, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 3 Apr 2017 14:45:12 +0200, Arnout Vandecappelle wrote:
> 
>>  And I guess musl also has it in a single libc? Yeah, I don't see a libintl
>> there so I guess it's bundled.
>>
>>  If we do enable the stubs in uClibc, and a packages links with -lintl, then in
>> fact the full gettext will be used and the stubs are not even linked in
>> (statically or dynamically), correct? In that case, we could indeed enable the
>> stubs in uClibc. As a result, in static linking, any package that links with
>> gettext but forgets an explicit -lintl will use the stubs instead of the full
>> implementation. So the package still works, but without translations.
> 
> Does this actually work? Won't the linker complain about conflicting
> symbols between libintl and libc? Are weak symbols in libc also working
> in static linking scenarios?

 As usual, I haven't tested. However:

- For dynamic linking, it is never a problem if a symbol was already loaded from
another library. Any UNDEF symbol in the .so file will link with the
already-loaded symbol, and the defined symbols will not be loaded.

- For static linking, it is done per .o file. A .o file from a .a is only linked
in if any symbol in the file is currently undefined. So you can have duplicate
definitions if libc itself requests a symbol that is defined in its own
libintl.o, but not in the external libintl.o, because then both the internal and
the external libintl.o will be linked in.

 As far as I can see, the gettext calls within uClibc itself are all #define'd
away, so this problem shouldn't occur. And anyway, the external libintl also
defines all of them, so unless you link with '-lc -lintl' (which gives libc
precedence over libintl), there shouldn't be a problem.

 I repeat: this is all theoretical, I haven't tried.

> 
>>  As a side note, pretty much independent of this issue: I wonder if our
>> BR2_NEEDS_GETTEXT_IF_LOCALE approach really is the right thing to do. It
>> basically says: if a package is able to use translations, we force translations
>> (by enabling gettext). Wouldn't it be much more appropriate to remove all those
>> selects from Config.in, and instead just add to _DEPENDENCIES:
>> $(if $(BR2_PACKAGE_GETTEXT),gettext) ? That allows the user to choose whether or
>> not they want translations.
> 
> No, that's not what BR2_NEEDS_GETTEXT_IF_LOCALE says. What
> BR2_NEEDS_GETTEXT_IF_LOCALE is really what it means: the package really
> absolutely NEEDS gettext to be available if locale support is available
> in the toolchain. I.e, with locale support enabled, the package would
> *NOT* build if gettext is not available.

 So this is *only* for package that stupidly assume that if you can call iconv,
you can also call gettext? That should be extremely rare, no?

 I found at least one commit that does the wrong thing then:

commit 96e89e4a69500a99e129777439b80af7df56cb8d
Author: Romain Naour <romain.naour@openwide.fr>
Date:   Sun May 10 22:33:04 2015

    package/gnuchess: add gettext dependency

    gnuchess check for libintl library if BR2_NEEDS_GETTEXT_IF_LOCALE is set.

    checking for GNU gettext in libintl... yes
    checking whether to use NLS... yes
    checking where the gettext function comes from... external libintl
    checking how to link with libintl... -lintl

    But the dependency on gettext package is missing to ensures
    reproducible builds.


> 
> Of course, there might be incorrect uses of BR2_NEEDS_GETTEXT_IF_LOCALE
> that don't follow this rule, and they should be fixed as you suggest,
> but BR2_NEEDS_GETTEXT_IF_LOCALE remains needed for the case I explained
> above. See the documentation:
> 
> """
> 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; 
> """

 That's not clear enough in the documentation then.

 Regards,
 Arnout

> 
> Best regards,
> 
> 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