diff mbox

gettext: add dependency on host-libxml2 for host-gettext.

Message ID 1424696593-21361-1-git-send-email-nicolas.cavallari@green-communications.fr
State Superseded
Headers show

Commit Message

Nicolas Cavallari Feb. 23, 2015, 1:03 p.m. UTC
From: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>

Otherwise, the locally-installed libxml2 will be used, which may
depend on a locally-installed liblzma which may create conflict
if host-liblzma is compiled.

Fixes https://bugs.busybox.net/show_bug.cgi?id=7886

Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
[nicolas: added extended commit message]
Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
---
Ignacy is unavailable this week, but he commited this change to our
local tree before leaving.

Comments

Thomas Petazzoni Feb. 23, 2015, 6:17 p.m. UTC | #1
Dear Nicolas Cavallari,

On Mon, 23 Feb 2015 14:03:13 +0100, Nicolas Cavallari wrote:
> From: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> 
> Otherwise, the locally-installed libxml2 will be used, which may
> depend on a locally-installed liblzma which may create conflict
> if host-liblzma is compiled.
> 
> Fixes https://bugs.busybox.net/show_bug.cgi?id=7886
> 
> Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> [nicolas: added extended commit message]
> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> ---
> Ignacy is unavailable this week, but he commited this change to our
> local tree before leaving.
> 
> diff --git a/package/gettext/gettext.mk b/package/gettext/gettext.mk
> index c2419c1..a554517 100644
> --- a/package/gettext/gettext.mk
> +++ b/package/gettext/gettext.mk
> @@ -12,7 +12,7 @@ GETTEXT_LICENSE = GPLv2+
>  GETTEXT_LICENSE_FILES = COPYING
>  
>  GETTEXT_DEPENDENCIES = $(if $(BR2_PACKAGE_LIBICONV),libiconv)
> -HOST_GETTEXT_DEPENDENCIES = # we don't want the libiconv dependency
> +HOST_GETTEXT_DEPENDENCIES = host-libxml2 # we don't want the libiconv dependency

I agree that we normally prefer to rely on external libraries, rather
than built-in copies. However, in this case, I am wondering if we
shouldn't rather rely on the built-in copy, which is more lightweight
that building host-libxml2.

From the DEPENDENCIES file of gettext:

* libxml2
  + Optional.
    Needed for the --color option of the various programs.
    If not present, a subset of libxml2 (included in this package) will be
    compiled into libgettextlib.
  + Homepage:
    http://xmlsoft.org/
  + Download:
    ftp://xmlsoft.org/libxml2/
  + If it is installed in a nonstandard directory, pass the option
    --with-libxml2-prefix=DIR to 'configure'.

So instead of building an external libxml2, we can pass
--with-included-libxml to the gettext-tools configure script to force
it to use the built-in libxml2 subset.

Note that there are several other libraries in the same situation:
glib2, libcroco and libunistring.

What is the opinion of other BR developers about this?

And all that just for a --color command line option that we really
don't care about. Why the heck isn't this thing optional in the first place?

Best regards,

Thomas
Yann E. MORIN Feb. 24, 2015, 10:24 p.m. UTC | #2
Thomas, Nicolas, All,

On 2015-02-23 19:17 +0100, Thomas Petazzoni spake thusly:
> On Mon, 23 Feb 2015 14:03:13 +0100, Nicolas Cavallari wrote:
> > From: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> > 
> > Otherwise, the locally-installed libxml2 will be used, which may
> > depend on a locally-installed liblzma which may create conflict
> > if host-liblzma is compiled.
> > 
> > Fixes https://bugs.busybox.net/show_bug.cgi?id=7886
> > 
> > Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> > [nicolas: added extended commit message]
> > Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> > ---
> > Ignacy is unavailable this week, but he commited this change to our
> > local tree before leaving.
> > 
> > diff --git a/package/gettext/gettext.mk b/package/gettext/gettext.mk
> > index c2419c1..a554517 100644
> > --- a/package/gettext/gettext.mk
> > +++ b/package/gettext/gettext.mk
> > @@ -12,7 +12,7 @@ GETTEXT_LICENSE = GPLv2+
> >  GETTEXT_LICENSE_FILES = COPYING
> >  
> >  GETTEXT_DEPENDENCIES = $(if $(BR2_PACKAGE_LIBICONV),libiconv)
> > -HOST_GETTEXT_DEPENDENCIES = # we don't want the libiconv dependency
> > +HOST_GETTEXT_DEPENDENCIES = host-libxml2 # we don't want the libiconv dependency
> 
> I agree that we normally prefer to rely on external libraries, rather
> than built-in copies. However, in this case, I am wondering if we
> shouldn't rather rely on the built-in copy, which is more lightweight
> that building host-libxml2.
> 
> From the DEPENDENCIES file of gettext:
> 
> * libxml2
>   + Optional.
>     Needed for the --color option of the various programs.
>     If not present, a subset of libxml2 (included in this package) will be
>     compiled into libgettextlib.
>   + Homepage:
>     http://xmlsoft.org/
>   + Download:
>     ftp://xmlsoft.org/libxml2/
>   + If it is installed in a nonstandard directory, pass the option
>     --with-libxml2-prefix=DIR to 'configure'.
> 
> So instead of building an external libxml2, we can pass
> --with-included-libxml to the gettext-tools configure script to force
> it to use the built-in libxml2 subset.
> 
> Note that there are several other libraries in the same situation:
> glib2, libcroco and libunistring.
> 
> What is the opinion of other BR developers about this?

