All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1/2] package/gettext: Turn into virtual package
Date: Sun, 30 Dec 2018 18:10:26 +0100	[thread overview]
Message-ID: <20181230181026.3ce6aca3@windsurf> (raw)
In-Reply-To: <20181223150448.21980-2-vadim4j@gmail.com>

Hello Vadim,

First of all, thanks a *lot* for pursuing this work, it is really nice
to see some progress in this area. Every time host-gettext is building
on my machine, I'm really annoyed, and remember about the need to have
gettext-tiny support. See below for some comment.

On Sun, 23 Dec 2018 17:04:47 +0200, Vadim Kochan wrote:

> diff --git a/package/gettext/Config.in b/package/gettext/Config.in
> index e55663b1d7..9546468571 100644
> --- a/package/gettext/Config.in
> +++ b/package/gettext/Config.in
> @@ -1,5 +1,12 @@
> -config BR2_PACKAGE_GETTEXT
> +menuconfig BR2_PACKAGE_GETTEXT
>  	bool "gettext"
> +	select BR2_PACKAGE_HAS_GETTEXT
> +
> +if BR2_PACKAGE_GETTEXT
> +
> +config BR2_PACKAGE_GETTEXT_GNU
> +	bool "gettext-gnu"
> +	default y
>  	depends on BR2_USE_WCHAR
>  	help
>  	  The GNU `gettext' utilities are a set of tools that provide a
> @@ -12,14 +19,23 @@ config BR2_PACKAGE_GETTEXT
>  
>  	  http://www.gnu.org/software/gettext/
>  
> -if BR2_PACKAGE_GETTEXT
> +comment "gettext-gnu needs a toolchain w/ wchar"
> +	depends on !BR2_USE_WCHAR
>  
>  config BR2_PACKAGE_GETTEXT_PROVIDES_LIBINTL
>  	bool
>  	default y if BR2_SYSTEM_ENABLE_NLS
>  	depends on !BR2_TOOLCHAIN_HAS_FULL_GETTEXT
>  
> -endif
> +config BR2_PACKAGE_PROVIDES_GETTEXT
> +	string
> +	default "gettext-gnu" if BR2_PACKAGE_GETTEXT_GNU
>  
> -comment "gettext needs a toolchain w/ wchar"
> -	depends on !BR2_USE_WCHAR
> +config BR2_PACKAGE_HAS_GETTEXT
> +	bool
> +
> +config BR2_PACKAGE_PROVIDES_HOST_GETTEXT
> +	string
> +	default "host-gettext-gnu"
> +
> +endif

I'm not convinced by how this virtual package is handled in Config.in.
In fact, if you apply both of your patches, it's broken, you have:

config BR2_PACKAGE_PROVIDES_HOST_GETTEXT
        string
        default "host-gettext-gnu" if BR2_PACKAGE_GETTEXT_GNU
        default "host-gettext-tiny" if BR2_PACKAGE_GETTEXT_TINY

which means that when the target package is not selected, the
host-gettext virtual package doesn't "point" to anything. And indeed:
do a simple build with nothing enable, and run "make host-flex". It
will fail with:

make[1]: *** No rule to make target 'host-', needed by '/home/thomas/projets/buildroot/output/build/host-gettext/.stamp_configured'.  Stop.
make: *** [Makefile:84: _all] Error 2

Because the host-gettext virtual package has no provider.

So let's analyze this in detail.

First, for the *target* gettext virtual package, what do we want. I
guess what we want is simply:

 - To use the full gettext when BR2_TARGET_ENABLE_NLS=y

 - To use gettext-tiny when BR2_TARGET_ENABLE_NLS is disabled

I don't see any meaningful use case for full gettext with
BR2_TARGET_ENABLE_NLS disabled, or for gettext-tiny with
BR2_TARGET_ENABLE_NLS=y.

To me, this means that for the target package,
package/gettext/Config.in shouldn't offer a choice between full and
tiny: it should automatically use full or tiny depending on the
BR2_TARGET_ENABLE_NLS value.

Now, there is the question of what to do with the host-gettext virtual
package, i.e in which case it should use the full host gettext or host
gettext-tiny.

So, we need host-gettext for two things:

 - Some packages explicitly need it.

 - Some packages need it through TARGET_NLS_DEPENDENCIES

 - Some packages need it through <pkg>_GETTEXTIZE = YES

So we need to understand if the rule is as simple as: full host gettext
when BR2_TARGET_ENABLE_NLS=y and tiny host gettext when
BR2_TARGET_ENABLE_NLS is disabled, or whether it is possible to use
gettext tiny for host gettext when BR2_TARGET_ENABLE_NLS=y.

Do you have some ideas about this ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  parent reply	other threads:[~2018-12-30 17:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-23 15:04 [Buildroot] [PATCH 0/2] Add gettext-tiny package Vadim Kochan
2018-12-23 15:04 ` [Buildroot] [PATCH 1/2] package/gettext: Turn into virtual package Vadim Kochan
2018-12-23 15:36   ` Yann E. MORIN
2018-12-30 17:10   ` Thomas Petazzoni [this message]
2018-12-31  4:03     ` Vadim Kochan
2018-12-31 15:06       ` Thomas Petazzoni
2019-01-03 14:28         ` Vadim Kochan
2019-01-03 14:28           ` Thomas Petazzoni
     [not found]             ` <CAMw6YJ+xU9GiZ809dCQTmPEgtpu3iCOErT5w7fqoee09R-hpaQ@mail.gmail.com>
     [not found]               ` <CAMw6YJLvioTwKbgAdYvtZqbVg1W-RAADT_umZkGMaw9MnVpHMw@mail.gmail.com>
2019-01-04  8:39                 ` Thomas Petazzoni
2018-12-23 15:04 ` [Buildroot] [PATCH 2/2] package/gettext-tiny: Add new package Vadim Kochan
2018-12-23 21:21   ` Yann E. MORIN
2018-12-31  4:07     ` Vadim Kochan

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=20181230181026.3ce6aca3@windsurf \
    --to=thomas.petazzoni@bootlin.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.