diff mbox

build log: add message when starting target-finalize target

Message ID 3230f72b2606158b2575.1399904367@argentina
State Accepted
Commit 6f6a2cfd819dd96e04d942e24612b7ada75773ed
Headers show

Commit Message

Thomas De Schampheleire May 12, 2014, 2:19 p.m. UTC
The target-finalize target does a large number of actions (removing
unnecessary files, stripping objects, etc.) but does not have a header. This
makes it seem that all these actions are done as part of the last action
before target-finalize, for example:
    >>> makedevs undefined Installing to target

To make a clear distinction, add a message to the beginning of
target-finalize:
    >>>   Finalizing target directory

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
 Makefile |  1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Note: in system/system.mk there are a number of smaller items that are
executed before target-finalize but do not have any message associated with
them. This includes the adding to securetty, the setting of issue and
hostname, ... Adding a message to each of these small targets is probably
too much, but if we could find an elegant way to add a message to the group
of system/system.mk, that would be nice.
Adding an initial target in system/system.mk just printing a message would
work, but only if there is no top-level parallel build.

Comments

Arnout Vandecappelle May 12, 2014, 4:07 p.m. UTC | #1
On 12/05/14 16:19, Thomas De Schampheleire wrote:
> The target-finalize target does a large number of actions (removing
> unnecessary files, stripping objects, etc.) but does not have a header. This
> makes it seem that all these actions are done as part of the last action
> before target-finalize, for example:
>     >>> makedevs undefined Installing to target
> 
> To make a clear distinction, add a message to the beginning of
> target-finalize:
>     >>>   Finalizing target directory
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

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

> 
> ---
>  Makefile |  1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> Note: in system/system.mk there are a number of smaller items that are
> executed before target-finalize but do not have any message associated with
> them. This includes the adding to securetty, the setting of issue and
> hostname, ... Adding a message to each of these small targets is probably
> too much, but if we could find an elegant way to add a message to the group
> of system/system.mk, that would be nice.

 IMHO those targets should be converted into command variables that are expanded
inside the target-finalize rule. I think that that makes it much more
understandable what this target is doing.

 In fact, Fabio has a patch series doing exactly that. Fabio, if you would
resend your series, I suggest to include this patch in it as well. Although most
likely this patch will still be committed for 2014.05 while Fabio's series is
too risky at this point.

 Regards,
 Arnout

> Adding an initial target in system/system.mk just printing a message would
> work, but only if there is no top-level parallel build.
> 
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -533,6 +533,7 @@ endif
>  $(TARGETS_ROOTFS): target-finalize
>  
>  target-finalize: $(TARGETS)
> +	@$(call MESSAGE,"Finalizing target directory")
>  	$(TARGET_PURGE_LOCALES)
>  	rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \
>  		$(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 
>
Peter Korsgaard May 12, 2014, 8:23 p.m. UTC | #2
>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

 > The target-finalize target does a large number of actions (removing
 > unnecessary files, stripping objects, etc.) but does not have a header. This
 > makes it seem that all these actions are done as part of the last action
 > before target-finalize, for example:
 >>>> makedevs undefined Installing to target

 > To make a clear distinction, add a message to the beginning of
 > target-finalize:
 >>>> Finalizing target directory

 > Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

Committed, thanks.
Fabio Porcedda May 13, 2014, 8:55 a.m. UTC | #3
Hi all,

On Mon, May 12, 2014 at 6:07 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 12/05/14 16:19, Thomas De Schampheleire wrote:
>> The target-finalize target does a large number of actions (removing
>> unnecessary files, stripping objects, etc.) but does not have a header. This
>> makes it seem that all these actions are done as part of the last action
>> before target-finalize, for example:
>>     >>> makedevs undefined Installing to target
>>
>> To make a clear distinction, add a message to the beginning of
>> target-finalize:
>>     >>>   Finalizing target directory
>>
>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
>>
>> ---
>>  Makefile |  1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> Note: in system/system.mk there are a number of smaller items that are
>> executed before target-finalize but do not have any message associated with
>> them. This includes the adding to securetty, the setting of issue and
>> hostname, ... Adding a message to each of these small targets is probably
>> too much, but if we could find an elegant way to add a message to the group
>> of system/system.mk, that would be nice.
>
>  IMHO those targets should be converted into command variables that are expanded
> inside the target-finalize rule. I think that that makes it much more
> understandable what this target is doing.
>
>  In fact, Fabio has a patch series doing exactly that. Fabio, if you would
> resend your series, I suggest to include this patch in it as well. Although most
> likely this patch will still be committed for 2014.05 while Fabio's series is
> too risky at this point.

I've sent a patch for converting taget-purgelocales that was already merged:

http://git.buildroot.net/buildroot/commit/?id=58d82dda1a3df4bfe00bc0c2c4257840030fff95

I've not already cooked a patch to convert system.mk targets because i
was not sure it was required or useful, but after reading your
thoughts i understands that is a useful thing so I will cook a patch
for converting those targets.

Regards
diff mbox

Patch

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -533,6 +533,7 @@  endif
 $(TARGETS_ROOTFS): target-finalize
 
 target-finalize: $(TARGETS)
+	@$(call MESSAGE,"Finalizing target directory")
 	$(TARGET_PURGE_LOCALES)
 	rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \
 		$(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \