[Buildroot] [PATCH v2 0/3] Add TARGET_FINALIZE_HOOKS

[Buildroot] [PATCH v2 0/3] Add TARGET_FINALIZE_HOOKS

[Fabio Porcedda]

This patch set add TARGET_FINALIZE_HOOKS and use it for system.mk
recipies.

v2:
- use hooks (Thomas & Arnout)

Fabio Porcedda (3):
  Makefile: target-finalize: add TARGET_FINALIZE_HOOKS
  system: convert "system.mk" recipes to "target-finalize" hooks
  Makefile: do not add to targets common dependencies

 Makefile         |  9 +++----
 system/system.mk | 82 +++++++++++++++++++++++++++++++-------------------------
 2 files changed, 48 insertions(+), 43 deletions(-)

-- 
2.0.0

[Buildroot] [PATCH v2 1/3] Makefile: target-finalize: add TARGET_FINALIZE_HOOKS

[Fabio Porcedda]

Add TARGET_FINALIZE_HOOKS to the "target-finalize" rule to be able to
add to it commands as needed.
This is useful for having a nicer output because commands are executed
after the "target-finalize" initial message, also it is useful to ensure
an executing order even when top-level parallel makefile is being used.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index b576267..9de4806 100644
--- a/Makefile
+++ b/Makefile
@@ -513,7 +513,7 @@ ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
 LOCALE_WHITELIST = $(BUILD_DIR)/locales.nopurge
 LOCALE_NOPURGE = $(call qstrip,$(BR2_ENABLE_LOCALE_WHITELIST))
 
-define TARGET_PURGE_LOCALES
+define PURGE_LOCALES
 	rm -f $(LOCALE_WHITELIST)
 	for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done
 
@@ -525,13 +525,14 @@ define TARGET_PURGE_LOCALES
 		done; \
 	done
 endef
+TARGET_FINALIZE_HOOKS += PURGE_LOCALES
 endif
 
 $(TARGETS_ROOTFS): target-finalize
 
 target-finalize: $(TARGETS)
 	@$(call MESSAGE,"Finalizing target directory")
-	$(TARGET_PURGE_LOCALES)
+	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
 	rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \
 		$(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \
 		$(TARGET_DIR)/usr/lib/cmake $(TARGET_DIR)/usr/share/cmake
-- 
2.0.0

[Buildroot] [PATCH v2 2/3] system: convert "system.mk" recipes to "target-finalize" hooks

[Fabio Porcedda]

Convert "system.mk" recipes to "target-finalize" hooks in order to:
- Ensure an ordering even if top-level parallel make is being used.
- Execute "system.mk" commands after the "target-finalize" initial message
  is printed so they can be clearly distinguished from packages
  building.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---

Notes:
    v2:
     - use hooks

 system/system.mk | 82 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/system/system.mk b/system/system.mk
index 01a6c3a..c01ab0f 100644
--- a/system/system.mk
+++ b/system/system.mk
@@ -7,68 +7,76 @@ TARGET_GENERIC_GETTY_BAUDRATE = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRAT
 TARGET_GENERIC_GETTY_TERM = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_TERM))
 TARGET_GENERIC_GETTY_OPTIONS = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_OPTIONS))
 
-target-generic-securetty:
+ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
+define SYSTEM_GENERIC_SECURETTY
 	grep -q '^$(TARGET_GENERIC_GETTY_PORT)$$' $(TARGET_DIR)/etc/securetty || \
 		echo '$(TARGET_GENERIC_GETTY_PORT)' >> $(TARGET_DIR)/etc/securetty
+endef
+TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_SECURETTY
+endif
 
-target-generic-hostname:
+ifneq ($(TARGET_GENERIC_HOSTNAME),)
+define SYSTEM_GENERIC_HOSTNAME
 	mkdir -p $(TARGET_DIR)/etc
 	echo "$(TARGET_GENERIC_HOSTNAME)" > $(TARGET_DIR)/etc/hostname
 	$(SED) '$$a \127.0.1.1\t$(TARGET_GENERIC_HOSTNAME)' \
 		-e '/^127.0.1.1/d' $(TARGET_DIR)/etc/hosts
+endef
+TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_HOSTNAME
+endif
 
-target-generic-issue:
+ifneq ($(TARGET_GENERIC_ISSUE),)
+define SYSTEM_GENERIC_ISSUE
 	mkdir -p $(TARGET_DIR)/etc
 	echo "$(TARGET_GENERIC_ISSUE)" > $(TARGET_DIR)/etc/issue
