[3/3] package/wget: use the new gettextize infra

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Currently, the gettextization of wget works by chance:
  - host-gettext is added as a dependency to wget;
  - gettextize is run as a post-patch hook.

But the dependencies are only guaranteed to be built and installed
for the configure step, not the patch step. Becasue post-patch hooks
are part of the patch step, we have no guarantee that the dependency
to host-gettext is done by the time we gettextize wget.

This happens to work by chance, since wget sorts alphabetically after
gettext, so we indeed have host-gettext built and installed by the
time we need to gettextize wget.

This is prone to fail in the parallel build case, sicne we can no
longer rely on alphabetical order in that case.

Instead, use the new gettextize infra we just added.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/wget/wget.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Hi Yann,

Thanks for noticing.

On Thu, Apr 10, 2014 at 12:20:05AM +0200, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Currently, the gettextization of wget works by chance:
>   - host-gettext is added as a dependency to wget;
>   - gettextize is run as a post-patch hook.
> 
> But the dependencies are only guaranteed to be built and installed
> for the configure step, not the patch step. Becasue post-patch hooks
> are part of the patch step, we have no guarantee that the dependency
> to host-gettext is done by the time we gettextize wget.
> 
> This happens to work by chance, since wget sorts alphabetically after
> gettext, so we indeed have host-gettext built and installed by the
> time we need to gettextize wget.
> 
> This is prone to fail in the parallel build case, sicne we can no
> longer rely on alphabetical order in that case.

Fixing this only requires using PRE_CONFIGURE_HOOK instead of POST_PATCH_HOOK, 
isn't it? Is there anything else this series attempts to fix?

baruch

> Instead, use the new gettextize infra we just added.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  package/wget/wget.mk | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/package/wget/wget.mk b/package/wget/wget.mk
> index f3a5274..031a8ea 100644
> --- a/package/wget/wget.mk
> +++ b/package/wget/wget.mk
> @@ -15,11 +15,7 @@ WGET_AUTORECONF = YES
>  
>  # Ugly kludge to fix autoreconf with old gettext infra
>  # We need to gettextize before autoreconf to upgrade
> -WGET_DEPENDENCIES += host-gettext
> -define WGET_GETTEXTIZE
> -	cd $(@D) ; $(HOST_DIR)/usr/bin/gettextize -f
> -endef
> -WGET_POST_PATCH_HOOKS += WGET_GETTEXTIZE
> +WGET_GETTEXTIZE = YES
>  
>  # Prefer full-blown wget over busybox
>  ifeq ($(BR2_PACKAGE_BUSYBOX),y)
> -- 
> 1.8.3.2

Baruch, All,

On 2014-04-10 06:34 +0300, Baruch Siach spake thusly:
> On Thu, Apr 10, 2014 at 12:20:05AM +0200, Yann E. MORIN wrote:
> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > 
> > Currently, the gettextization of wget works by chance:
> >   - host-gettext is added as a dependency to wget;
> >   - gettextize is run as a post-patch hook.
> > 
> > But the dependencies are only guaranteed to be built and installed
> > for the configure step, not the patch step. Becasue post-patch hooks
> > are part of the patch step, we have no guarantee that the dependency
> > to host-gettext is done by the time we gettextize wget.
> > 
> > This happens to work by chance, since wget sorts alphabetically after
> > gettext, so we indeed have host-gettext built and installed by the
> > time we need to gettextize wget.
> > 
> > This is prone to fail in the parallel build case, sicne we can no
> > longer rely on alphabetical order in that case.
> 
> Fixing this only requires using PRE_CONFIGURE_HOOK instead of POST_PATCH_HOOK, 
> isn't it? Is there anything else this series attempts to fix?

Yes, switching it to a pre-configure hook would fix it.
But that would be a one-off solution.

Providing it in the autotools-infra will allow us to have consistency
across packages and would allow us to fix it in a single place in the
future, rather than having to hunt down allow the offenders.

But as I said, this is an RFC, so we can discuss it further.

First, we have to agree we want it in the autotools infra or not.

Then, we'll have to check whether a boolean + an _OPT is enough, or if
we need something a bit more involved (e.g. let packages define the
complete hook, and just have the infra call it at the proper moment.)

Regards,
Yann E. MORIN.