[PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Athira Rajeev]

Add rule in new Makefile "tests/Makefile.tests" for running
shellcheck on shell test scripts. This automates below shellcheck
into the build.

	$ for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S warning $F; done

Condition for shellcheck is added in Makefile.perf to avoid build
breakage in the absence of shellcheck binary. Update Makefile.perf
to contain new rule for "SHELLCHECK_TEST" which is for making
shellcheck test as a dependency on perf binary.

Added "tests/Makefile.tests" to run shellcheck on shellscripts in
tests/shell. The make rule "SHLLCHECK_RUN" ensures that, every
time during make, shellcheck will be run only on modified files
during subsequent invocations. By this, if any newly added shell
scripts or fixes in existing scripts breaks coding/formatting
style, it will get captured during the perf build.

Example build failure by modifying probe_vfs_getname.sh in tests/shell:

	In tests/shell/probe_vfs_getname.sh line 8:
	. $(dirname $0)/lib/probe.sh
	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.

	For more information:
	  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
	make[3]: *** [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: tests/shell/.probe_vfs_getname.sh.shellcheck_log] Error 1
	make[2]: *** [Makefile.perf:686: SHELLCHECK_TEST] Error 2
	make[2]: *** Waiting for unfinished jobs....
	make[1]: *** [Makefile.perf:244: sub-make] Error 2
	make: *** [Makefile:70: all] Error 2

Here, like other files which gets created during
compilation (ex: .builtin-bench.o.cmd or .perf.o.cmd ),
create .shellcheck_log also as a hidden file.
Example: tests/shell/.probe_vfs_getname.sh.shellcheck_log
shellcheck is re-run if any of the script gets modified
based on its dependency of this log file.

After this, for testing, changed "tests/shell/trace+probe_vfs_getname.sh" to
break shellcheck format. In the next make run, it is
also captured:

	In tests/shell/probe_vfs_getname.sh line 8:
	. $(dirname $0)/lib/probe.sh
	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.

	For more information:
	  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
	make[3]: *** [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: tests/shell/.probe_vfs_getname.sh.shellcheck_log] Error 1
	make[3]: *** Waiting for unfinished jobs....

	In tests/shell/trace+probe_vfs_getname.sh line 14:
	. $(dirname $0)/lib/probe.sh
	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.

	For more information:
	  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
	make[3]: *** [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: tests/shell/.trace+probe_vfs_getname.sh.shellcheck_log] Error 1
	make[2]: *** [Makefile.perf:686: SHELLCHECK_TEST] Error 2
	make[2]: *** Waiting for unfinished jobs....
	make[1]: *** [Makefile.perf:244: sub-make] Error 2
	make: *** [Makefile:70: all] Error 2

Failure log can be found in the stdout of make itself.

This is reported at build time. To be able to go ahead with
the build or disable shellcheck even though it is known that
some test is broken, add a "NO_SHELLCHECK" option. Example:

  make NO_SHELLCHECK=1

	  INSTALL libsubcmd_headers
	  INSTALL libsymbol_headers
	  INSTALL libapi_headers
	  INSTALL libperf_headers
	  INSTALL libbpf_headers
	  LINK    perf

Note:
This is tested on RHEL and also SLES. Use below check:
"$(shell which shellcheck 2> /dev/null)" to look for presence
of shellcheck binary. The approach "shell command -v" is not
used here. In some of the distros(RHEL), command is available
as executable file (/usr/bin/command). But in some distros(SLES),
it is a shell builtin and not available as executable file.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
changelog:
v3 -> v4:
Addressed review comments from James Clark.
- Made the shellcheck errors to be reported in make output
  itself during make like any other build error.
