diff mbox

[RFC,3/3] kbuild: In quiet mode, print the full command line if it fails

Message ID 20180712221730.GK14131@decadent.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Hutchings July 12, 2018, 10:17 p.m. UTC
In the $(run-cmd) macro, add a trap on EXIT that prints the
full command line.  Remove the trap after running the command(s)
successfully.

(A more straightforward approach would be to use "if" or "||" to test
for failure, but that doesn't work.  Some command lines given to
$(run-cmd) have multiple commands separated by semi-colons, and the
caller must run "set -e" to enable exit-on-error.  Testing the result
of such a command line, even if it is probably grouped and run in a
sub-shell, inhibits exit-on-error and would cause some errors to be
ignored.)

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 scripts/Kbuild.include | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Masahiro Yamada July 18, 2018, 5:26 a.m. UTC | #1
2018-07-13 7:17 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> In the $(run-cmd) macro, add a trap on EXIT that prints the
> full command line.  Remove the trap after running the command(s)
> successfully.
>
> (A more straightforward approach would be to use "if" or "||" to test
> for failure, but that doesn't work.  Some command lines given to
> $(run-cmd) have multiple commands separated by semi-colons, and the
> caller must run "set -e" to enable exit-on-error.  Testing the result
> of such a command line, even if it is probably grouped and run in a
> sub-shell, inhibits exit-on-error and would cause some errors to be
> ignored.)
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

OK, this seems useful to me.


> ---
>  scripts/Kbuild.include | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 8778ae4a3476..c2525aaa36ac 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -224,7 +224,10 @@ flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o)))
>  echo-cmd = $(if $($(quiet)cmd_$(1)),\
>         echo '  $(call escsq,$($(quiet)cmd_$(1)))$(echo-why)';)
>
> -run-cmd = $(echo-cmd) $(cmd_$(1))
> +trap-cmd-failed = trap 'test $$? = 0 || echo Failed command: '\''$(call escsq,$(call escsq,$(cmd_$(1))))'\' EXIT
> +untrap-cmd-failed = trap - EXIT


Why $(call escsq,...) twice?


I think
... echo Failed command: '\''$(call escsq,$(cmd_$(1)))'\' EXIT
is enough.



I see another problem for this.
Make cannot detect the failure of $(call cmd,...)



--------------(sample code)---------------

extra-y := foo.txt

quiet_cmd_always_fail = FAIL    $@
      cmd_always_fail = /bin/false

$(obj)/foo.txt:
        $(call cmd,always_fail)

------------(sample code end)-------------



This should terminate the build.


With your series, this always succeeds
because it returns the exit code of 'trap - EXIT', which is 0.


Maybe,
$(cmd_$(1)) && $(untrap-cmd-failed)
?







> +
> +run-cmd = $(echo-cmd) $(if $(cmd_$(1)), $(if $(quiet), $(trap-cmd-failed); $(cmd_$(1)); $(untrap-cmd-failed), $(cmd_$(1))))
>
>  # printing commands
>  cmd = @$(run-cmd)
diff mbox

Patch

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 8778ae4a3476..c2525aaa36ac 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -224,7 +224,10 @@  flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o)))
 echo-cmd = $(if $($(quiet)cmd_$(1)),\
 	echo '  $(call escsq,$($(quiet)cmd_$(1)))$(echo-why)';)
 
-run-cmd = $(echo-cmd) $(cmd_$(1))
+trap-cmd-failed = trap 'test $$? = 0 || echo Failed command: '\''$(call escsq,$(call escsq,$(cmd_$(1))))'\' EXIT
+untrap-cmd-failed = trap - EXIT
+
+run-cmd = $(echo-cmd) $(if $(cmd_$(1)), $(if $(quiet), $(trap-cmd-failed); $(cmd_$(1)); $(untrap-cmd-failed), $(cmd_$(1))))
 
 # printing commands
 cmd = @$(run-cmd)