+endef
+TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_ISSUE
+endif
 
 ifneq ($(TARGET_GENERIC_ROOT_PASSWD),)
-target-root-passwd: host-mkpasswd
+TARGETS += host-mkpasswd
 endif
-target-root-passwd:
+
+ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
+
+define SYSTEM_ROOT_PASSWD
 	[ -n "$(TARGET_GENERIC_ROOT_PASSWD)" ] && \
 		TARGET_GENERIC_ROOT_PASSWD_HASH=$$($(MKPASSWD) -m "$(TARGET_GENERIC_PASSWD_METHOD)" "$(TARGET_GENERIC_ROOT_PASSWD)"); \
 	$(SED) "s,^root:[^:]*:,root:$$TARGET_GENERIC_ROOT_PASSWD_HASH:," $(TARGET_DIR)/etc/shadow
+endef
+TARGET_FINALIZE_HOOKS += SYSTEM_ROOT_PASSWD
 
-target-generic-getty-busybox:
-	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
-		$(TARGET_DIR)/etc/inittab
-
+ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
+ifeq ($(BR2_PACKAGE_SYSVINIT),y)
 # In sysvinit inittab, the "id" must not be longer than 4 bytes, so we
 # skip the "tty" part and keep only the remaining.
-target-generic-getty-sysvinit:
+define SYSTEM_GENERIC_GETTY_SYSVINIT
 	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
 		$(TARGET_DIR)/etc/inittab
+endef
+TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_GETTY_SYSVINIT
+else
+# Add getty to busybox inittab
+define SYSTEM_GENERIC_GETTY_BUSYBOX
+	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
+		$(TARGET_DIR)/etc/inittab
+endef
+TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_GETTY_BUSYBOX
+endif
+endif
 
+ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
 # Find commented line, if any, and remove leading '#'s
-target-generic-do-remount-rw:
+define SYSTEM_GENERIC_DO_REMOUNT_RW
 	$(SED) '/^#.*# REMOUNT_ROOTFS_RW$$/s~^#\+~~' $(TARGET_DIR)/etc/inittab
-
+endef
+TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_DO_REMOUNT_RW
+else
 # Find uncommented line, if any, and add a leading '#'
-target-generic-dont-remount-rw:
+define SYSTEM_GENERIC_DONT_REMOUNT_RW
 	$(SED) '/^[^#].*# REMOUNT_ROOTFS_RW$$/s~^~#~' $(TARGET_DIR)/etc/inittab
-
-ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
-TARGETS += target-generic-securetty
-endif
-
-ifneq ($(TARGET_GENERIC_HOSTNAME),)
-TARGETS += target-generic-hostname
-endif
-
-ifneq ($(TARGET_GENERIC_ISSUE),)
-TARGETS += target-generic-issue
+endef
+TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_DONT_REMOUNT_RW
 endif
 
-ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
-TARGETS += target-root-passwd
-
-ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
-TARGETS += target-generic-getty-$(if $(BR2_PACKAGE_SYSVINIT),sysvinit,busybox)
-endif
-
-ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
-TARGETS += target-generic-do-remount-rw
-else
-TARGETS += target-generic-dont-remount-rw
-endif
-endif
+endif # BR2_ROOTFS_SKELETON_DEFAULT
-- 
2.0.0

[Buildroot] [PATCH v2 3/3] Makefile: do not add to targets common dependencies

[Fabio Porcedda]

Remove the rule that add common dependencies to every targets in the
"TARGETS" variable because all those targets are packages that use the package
framework that already add common dependencies.
---
 Makefile | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/Makefile b/Makefile
index 9de4806..c028cf8 100644
--- a/Makefile
+++ b/Makefile
@@ -433,10 +433,6 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
 
 prepare: $(BUILD_DIR)/buildroot-config/auto.conf
 
-# Add base dependencies to all targets even on those not based on the
-# package framework.
-$(TARGETS): dirs prepare dependencies
-
 world: target-post-image
 
 .PHONY: all world toolchain dirs clean distclean source outputmakefile \
-- 
2.0.0

[Buildroot] [PATCH v2 1/3] Makefile: target-finalize: add TARGET_FINALIZE_HOOKS

[Arnout Vandecappelle]

On 20/06/14 10:26, Fabio Porcedda wrote:
> Add TARGET_FINALIZE_HOOKS to the "target-finalize" rule to be able to
> add to it commands as needed.
> This is useful for having a nicer output because commands are executed
> after the "target-finalize" initial message, also it is useful to ensure
> an executing order even when top-level parallel makefile is being used.

 This commit message suggests that the patch would just add the call to the