- Removed creating .dep files. Instead use the log file
to determine whether shellcheck has to be re-run when
there is a change in source file.
- Change log file to have suffix as shellcheck_log so
as to differentiate it from test execution log.
- Also like other files which gets created during
compilation, example, .builtin-bench.o.cmd or .perf.o.cmd,
create .shellcheck_log as hidden file.
Example: tests/shell/.buildid.sh.shellcheck_log
- Initial version used "command -v shellcheck" to check
presence of shellcheck. But while testing SLES, hit an
issue with using "command". In RHEL, /usr/bin/command
is available as part of bash rpm
# rpm -qf /usr/bin/command 
bash-5.1.8-6.el9_1.ppc64le

But in SLES ( tested in SLES 15 SP4 ), this is not
available as an executable file, instead it is only
a shell builtin. This resulted in below fail:
make[2]: command: Command not found

So use "which shellcheck" to check for presence of
shellcheck binary

v2 -> v3:
Add option "NO_SHELLCHECK". This will allow to go ahead
with the build or disable shellcheck even though it is
known that some test is broken

v1 -> v2:
Version1 had shellcheck in feature check which is not
required since shellcheck is already a binary. Presence
of binary can be checked using:
$(shell command -v shellcheck)
Addressed these changes as suggested by Namhyung in V2
Feature test logic is removed in V2. Also added example
for build breakage when shellcheck fails in commit message

 tools/perf/Makefile.perf        | 21 ++++++++++++++++++++-
 tools/perf/tests/Makefile.tests | 22 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/tests/Makefile.tests

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index d80dcaa5a1e3..824cbc0af7d7 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -134,6 +134,8 @@ include ../scripts/utilities.mak
 #	x86 instruction decoder - new instructions test
 #
 # Define GEN_VMLINUX_H to generate vmlinux.h from the BTF.
+#
+# Define NO_SHELLCHECK if you do not want to run shellcheck during build
 
 # As per kernel Makefile, avoid funny character set dependencies
 unexport LC_ALL
@@ -671,7 +673,23 @@ $(PERF_IN): prepare FORCE
 $(PMU_EVENTS_IN): FORCE prepare
 	$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events obj=pmu-events
 
-$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN)
+# Runs shellcheck on perf test shell scripts
+
+SHELLCHECK := $(shell which shellcheck 2> /dev/null)
+
+ifeq ($(NO_SHELLCHECK),1)
+SHELLCHECK :=
+endif
+
+ifneq ($(SHELLCHECK),)
+SHELLCHECK_TEST: FORCE prepare
+	$(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests
+else
+SHELLCHECK_TEST:
+	@:
+endif
+
+$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) SHELLCHECK_TEST
 	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) \
 		$(PERF_IN) $(PMU_EVENTS_IN) $(LIBS) -o $@
 
@@ -1134,6 +1152,7 @@ bpf-skel-clean:
 	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
 
 clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
