Add TARGET_FINALIZE_HOOKS

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 <[email protected]>

---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Makefile +3 -2

index b576267..9de4806 100644
@@ -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

Quoted 5 lines · 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.

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.

Quoted 11 lines · Arnout Vandecappelle

Quoted 5 lines · 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.

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).

Quoted 2 lines

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

Quoted 12 lines · Fabio Porcedda

Quoted 11 lines · Arnout Vandecappelle

Quoted 5 lines · 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.

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).

Quoted 6 lines

Quoted 2 lines

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) <[email protected]>

in that case.

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 <[email protected]>

system/system.mk +3 -1

+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) #~' \

system/system.mk +11 -27

+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

Quoted 7 lines · 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 <[email protected]>

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

One small remark:

[snip]

Quoted 13 lines

system/system.mk +8 -1

+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

system/system.mk TARGET_GENERIC_GETTY_BAUDRATE = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRAT · line 64

Quoted 4 lines

system/system.mk +4 -0

+     $(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.

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(-)

Makefile +0 -4

index 9de4806..c028cf8 100644
@@ -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 \

Quoted 3 lines · 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.

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.

Quoted 4 lines · Arnout Vandecappelle

Quoted 3 lines · 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.

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.

Quoted 5 lines

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

Quoted 2 lines

- 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.

Quoted 3 lines

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.

Quoted 2 lines

Also, you forgot you Sob.

Yes, thanks.

BR

Quoted 6 lines · Fabio Porcedda

Quoted 4 lines · Arnout Vandecappelle

Quoted 3 lines · 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.

Quoted 7 lines

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.

Quoted 7 lines

Quoted 6 lines

There are still two targets that don't:

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

Quoted 4 lines

I will do:

ifneq ($(GENERATE_LOCALE),)
TARGETS+=host-localedef

Spaces around +=

Otherwise it's perfect.

Quoted 9 lines

define GENERATE_LOCALE_HOOK
...
endif
TARGET_FINALIZE_HOOKS += GENERATE_LOCALE_HOOK
endif

Quoted 2 lines

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

Quoted 2 lines

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?