hooks, not actually changing the purge-locales implementation (which would be a
separate patch).

 For me, it doesn't have to a be a separate patch, but at least that additional
change should be mentioned in the commit message.

 Regards,
 Arnout

> 
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  Makefile | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index b576267..9de4806 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -513,7 +513,7 @@ ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
>  LOCALE_WHITELIST = $(BUILD_DIR)/locales.nopurge
>  LOCALE_NOPURGE = $(call qstrip,$(BR2_ENABLE_LOCALE_WHITELIST))
>  
> -define TARGET_PURGE_LOCALES
> +define PURGE_LOCALES
>  	rm -f $(LOCALE_WHITELIST)
>  	for i in $(LOCALE_NOPURGE); do echo $$i >> $(LOCALE_WHITELIST); done
>  
> @@ -525,13 +525,14 @@ define TARGET_PURGE_LOCALES
>  		done; \
>  	done
>  endef
> +TARGET_FINALIZE_HOOKS += PURGE_LOCALES
>  endif
>  
>  $(TARGETS_ROOTFS): target-finalize
>  
>  target-finalize: $(TARGETS)
>  	@$(call MESSAGE,"Finalizing target directory")
> -	$(TARGET_PURGE_LOCALES)
> +	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
>  	rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \
>  		$(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \
>  		$(TARGET_DIR)/usr/lib/cmake $(TARGET_DIR)/usr/share/cmake
> 


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 2/3] system: convert "system.mk" recipes to "target-finalize" hooks

[Arnout Vandecappelle]

On 20/06/14 10:26, Fabio Porcedda wrote:
> Convert "system.mk" recipes to "target-finalize" hooks in order to:
> - Ensure an ordering even if top-level parallel make is being used.
> - Execute "system.mk" commands after the "target-finalize" initial message
>   is printed so they can be clearly distinguished from packages
>   building.
> 
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 One small remark:

[snip]
> +ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
> +ifeq ($(BR2_PACKAGE_SYSVINIT),y)
>  # In sysvinit inittab, the "id" must not be longer than 4 bytes, so we
>  # skip the "tty" part and keep only the remaining.
> -target-generic-getty-sysvinit:
> +define SYSTEM_GENERIC_GETTY_SYSVINIT
>  	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
>  		$(TARGET_DIR)/etc/inittab
> +endef
> +TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_GETTY_SYSVINIT
> +else
> +# Add getty to busybox inittab
> +define SYSTEM_GENERIC_GETTY_BUSYBOX
> +	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
> +		$(TARGET_DIR)/etc/inittab
> +endef
> +TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_GETTY_BUSYBOX

 Wouldn't it be better/cleaner to define SYSTEM_GENERIC_GETTY in both cases, and
just define it differently depending on BR2_PACKAGE_SYSVINIT? Same for the
REMOUNT_ROOTFS, below.


 Regards,
 Arnout

> +endif
> +endif
>  
> +ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
>  # Find commented line, if any, and remove leading '#'s
> -target-generic-do-remount-rw:
> +define SYSTEM_GENERIC_DO_REMOUNT_RW
>  	$(SED) '/^#.*# REMOUNT_ROOTFS_RW$$/s~^#\+~~' $(TARGET_DIR)/etc/inittab
> -
> +endef
> +TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_DO_REMOUNT_RW
> +else
>  # Find uncommented line, if any, and add a leading '#'
> -target-generic-dont-remount-rw:
> +define SYSTEM_GENERIC_DONT_REMOUNT_RW
>  	$(SED) '/^[^#].*# REMOUNT_ROOTFS_RW$$/s~^~#~' $(TARGET_DIR)/etc/inittab
> -
> -ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
> -TARGETS += target-generic-securetty
> -endif
> -
> -ifneq ($(TARGET_GENERIC_HOSTNAME),)
> -TARGETS += target-generic-hostname
> -endif
> -
> -ifneq ($(TARGET_GENERIC_ISSUE),)
> -TARGETS += target-generic-issue
> +endef
> +TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_DONT_REMOUNT_RW
>  endif
>  
> -ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
> -TARGETS += target-root-passwd
> -
> -ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
> -TARGETS += target-generic-getty-$(if $(BR2_PACKAGE_SYSVINIT),sysvinit,busybox)
> -endif
> -
> -ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
> -TARGETS += target-generic-do-remount-rw
> -else
> -TARGETS += target-generic-dont-remount-rw
> -endif
> -endif
> +endif # BR2_ROOTFS_SKELETON_DEFAULT
> 


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 3/3] Makefile: do not add to targets common dependencies