+	$(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
 	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
 	$(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
 	$(Q)$(RM) $(OUTPUT).config-detected
diff --git a/tools/perf/tests/Makefile.tests b/tools/perf/tests/Makefile.tests
new file mode 100644
index 000000000000..fdaca5f7a946
--- /dev/null
+++ b/tools/perf/tests/Makefile.tests
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0
+# Athira Rajeev <atrajeev@linux.vnet.ibm.com>, 2023
+
+PROGS := $(shell find tests/shell -perm -o=x -type f -name '*.sh')
+FILE_NAME := $(notdir $(PROGS))
+FILE_NAME := $(FILE_NAME:%=.%)
+LOGS := $(join $(dir $(PROGS)),$(FILE_NAME))
+LOGS := $(LOGS:%=%.shellcheck_log)
+
+.PHONY: all
+all: SHELLCHECK_RUN
+	@:
+
+SHELLCHECK_RUN: $(LOGS)
+
+.%.shellcheck_log: %
+	$(call rule_mkdir)
+	$(Q)$(call frecho-cmd,test)@shellcheck -S warning "$<" > $@ || (cat $@ && rm $@ && false)
+
+clean:
+	$(eval log_files := $(shell find . -name '.*.shellcheck_log'))
+	@rm -rf $(log_files)
-- 
2.35.3

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[James Clark]

On 23/11/2023 16:02, Athira Rajeev wrote:
> Add rule in new Makefile "tests/Makefile.tests" for running
> shellcheck on shell test scripts. This automates below shellcheck
> into the build.
> 

Seems to work really well. I also tested it on Ubuntu, and checked
NO_SHELLCHECK, cleaning and with and without shellcheck installed etc.

Reviewed-by: James Clark <james.clark@arm.com>

> 	$ for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck -S warning $F; done
> 
> Condition for shellcheck is added in Makefile.perf to avoid build
> breakage in the absence of shellcheck binary. Update Makefile.perf
> to contain new rule for "SHELLCHECK_TEST" which is for making
> shellcheck test as a dependency on perf binary.
> 
> Added "tests/Makefile.tests" to run shellcheck on shellscripts in
> tests/shell. The make rule "SHLLCHECK_RUN" ensures that, every
> time during make, shellcheck will be run only on modified files
> during subsequent invocations. By this, if any newly added shell
> scripts or fixes in existing scripts breaks coding/formatting
> style, it will get captured during the perf build.
> 
> Example build failure by modifying probe_vfs_getname.sh in tests/shell:
> 
> 	In tests/shell/probe_vfs_getname.sh line 8:
> 	. $(dirname $0)/lib/probe.sh
> 	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.
> 
> 	For more information:
> 	  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
> 	make[3]: *** [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: tests/shell/.probe_vfs_getname.sh.shellcheck_log] Error 1
> 	make[2]: *** [Makefile.perf:686: SHELLCHECK_TEST] Error 2
> 	make[2]: *** Waiting for unfinished jobs....
> 	make[1]: *** [Makefile.perf:244: sub-make] Error 2
> 	make: *** [Makefile:70: all] Error 2
> 
> Here, like other files which gets created during
> compilation (ex: .builtin-bench.o.cmd or .perf.o.cmd ),
> create .shellcheck_log also as a hidden file.
> Example: tests/shell/.probe_vfs_getname.sh.shellcheck_log
> shellcheck is re-run if any of the script gets modified
> based on its dependency of this log file.
> 
> After this, for testing, changed "tests/shell/trace+probe_vfs_getname.sh" to
> break shellcheck format. In the next make run, it is
> also captured:
> 
> 	In tests/shell/probe_vfs_getname.sh line 8:
> 	. $(dirname $0)/lib/probe.sh
> 	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.
> 
> 	For more information:
> 	  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
> 	make[3]: *** [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: tests/shell/.probe_vfs_getname.sh.shellcheck_log] Error 1
> 	make[3]: *** Waiting for unfinished jobs....
> 
> 	In tests/shell/trace+probe_vfs_getname.sh line 14:
> 	. $(dirname $0)/lib/probe.sh
> 	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.
> 
> 	For more information:
> 	  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
> 	make[3]: *** [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: tests/shell/.trace+probe_vfs_getname.sh.shellcheck_log] Error 1
> 	make[2]: *** [Makefile.perf:686: SHELLCHECK_TEST] Error 2
> 	make[2]: *** Waiting for unfinished jobs....
> 	make[1]: *** [Makefile.perf:244: sub-make] Error 2
> 	make: *** [Makefile:70: all] Error 2
> 
> Failure log can be found in the stdout of make itself.
> 
> This is reported at build time. To be able to go ahead with
> the build or disable shellcheck even though it is known that
> some test is broken, add a "NO_SHELLCHECK" option. Example:
> 
>   make NO_SHELLCHECK=1
> 
> 	  INSTALL libsubcmd_headers
> 	  INSTALL libsymbol_headers
> 	  INSTALL libapi_headers
> 	  INSTALL libperf_headers
> 	  INSTALL libbpf_headers
> 	  LINK    perf
> 
> Note:
> This is tested on RHEL and also SLES. Use below check:
> "$(shell which shellcheck 2> /dev/null)" to look for presence
> of shellcheck binary. The approach "shell command -v" is not
> used here. In some of the distros(RHEL), command is available
> as executable file (/usr/bin/command). But in some distros(SLES),
> it is a shell builtin and not available as executable file.
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> changelog:
> v3 -> v4:
> Addressed review comments from James Clark.
> - Made the shellcheck errors to be reported in make output
>   itself during make like any other build error.
> - Removed creating .dep files. Instead use the log file
> to determine whether shellcheck has to be re-run when
> there is a change in source file.
> - Change log file to have suffix as shellcheck_log so
> as to differentiate it from test execution log.
> - Also like other files which gets created during
> compilation, example, .builtin-bench.o.cmd or .perf.o.cmd,
> create .shellcheck_log as hidden file.
> Example: tests/shell/.buildid.sh.shellcheck_log
> - Initial version used "command -v shellcheck" to check
> presence of shellcheck. But while testing SLES, hit an
> issue with using "command". In RHEL, /usr/bin/command
> is available as part of bash rpm
> # rpm -qf /usr/bin/command 
> bash-5.1.8-6.el9_1.ppc64le
> 
> But in SLES ( tested in SLES 15 SP4 ), this is not
> available as an executable file, instead it is only
> a shell builtin. This resulted in below fail:
> make[2]: command: Command not found
> 
> So use "which shellcheck" to check for presence of
> shellcheck binary
> 
> v2 -> v3:
> Add option "NO_SHELLCHECK". This will allow to go ahead
> with the build or disable shellcheck even though it is
> known that some test is broken
> 
> v1 -> v2:
> Version1 had shellcheck in feature check which is not
> required since shellcheck is already a binary. Presence
> of binary can be checked using:
> $(shell command -v shellcheck)
> Addressed these changes as suggested by Namhyung in V2
> Feature test logic is removed in V2. Also added example
> for build breakage when shellcheck fails in commit message
> 
>  tools/perf/Makefile.perf        | 21 ++++++++++++++++++++-
>  tools/perf/tests/Makefile.tests | 22 ++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 1 deletion(-)
>  create mode 100644 tools/perf/tests/Makefile.tests
> 
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index d80dcaa5a1e3..824cbc0af7d7 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -134,6 +134,8 @@ include ../scripts/utilities.mak
>  #	x86 instruction decoder - new instructions test
>  #
>  # Define GEN_VMLINUX_H to generate vmlinux.h from the BTF.
> +#
> +# Define NO_SHELLCHECK if you do not want to run shellcheck during build
>  
>  # As per kernel Makefile, avoid funny character set dependencies
>  unexport LC_ALL
> @@ -671,7 +673,23 @@ $(PERF_IN): prepare FORCE
>  $(PMU_EVENTS_IN): FORCE prepare
>  	$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events obj=pmu-events
>  
> -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN)
> +# Runs shellcheck on perf test shell scripts
> +
> +SHELLCHECK := $(shell which shellcheck 2> /dev/null)
> +
> +ifeq ($(NO_SHELLCHECK),1)
> +SHELLCHECK :=
> +endif
> +
> +ifneq ($(SHELLCHECK),)
> +SHELLCHECK_TEST: FORCE prepare
> +	$(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests
> +else
> +SHELLCHECK_TEST:
> +	@:
> +endif
> +
> +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) SHELLCHECK_TEST
>  	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) \
>  		$(PERF_IN) $(PMU_EVENTS_IN) $(LIBS) -o $@
>  
> @@ -1134,6 +1152,7 @@ bpf-skel-clean:
>  	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
>  
>  clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
> +	$(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
>  	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
>  	$(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
>  	$(Q)$(RM) $(OUTPUT).config-detected
> diff --git a/tools/perf/tests/Makefile.tests b/tools/perf/tests/Makefile.tests
> new file mode 100644
> index 000000000000..fdaca5f7a946
> --- /dev/null
> +++ b/tools/perf/tests/Makefile.tests
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Athira Rajeev <atrajeev@linux.vnet.ibm.com>, 2023
> +
> +PROGS := $(shell find tests/shell -perm -o=x -type f -name '*.sh')
> +FILE_NAME := $(notdir $(PROGS))
> +FILE_NAME := $(FILE_NAME:%=.%)
> +LOGS := $(join $(dir $(PROGS)),$(FILE_NAME))
> +LOGS := $(LOGS:%=%.shellcheck_log)
> +
> +.PHONY: all
> +all: SHELLCHECK_RUN
> +	@:
> +
> +SHELLCHECK_RUN: $(LOGS)
> +
> +.%.shellcheck_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call frecho-cmd,test)@shellcheck -S warning "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +clean:
> +	$(eval log_files := $(shell find . -name '.*.shellcheck_log'))
> +	@rm -rf $(log_files)

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Arnaldo Carvalho de Melo]