On my machine, host-libxml2 takes about 0m53.693s while host-gettext
takes about 2m34.112s. So the overhead (~20%-pf-gettext)) is not
negligible...

I am compeltely undecided on that... Do we consider a ~20%-of-gettext
overhead for a full build to be important or not?

> And all that just for a --color command line option that we really
> don't care about. Why the heck isn't this thing optional in the first place?

Eh... Upstream... ;-)

Regards,
Yann E. MORIN.
Vicente Olivert Riera Sept. 14, 2015, 3:09 p.m. UTC | #3
Dear all,

On 02/24/2015 10:24 PM, Yann E. MORIN wrote:
> Thomas, Nicolas, All,
> 
> On 2015-02-23 19:17 +0100, Thomas Petazzoni spake thusly:
>> On Mon, 23 Feb 2015 14:03:13 +0100, Nicolas Cavallari wrote:
>>> From: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
>>>
>>> Otherwise, the locally-installed libxml2 will be used, which may
>>> depend on a locally-installed liblzma which may create conflict
>>> if host-liblzma is compiled.
>>>
>>> Fixes https://bugs.busybox.net/show_bug.cgi?id=7886
>>>
>>> Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
>>> [nicolas: added extended commit message]
>>> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
>>> ---
>>> Ignacy is unavailable this week, but he commited this change to our
>>> local tree before leaving.
>>>
>>> diff --git a/package/gettext/gettext.mk b/package/gettext/gettext.mk
>>> index c2419c1..a554517 100644
>>> --- a/package/gettext/gettext.mk
>>> +++ b/package/gettext/gettext.mk
>>> @@ -12,7 +12,7 @@ GETTEXT_LICENSE = GPLv2+
>>>  GETTEXT_LICENSE_FILES = COPYING
>>>  
>>>  GETTEXT_DEPENDENCIES = $(if $(BR2_PACKAGE_LIBICONV),libiconv)
>>> -HOST_GETTEXT_DEPENDENCIES = # we don't want the libiconv dependency
>>> +HOST_GETTEXT_DEPENDENCIES = host-libxml2 # we don't want the libiconv dependency
>>
>> I agree that we normally prefer to rely on external libraries, rather
>> than built-in copies. However, in this case, I am wondering if we
>> shouldn't rather rely on the built-in copy, which is more lightweight
>> that building host-libxml2.
>>
>> From the DEPENDENCIES file of gettext:
>>
>> * libxml2
>>   + Optional.
>>     Needed for the --color option of the various programs.
>>     If not present, a subset of libxml2 (included in this package) will be
>>     compiled into libgettextlib.
>>   + Homepage:
>>     http://xmlsoft.org/
>>   + Download:
>>     ftp://xmlsoft.org/libxml2/
>>   + If it is installed in a nonstandard directory, pass the option
>>     --with-libxml2-prefix=DIR to 'configure'.
>>
>> So instead of building an external libxml2, we can pass
>> --with-included-libxml to the gettext-tools configure script to force
>> it to use the built-in libxml2 subset.
>>
>> Note that there are several other libraries in the same situation:
>> glib2, libcroco and libunistring.
>>
>> What is the opinion of other BR developers about this?
> 
> On my machine, host-libxml2 takes about 0m53.693s while host-gettext
> takes about 2m34.112s. So the overhead (~20%-pf-gettext)) is not
> negligible...
> 
> I am compeltely undecided on that... Do we consider a ~20%-of-gettext
> overhead for a full build to be important or not?
> 
>> And all that just for a --color command line option that we really
>> don't care about. Why the heck isn't this thing optional in the first place?
> 
> Eh... Upstream... ;-)