[Arnout Vandecappelle]

On 20/06/14 10:26, Fabio Porcedda wrote:
> Remove the rule that add common dependencies to every targets in the
> "TARGETS" variable because all those targets are packages that use the package
> framework that already add common dependencies.

 There are still two targets that don't:

- target-generatelocales - should probably be converted to a
TARGET_FINALIZE_HOOK as well;

- toolchain-eclipse-register - doesn't need to depend on anything I think, but
anyway could also be converted to a TARGET_FINALIZE_HOOK.


 So neither of these are really critical.

 Also, you forgot you Sob.


 Regards,
 Arnout

> ---
>  Makefile | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9de4806..c028cf8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -433,10 +433,6 @@ $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
>  
>  prepare: $(BUILD_DIR)/buildroot-config/auto.conf
>  
> -# Add base dependencies to all targets even on those not based on the
> -# package framework.
> -$(TARGETS): dirs prepare dependencies
> -
>  world: target-post-image
>  
>  .PHONY: all world toolchain dirs clean distclean source outputmakefile \
> 


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 1/3] Makefile: target-finalize: add TARGET_FINALIZE_HOOKS

[Fabio Porcedda]

On Sat, Jun 21, 2014 at 1:19 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 20/06/14 10:26, Fabio Porcedda wrote:
>> Add TARGET_FINALIZE_HOOKS to the "target-finalize" rule to be able to
>> add to it commands as needed.
>> This is useful for having a nicer output because commands are executed
>> after the "target-finalize" initial message, also it is useful to ensure
>> an executing order even when top-level parallel makefile is being used.
>
>  This commit message suggests that the patch would just add the call to the
> hooks, not actually changing the purge-locales implementation (which would be a
> separate patch).
>
>  For me, it doesn't have to a be a separate patch, but at least that additional
> change should be mentioned in the commit message.

Ok, i will add at the end:

Also convert "TARGET_PURGE_LOCALES" to a hook that use
"TARGET_FINALIZE_HOOKS".

Thanks && BR
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 1/3] Makefile: target-finalize: add TARGET_FINALIZE_HOOKS

[Arnout Vandecappelle]

On 23/06/14 11:14, Fabio Porcedda wrote:
> On Sat, Jun 21, 2014 at 1:19 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 20/06/14 10:26, Fabio Porcedda wrote:
>>> Add TARGET_FINALIZE_HOOKS to the "target-finalize" rule to be able to
>>> add to it commands as needed.
>>> This is useful for having a nicer output because commands are executed
>>> after the "target-finalize" initial message, also it is useful to ensure
>>> an executing order even when top-level parallel makefile is being used.
>>
>>  This commit message suggests that the patch would just add the call to the
>> hooks, not actually changing the purge-locales implementation (which would be a
>> separate patch).
>>
>>  For me, it doesn't have to a be a separate patch, but at least that additional
>> change should be mentioned in the commit message.
> 
> Ok, i will add at the end:
> 
> Also convert "TARGET_PURGE_LOCALES" to a hook that use

 uses


 You can also add my

    Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

in that case.

 Regards,
 Arnout

> "TARGET_FINALIZE_HOOKS".
> 
> Thanks && BR
> 


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 2/3] system: convert "system.mk" recipes to "target-finalize" hooks

[Fabio Porcedda]

On Sat, Jun 21, 2014 at 1:35 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 20/06/14 10:26, Fabio Porcedda wrote:
>> Convert "system.mk" recipes to "target-finalize" hooks in order to:
>> - Ensure an ordering even if top-level parallel make is being used.
>> - Execute "system.mk" commands after the "target-finalize" initial message
>>   is printed so they can be clearly distinguished from packages
>>   building.
>>
>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
>  One small remark:
>
> [snip]
>> +ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
>> +ifeq ($(BR2_PACKAGE_SYSVINIT),y)
>>  # In sysvinit inittab, the "id" must not be longer than 4 bytes, so we
>>  # skip the "tty" part and keep only the remaining.
>> -target-generic-getty-sysvinit:
>> +define SYSTEM_GENERIC_GETTY_SYSVINIT
>>       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
>>               $(TARGET_DIR)/etc/inittab
>> +endef
>> +TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_GETTY_SYSVINIT
>> +else
>> +# Add getty to busybox inittab
>> +define SYSTEM_GENERIC_GETTY_BUSYBOX
>> +     $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
>> +             $(TARGET_DIR)/etc/inittab
>> +endef
>> +TARGET_FINALIZE_HOOKS += SYSTEM_GENERIC_GETTY_BUSYBOX
>
>  Wouldn't it be better/cleaner to define SYSTEM_GENERIC_GETTY in both cases, and
> just define it differently depending on BR2_PACKAGE_SYSVINIT? Same for the
> REMOUNT_ROOTFS, below.
<snip>