Em Mon, Nov 27, 2023 at 11:12:57AM +0000, James Clark escreveu:
> On 23/11/2023 16:02, Athira Rajeev wrote:
> > Add rule in new Makefile "tests/Makefile.tests" for running
> > shellcheck on shell test scripts. This automates below shellcheck
> > into the build.

> Seems to work really well. I also tested it on Ubuntu, and checked
> NO_SHELLCHECK, cleaning and with and without shellcheck installed etc.
 
> Reviewed-by: James Clark <james.clark@arm.com>

Tested on Fedora 38, works as advertised, applied.
 
- Arnaldo

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Athira Rajeev]

> On 27-Nov-2023, at 8:21 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> Em Mon, Nov 27, 2023 at 11:12:57AM +0000, James Clark escreveu:
>> On 23/11/2023 16:02, Athira Rajeev wrote:
>>> Add rule in new Makefile "tests/Makefile.tests" for running
>>> shellcheck on shell test scripts. This automates below shellcheck
>>> into the build.
> 
>> Seems to work really well. I also tested it on Ubuntu, and checked
>> NO_SHELLCHECK, cleaning and with and without shellcheck installed etc.
> 
>> Reviewed-by: James Clark <james.clark@arm.com>
> 
> Tested on Fedora 38, works as advertised, applied.
> 
> - Arnaldo