Given the following reasons:

- The difference of build time caused by using host-libxml2 is not that
much, as Yann said.

- We normally prefer to rely on external libraries

- Other packages could benefit on having host-libxml2 already built.
Otherwise we could end up with building libxml2 twice, the one bundled
in gettext and the external one.

I vote for using the host-libxml2 package instead of the bundled libxml2.

Regards,

Vincent.

> Regards,
> Yann E. MORIN.
>
Yann E. MORIN Dec. 26, 2015, 9:37 p.m. UTC | #4
Nicolas, Ignacy, All,

On 2015-02-23 14:03 +0100, Nicolas Cavallari spake thusly:
> From: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> 
> Otherwise, the locally-installed libxml2 will be used, which may
> depend on a locally-installed liblzma which may create conflict
> if host-liblzma is compiled.
> 
> Fixes https://bugs.busybox.net/show_bug.cgi?id=7886

I've sent an updated version of this patch:
    https://patchwork.ozlabs.org/patch/561119/

Thanks for the initial patch! :-)

Regards,
Yann E. MORIN.

> Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
> [nicolas: added extended commit message]
> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> ---
> Ignacy is unavailable this week, but he commited this change to our
> local tree before leaving.
> 
> diff --git a/package/gettext/gettext.mk b/package/gettext/gettext.mk
> index c2419c1..a554517 100644
> --- a/package/gettext/gettext.mk
> +++ b/package/gettext/gettext.mk
> @@ -12,7 +12,7 @@ GETTEXT_LICENSE = GPLv2+
>  GETTEXT_LICENSE_FILES = COPYING
>  
>  GETTEXT_DEPENDENCIES = $(if $(BR2_PACKAGE_LIBICONV),libiconv)
> -HOST_GETTEXT_DEPENDENCIES = # we don't want the libiconv dependency
> +HOST_GETTEXT_DEPENDENCIES = host-libxml2 # we don't want the libiconv dependency
>  
>  GETTEXT_CONF_OPTS += \
>  	--disable-libasprintf \
> -- 
> 2.1.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Gustavo Zacarias Dec. 29, 2015, 4:51 p.m. UTC | #5
On 14/09/15 12:09, Vicente Olivert Riera wrote:

> Given the following reasons:
>
> - The difference of build time caused by using host-libxml2 is not that
> much, as Yann said.
>
> - We normally prefer to rely on external libraries
>
> - Other packages could benefit on having host-libxml2 already built.
> Otherwise we could end up with building libxml2 twice, the one bundled
> in gettext and the external one.
>
> I vote for using the host-libxml2 package instead of the bundled libxml2.
>
> Regards,
>
> Vincent.

Hi All.
Given my recent bump to gettext 0.19.7 libxml2 is now required more than 
ever for the new ITS handling code.
The problem now is that older libxml2 versions might not be enough for 
host-gettext and result in build failure:
http://autobuild.buildroot.net/results/1b7/1b7b1244ccab111dbcdb6350525077d2a7157d91/build-end.log

It is silly that the configure script doesn't check for this, however we 
can't make conditionals for host packages (or better said, for things 
that may or may not be present in the distro), and i follow the same 
line of reasoning of avoiding bundled duplication - it's more a matter 
of build time than space for host packages, but still.

I'd vote for making it so as well.

Regards.
diff mbox

Patch

diff --git a/package/gettext/gettext.mk b/package/gettext/gettext.mk
index c2419c1..a554517 100644
--- a/package/gettext/gettext.mk
+++ b/package/gettext/gettext.mk
@@ -12,7 +12,7 @@  GETTEXT_LICENSE = GPLv2+
 GETTEXT_LICENSE_FILES = COPYING
 
 GETTEXT_DEPENDENCIES = $(if $(BR2_PACKAGE_LIBICONV),libiconv)
-HOST_GETTEXT_DEPENDENCIES = # we don't want the libiconv dependency
+HOST_GETTEXT_DEPENDENCIES = host-libxml2 # we don't want the libiconv dependency
 
 GETTEXT_CONF_OPTS += \
 	--disable-libasprintf \