Well, In a first attempt i was doing like that but later I've decided
to follow the original style because is more descriptive.

I will change the patch as you suggested.

Thanks & BR
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 1/3] Makefile: target-finalize: add TARGET_FINALIZE_HOOKS

[Fabio Porcedda]

On Mon, Jun 23, 2014 at 11:20 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 23/06/14 11:14, Fabio Porcedda wrote:
>> On Sat, Jun 21, 2014 at 1:19 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>> On 20/06/14 10:26, Fabio Porcedda wrote:
>>>> Add TARGET_FINALIZE_HOOKS to the "target-finalize" rule to be able to
>>>> add to it commands as needed.
>>>> This is useful for having a nicer output because commands are executed
>>>> after the "target-finalize" initial message, also it is useful to ensure
>>>> an executing order even when top-level parallel makefile is being used.
>>>
>>>  This commit message suggests that the patch would just add the call to the
>>> hooks, not actually changing the purge-locales implementation (which would be a
>>> separate patch).
>>>
>>>  For me, it doesn't have to a be a separate patch, but at least that additional
>>> change should be mentioned in the commit message.
>>
>> Ok, i will add at the end:
>>
>> Also convert "TARGET_PURGE_LOCALES" to a hook that use
>
>  uses
>
>
>  You can also add my
>
>     Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
> in that case.

Done, thanks.

BR
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 3/3] Makefile: do not add to targets common dependencies

[Fabio Porcedda]

On Sat, Jun 21, 2014 at 1:48 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 20/06/14 10:26, Fabio Porcedda wrote:
>> Remove the rule that add common dependencies to every targets in the
>> "TARGETS" variable because all those targets are packages that use the package
>> framework that already add common dependencies.

I will update the description with a better description:

Remove the rule that add common dependencies to every targets in the
"TARGETS" variable because all those targets are packages that use the
package framework or they depends on targets that use the package
framework, because the package framework already add common dependencies
this rule it's useless.

>  There are still two targets that don't:
>
> - target-generatelocales - should probably be converted to a
> TARGET_FINALIZE_HOOK as well;

I will do:

ifneq ($(GENERATE_LOCALE),)
TARGETS+=host-localedef
define GENERATE_LOCALE_HOOK
...
endif
TARGET_FINALIZE_HOOKS += GENERATE_LOCALE_HOOK
endif

> - toolchain-eclipse-register - doesn't need to depend on anything I think, but
> anyway could also be converted to a TARGET_FINALIZE_HOOK.

It depends on the toolchain target, but the toolchain is always built
so it's safe to convert to a hook.

>
>  So neither of these are really critical.

The only advantage to convert those targets to hooks is for consistency?

BTW: Both of those targets depends on the toolchain target that use
the package framework so they have already the common dependencies.

>  Also, you forgot you Sob.

Yes, thanks.

BR
-- 
Fabio Porcedda

[Buildroot] [PATCH v2 3/3] Makefile: do not add to targets common dependencies

[Arnout Vandecappelle]

On 23/06/14 11:51, Fabio Porcedda wrote:
> On Sat, Jun 21, 2014 at 1:48 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 20/06/14 10:26, Fabio Porcedda wrote:
>>> Remove the rule that add common dependencies to every targets in the
>>> "TARGETS" variable because all those targets are packages that use the package
>>> framework that already add common dependencies.
> 
> I will update the description with a better description:
> 
> Remove the rule that add common dependencies to every targets in the
> "TARGETS" variable because all those targets are packages that use the
> package framework or they depends on targets that use the package
> framework, because the package framework already add common dependencies
> this rule it's useless.

 Several spelling issues here; corrected text:

Remove the rule that adds common dependencies to every target in the
"TARGETS" variable, because all those targets are packages that use the
package infrastructure or they depend on targets that use the package
infrastructure. The package infrastructure already adds common dependencies.
Therefore, this rule is useless.