Hi James, Arnaldo

Thanks for testing the patch and comments.

Athira Rajeev

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Arnaldo Carvalho de Melo]

Em Mon, Nov 27, 2023 at 11:12:57AM +0000, James Clark escreveu:
> On 23/11/2023 16:02, Athira Rajeev wrote:
> > --- a/tools/perf/Makefile.perf
> > @@ -1134,6 +1152,7 @@ bpf-skel-clean:
> >  	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
> >  
> >  clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
> > +	$(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
> >  	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
> >  	$(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
> >  	$(Q)$(RM) $(OUTPUT).config-detected

While merging perf-tools-next with torvalds/master I noticed that maybe
we better have the above added line as:

+   $(call QUIET_CLEAN, tests) $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean

No?

Anyway I'm merging as-is, but it just hit my eye while merging,

- Arnaldo

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Ian Rogers]

On Tue, Dec 5, 2023 at 1:50 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Mon, Nov 27, 2023 at 11:12:57AM +0000, James Clark escreveu:
> > On 23/11/2023 16:02, Athira Rajeev wrote:
> > > --- a/tools/perf/Makefile.perf
> > > @@ -1134,6 +1152,7 @@ bpf-skel-clean:
> > >     $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
> > >
> > >  clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
> > > +   $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
> > >     $(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
> > >     $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
> > >     $(Q)$(RM) $(OUTPUT).config-detected
>
> While merging perf-tools-next with torvalds/master I noticed that maybe
> we better have the above added line as:
>
> +   $(call QUIET_CLEAN, tests) $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
>
> No?
>
> Anyway I'm merging as-is, but it just hit my eye while merging,
>
> - Arnaldo

Makefile.tests was removed in these recent patches adding support for
the OUTPUT directory:
https://lore.kernel.org/lkml/9C33887F-8A88-4973-8593-7936E36AFCE1@linux.vnet.ibm.com/

Thanks,
Ian

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Athira Rajeev]

> On 06-Dec-2023, at 3:20 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> Em Mon, Nov 27, 2023 at 11:12:57AM +0000, James Clark escreveu:
>> On 23/11/2023 16:02, Athira Rajeev wrote:
>>> --- a/tools/perf/Makefile.perf
>>> @@ -1134,6 +1152,7 @@ bpf-skel-clean:
>>> $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
>>> 
>>> clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
>>> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
>>> $(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
>>> $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
>>> $(Q)$(RM) $(OUTPUT).config-detected
> 
> While merging perf-tools-next with torvalds/master I noticed that maybe
> we better have the above added line as:
> 
> +   $(call QUIET_CLEAN, tests) $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
> 
> No?
> 
> Anyway I'm merging as-is, but it just hit my eye while merging,
> 
> - Arnaldo
Hi Arnaldo

As Ian pointed we removed Makefile.tests as part of :
https://lore.kernel.org/lkml/20231129213428.2227448-1-irogers@google.com/

Thanks
Athira

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Arnaldo Carvalho de Melo]

Em Tue, Dec 05, 2023 at 02:09:01PM -0800, Ian Rogers escreveu:
> On Tue, Dec 5, 2023 at 1:50 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> > Em Mon, Nov 27, 2023 at 11:12:57AM +0000, James Clark escreveu:
> > > On 23/11/2023 16:02, Athira Rajeev wrote:
> > > > --- a/tools/perf/Makefile.perf
> > > > @@ -1134,6 +1152,7 @@ bpf-skel-clean:
> > > >     $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
> > > >
> > > >  clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
> > > > +   $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
> > > >     $(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
> > > >     $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
> > > >     $(Q)$(RM) $(OUTPUT).config-detected
> >
> > While merging perf-tools-next with torvalds/master I noticed that maybe
> > we better have the above added line as:
> >
> > +   $(call QUIET_CLEAN, tests) $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
> >
> > No?
> >
> > Anyway I'm merging as-is, but it just hit my eye while merging,
> >
> > - Arnaldo
> 
> Makefile.tests was removed in these recent patches adding support for
> the OUTPUT directory:
> https://lore.kernel.org/lkml/9C33887F-8A88-4973-8593-7936E36AFCE1@linux.vnet.ibm.com/

Right, I made a mistake and was doing the merge on a different branch,
now that I tried it on my latest perf-tools-next local branch all is
well and the other merge conflict gets auto-resolved
(arm64-sysreg-defs-clean stuff in tools/perf/Makefile.perf "clean" target).

Thanks for checking,

- Arnaldo

Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf

[Arnaldo Carvalho de Melo]

Em Wed, Dec 06, 2023 at 10:15:06AM +0530, Athira Rajeev escreveu:
> 
> 
> > On 06-Dec-2023, at 3:20 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > 
> > Em Mon, Nov 27, 2023 at 11:12:57AM +0000, James Clark escreveu:
> >> On 23/11/2023 16:02, Athira Rajeev wrote:
> >>> --- a/tools/perf/Makefile.perf
> >>> @@ -1134,6 +1152,7 @@ bpf-skel-clean:
> >>> $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
> >>> 
> >>> clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
> >>> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
> >>> $(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
> >>> $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
> >>> $(Q)$(RM) $(OUTPUT).config-detected
> > 
> > While merging perf-tools-next with torvalds/master I noticed that maybe
> > we better have the above added line as:
> > 
> > +   $(call QUIET_CLEAN, tests) $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean
> > 
> > No?
> > 
> > Anyway I'm merging as-is, but it just hit my eye while merging,
> > 
> > - Arnaldo
> Hi Arnaldo
> 
> As Ian pointed we removed Makefile.tests as part of :
> https://lore.kernel.org/lkml/20231129213428.2227448-1-irogers@google.com/

Right, thanks for checking, see my reply to Ian for further details.

- Arnaldo