diff mbox

[1/1] kbd: fix target install

Message ID 1457786638-13861-1-git-send-email-niels@tonebarker.dk
State Changes Requested
Headers show

Commit Message

Niels Skou Olsen March 12, 2016, 12:43 p.m. UTC
The package makefile fails to append to the KBD_INSTALL_TARGET_OPTS variable in
autotools-package. This means that the DESTDIR and install target are lost, and
so the package is never installed to target.

To fix it the append must happen after the eval of autotools-package.

Signed-off-by: Niels Skou Olsen <niels@tonebarker.dk>
---
 package/kbd/kbd.mk | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Yann E. MORIN March 12, 2016, 10:01 p.m. UTC | #1
Niel, All,

On 2016-03-12 13:43 +0100, Niels Skou Olsen spake thusly:
> The package makefile fails to append to the KBD_INSTALL_TARGET_OPTS variable in
> autotools-package. This means that the DESTDIR and install target are lost, and
> so the package is never installed to target.
> 
> To fix it the append must happen after the eval of autotools-package.
> 
> Signed-off-by: Niels Skou Olsen <niels@tonebarker.dk>
> ---
>  package/kbd/kbd.mk | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/package/kbd/kbd.mk b/package/kbd/kbd.mk
> index 9dabce1..72519ff 100644
> --- a/package/kbd/kbd.mk
> +++ b/package/kbd/kbd.mk
> @@ -17,6 +17,7 @@ KBD_LICENSE = GPLv2+
>  KBD_LICENSE_FILES = COPYING
>  KBD_AUTORECONF = YES
>  
> -KBD_INSTALL_TARGET_OPTS += MKINSTALLDIRS=$(@D)/config/mkinstalldirs
> -
>  $(eval $(autotools-package))
> +
> +# must happen after the autotools-package eval, or else the append becomes an overwrite
> +KBD_INSTALL_TARGET_OPTS += MKINSTALLDIRS=$(@D)/config/mkinstalldirs

We don;t usually do that.

Instead, when the package does not behave properly, we prefer to
override all of it, like, for example, in  the rpm package:

    KBD_INSTALL_TARGET_OPTS = \
        DESTDIR=$(TARGET_DIR)/usr \
        MKINSTALLDIRS=$(@D)/config/mkinstalldirs \
        install

    $(eval $(autotools-package))

I marked your patch as Changes Requested on outr Patchwork. Care to fix
and respi, please?

Regards,
Yann E. MORIN.

> -- 
> 2.5.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Niels Skou Olsen March 13, 2016, 4:37 p.m. UTC | #2
> On 12. mar. 2016, at 23.01, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> 
> Niel, All,
> 
> On 2016-03-12 13:43 +0100, Niels Skou Olsen spake thusly:
>> The package makefile fails to append to the KBD_INSTALL_TARGET_OPTS variable in
>> autotools-package. This means that the DESTDIR and install target are lost, and
>> so the package is never installed to target.
>> 
>> To fix it the append must happen after the eval of autotools-package.
> 
> We don;t usually do that.
> 
> Instead, when the package does not behave properly, we prefer to
> override all of it, like, for example, in  the rpm package:

OK, I agree that overriding is cleaner. I will re-submit a kbd patch that overrides instead of appending.

Btw, I just grepped through the packages, and found that util-linux does the same. It appends to autotools-package and host-autotools-package variables *after* evaluating the macros. 

Best regards,
Niels
diff mbox

Patch

diff --git a/package/kbd/kbd.mk b/package/kbd/kbd.mk
index 9dabce1..72519ff 100644
--- a/package/kbd/kbd.mk
+++ b/package/kbd/kbd.mk
@@ -17,6 +17,7 @@  KBD_LICENSE = GPLv2+
 KBD_LICENSE_FILES = COPYING
 KBD_AUTORECONF = YES
 
-KBD_INSTALL_TARGET_OPTS += MKINSTALLDIRS=$(@D)/config/mkinstalldirs
-
 $(eval $(autotools-package))
+
+# must happen after the autotools-package eval, or else the append becomes an overwrite
+KBD_INSTALL_TARGET_OPTS += MKINSTALLDIRS=$(@D)/config/mkinstalldirs