> 
>>  There are still two targets that don't:
>>
>> - target-generatelocales - should probably be converted to a
>> TARGET_FINALIZE_HOOK as well;
> 
> I will do:
> 
> ifneq ($(GENERATE_LOCALE),)
> TARGETS+=host-localedef

 Spaces around +=

 Otherwise it's perfect.

> define GENERATE_LOCALE_HOOK
> ...
> endif
> TARGET_FINALIZE_HOOKS += GENERATE_LOCALE_HOOK
> endif
> 
>> - toolchain-eclipse-register - doesn't need to depend on anything I think, but
>> anyway could also be converted to a TARGET_FINALIZE_HOOK.
> 
> It depends on the toolchain target, but the toolchain is always built
> so it's safe to convert to a hook.

 Actually, the toolchain is not always built, e.g. 'make host-foo' will not
build it. Also if you haven't selected any target package, it won't be built.
But that's a bit far-fetched I guess.

 And anyway, the toolchain-eclipse-register target doesn't really depend on the
toolchain - it just creates a file that refers to it. Right?

> 
>>
>>  So neither of these are really critical.
> 
> The only advantage to convert those targets to hooks is for consistency?

 Indeed.

> 
> BTW: Both of those targets depends on the toolchain target that use
> the package framework so they have already the common dependencies.

 Indeed.


 Regards,
 Arnout


> 
>>  Also, you forgot you Sob.
> 
> Yes, thanks.
> 
> BR
> 


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 3/3] Makefile: do not add to targets common dependencies

[Fabio Porcedda]

On Mon, Jun 23, 2014 at 2:34 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 23/06/14 11:51, Fabio Porcedda wrote:
>> On Sat, Jun 21, 2014 at 1:48 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>> On 20/06/14 10:26, Fabio Porcedda wrote:
>>>> Remove the rule that add common dependencies to every targets in the
>>>> "TARGETS" variable because all those targets are packages that use the package
>>>> framework that already add common dependencies.
>>
>> I will update the description with a better description:
>>
>> Remove the rule that add common dependencies to every targets in the
>> "TARGETS" variable because all those targets are packages that use the
>> package framework or they depends on targets that use the package
>> framework, because the package framework already add common dependencies
>> this rule it's useless.
>
>  Several spelling issues here; corrected text:
>
> Remove the rule that adds common dependencies to every target in the
> "TARGETS" variable, because all those targets are packages that use the
> package infrastructure or they depend on targets that use the package
> infrastructure. The package infrastructure already adds common dependencies.
> Therefore, this rule is useless.

Thanks a lot, I've updated the description.

>
>>
>>>  There are still two targets that don't:
>>>
>>> - target-generatelocales - should probably be converted to a
>>> TARGET_FINALIZE_HOOK as well;
>>
>> I will do:
>>
>> ifneq ($(GENERATE_LOCALE),)
>> TARGETS+=host-localedef
>
>  Spaces around +=
>
>  Otherwise it's perfect.

Ok.

>> define GENERATE_LOCALE_HOOK
>> ...
>> endif
>> TARGET_FINALIZE_HOOKS += GENERATE_LOCALE_HOOK
>> endif
>>
>>> - toolchain-eclipse-register - doesn't need to depend on anything I think, but
>>> anyway could also be converted to a TARGET_FINALIZE_HOOK.
>>
>> It depends on the toolchain target, but the toolchain is always built
>> so it's safe to convert to a hook.
>
>  Actually, the toolchain is not always built, e.g. 'make host-foo' will not
> build it. Also if you haven't selected any target package, it won't be built.
> But that's a bit far-fetched I guess.

With "the toolchain is always built" I was speaking about when
$(TARGET_FINALIZE_HOOKS) are executed because those hooks are executed
only after $(TARGETS) are built.
The "toolchain" is always built, even when no target packages are
selected because in "toolchain/Config.in" there is:

# Invisible option that makes sure the toolchain package always gets
# built
config BR2_TOOLCHAIN
        bool
        default y

>  And anyway, the toolchain-eclipse-register target doesn't really depend on the
> toolchain - it just creates a file that refers to it. Right?
<snip>

Well, it seems to depend on it, it fails if the toolchain dependency is removed:

./support/scripts/eclipse-register-toolchain `readlink -f output`
i686-buildroot-linux-uclibc- "i686"
Cannot find the cross-compiler in the project directory
make: *** [toolchain-eclipse-register] Error 1


I will send two additional patches to convert those targets.

Best regards
-- 
Fabio Porcedda