[PATCH v1 0/6] Mypy and pylint build support

[PATCH v1 0/6] Mypy and pylint build support

[Ian Rogers]

Support building perf checking the python code with mypy and/or
pylint. Currently there are too many errors to make this a default.

Shellcheck generates no output on success, so linking shellcheck files
doesn't cause `ld` to fail. Mypy and pylint generate output that will
break `ld` so change the Makefile.build to ignore test log files.

Address some initial mypy errors.

Ian Rogers (6):
  tools/build: Don't pass test log files to linker
  perf build: Rename TEST_LOGS to SHELL_TEST_LOGS
  perf build: Add mypy build tests
  perf build: Add pylint build tests
  perf test: Address attr.py mypy error
  perf python: Fix setup.py mypy errors

 tools/build/Makefile.build         |  6 +++++-
 tools/perf/Build                   | 32 +++++++++++++++++++++++++++---
 tools/perf/Makefile.perf           | 14 +++++++++++--
 tools/perf/arch/x86/Build          |  6 +++---
 tools/perf/arch/x86/tests/Build    |  6 +++---
 tools/perf/pmu-events/Build        | 25 ++++++++++++++++++++++-
 tools/perf/scripts/Build           | 26 ++++++++++++++++++++++++
 tools/perf/tests/Build             | 32 +++++++++++++++++++++++++++---
 tools/perf/tests/shell/lib/attr.py |  8 +-------
 tools/perf/trace/beauty/Build      |  6 +++---
 tools/perf/util/Build              | 31 ++++++++++++++++++++++++++---
 tools/perf/util/setup.py           | 10 ++++++++--
 12 files changed, 171 insertions(+), 31 deletions(-)

-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH v1 1/6] tools/build: Don't pass test log files to linker

[Ian Rogers]

Separate test log files from object files. Depend on test log output
but don't pass to the linker.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/build/Makefile.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/build/Makefile.build b/tools/build/Makefile.build
index 5fb3fb3d97e0..ffe988867703 100644
--- a/tools/build/Makefile.build
+++ b/tools/build/Makefile.build
@@ -149,6 +149,10 @@ objprefix    := $(subst ./,,$(OUTPUT)$(dir)/)
 obj-y        := $(addprefix $(objprefix),$(obj-y))
 subdir-obj-y := $(addprefix $(objprefix),$(subdir-obj-y))
 
+# Separate out test log files from real build objects.
+test-y       := $(filter %_log, $(obj-y))
+obj-y        := $(filter-out %_log, $(obj-y))
+
 # Final '$(obj)-in.o' object
 in-target := $(objprefix)$(obj)-in.o
 
@@ -159,7 +163,7 @@ $(subdir-y):
 
 $(sort $(subdir-obj-y)): $(subdir-y) ;
 
-$(in-target): $(obj-y) FORCE
+$(in-target): $(obj-y) $(test-y) FORCE
 	$(call rule_mkdir)
 	$(call if_changed,$(host)ld_multi)
 
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH v1 2/6] perf build: Rename TEST_LOGS to SHELL_TEST_LOGS

[Ian Rogers]

Rename TEST_LOGS to SHELL_TEST_LOGS as later changes will add more
kinds of test logs.
Minor comment tweak in Makefile.perf as more than just test shell
tests are checked.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Build                | 6 +++---
 tools/perf/Makefile.perf        | 2 +-
 tools/perf/arch/x86/Build       | 6 +++---
 tools/perf/arch/x86/tests/Build | 6 +++---
 tools/perf/tests/Build          | 6 +++---
 tools/perf/trace/beauty/Build   | 6 +++---
 tools/perf/util/Build           | 6 +++---
 7 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/tools/perf/Build b/tools/perf/Build
index 3e486f7df94b..5e385f370dd7 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -65,14 +65,14 @@ gtk-y += ui/gtk/
 
 ifdef SHELLCHECK
   SHELL_TESTS := $(wildcard *.sh)
-  TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
+  SHELL_TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
 else
   SHELL_TESTS :=
-  TEST_LOGS :=
+  SHELL_TEST_LOGS :=
 endif
 
 $(OUTPUT)%.shellcheck_log: %
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
-perf-y += $(TEST_LOGS)
+perf-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index b4dee7c20ed1..a9f2a9f6ebf0 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -260,7 +260,7 @@ else
   force_fixdep := $(config)
 endif
 
-# Runs shellcheck on perf test shell scripts
+# Runs shellcheck on perf shell scripts
 ifeq ($(NO_SHELLCHECK),1)
   SHELLCHECK :=
 else
diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
index 87d057491343..1d91c46ad031 100644
--- a/tools/perf/arch/x86/Build
+++ b/tools/perf/arch/x86/Build
@@ -3,14 +3,14 @@ perf-test-y += tests/
 
 ifdef SHELLCHECK
   SHELL_TESTS := entry/syscalls/syscalltbl.sh
-  TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
+  SHELL_TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
 else
   SHELL_TESTS :=
-  TEST_LOGS :=
+  SHELL_TEST_LOGS :=
 endif
 
 $(OUTPUT)%.shellcheck_log: %
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
-perf-test-y += $(TEST_LOGS)
+perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index 3227053f3355..86262c720857 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -13,14 +13,14 @@ perf-test-y += amd-ibs-via-core-pmu.o
 
 ifdef SHELLCHECK
   SHELL_TESTS := gen-insn-x86-dat.sh
-  TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
+  SHELL_TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
 else
   SHELL_TESTS :=
-  TEST_LOGS :=
+  SHELL_TEST_LOGS :=
 endif
 
 $(OUTPUT)%.shellcheck_log: %
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
-perf-test-y += $(TEST_LOGS)
+perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 03cbdf7c50a0..a51d28c7431c 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -80,14 +80,14 @@ perf-test-y += workloads/
 
 ifdef SHELLCHECK
   SHELL_TESTS := $(shell find tests/shell -executable -type f -name '*.sh')
-  TEST_LOGS := $(SHELL_TESTS:tests/shell/%=shell/%.shellcheck_log)
+  SHELL_TEST_LOGS := $(SHELL_TESTS:tests/shell/%=shell/%.shellcheck_log)
 else
   SHELL_TESTS :=
-  TEST_LOGS :=
+  SHELL_TEST_LOGS :=
 endif
 
 $(OUTPUT)%.shellcheck_log: %
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
-perf-test-y += $(TEST_LOGS)
+perf-test-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
index cb3c1399ff40..f50ebdc445b8 100644
--- a/tools/perf/trace/beauty/Build
+++ b/tools/perf/trace/beauty/Build
@@ -23,14 +23,14 @@ perf-y += tracepoints/
 
 ifdef SHELLCHECK
   SHELL_TESTS := $(wildcard trace/beauty/*.sh)
-  TEST_LOGS := $(SHELL_TESTS:trace/beauty/%=%.shellcheck_log)
+  SHELL_TEST_LOGS := $(SHELL_TESTS:trace/beauty/%=%.shellcheck_log)
 else
   SHELL_TESTS :=
-  TEST_LOGS :=
+  SHELL_TEST_LOGS :=
 endif
 
 $(OUTPUT)%.shellcheck_log: %
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
-perf-y += $(TEST_LOGS)
+perf-y += $(SHELL_TEST_LOGS)
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 1eedead5f2f2..18cd02ccd3ff 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -398,14 +398,14 @@ $(OUTPUT)util/list_sort.o: ../lib/list_sort.c FORCE
 
 ifdef SHELLCHECK
   SHELL_TESTS := generate-cmdlist.sh
-  TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
+  SHELL_TEST_LOGS := $(SHELL_TESTS:%=%.shellcheck_log)
 else
   SHELL_TESTS :=
-  TEST_LOGS :=
+  SHELL_TEST_LOGS :=
 endif
 
 $(OUTPUT)%.shellcheck_log: %
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
-perf-util-y += $(TEST_LOGS)
+perf-util-y += $(SHELL_TEST_LOGS)
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH v1 3/6] perf build: Add mypy build tests

[Ian Rogers]

If MYPY=1 is passed to the build then run mypy over python code in
perf. Unlike shellcheck this isn't default on as there are currently
too many errors.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Build            | 13 +++++++++++++
 tools/perf/Makefile.perf    |  7 ++++++-
 tools/perf/pmu-events/Build | 14 +++++++++++++-
 tools/perf/scripts/Build    | 13 +++++++++++++
 tools/perf/tests/Build      | 13 +++++++++++++
 tools/perf/util/Build       | 13 +++++++++++++
 6 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Build b/tools/perf/Build
index 5e385f370dd7..312914994c89 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -76,3 +76,16 @@ $(OUTPUT)%.shellcheck_log: %
 	$(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
 perf-y += $(SHELL_TEST_LOGS)
+
+ifdef MYPY
+  PY_TESTS := $(shell find python -type f -name '*.py')
+  MYPY_TEST_LOGS := $(PY_TESTS:python/%=python/%.mypy_log)
+else
+  MYPY_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.mypy_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-y += $(MYPY_TEST_LOGS)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index a9f2a9f6ebf0..333afe29b1e2 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -277,8 +277,13 @@ ifneq ($(SHELLCHECK),)
   endif
 endif
 
+# Runs mypy on perf python files
+ifeq ($(MYPY),1)
+  MYPY := $(shell which mypy 2> /dev/null)
+endif
+
 export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
-export HOSTCC HOSTLD HOSTAR HOSTCFLAGS SHELLCHECK
+export HOSTCC HOSTLD HOSTAR HOSTCFLAGS SHELLCHECK MYPY
 
 include $(srctree)/tools/build/Makefile.include
 
diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
index d941bc9d16e9..fc1dc810ffb6 100644
--- a/tools/perf/pmu-events/Build
+++ b/tools/perf/pmu-events/Build
@@ -41,7 +41,19 @@ $(EMPTY_PMU_EVENTS_TEST_LOG): $(EMPTY_PMU_EVENTS_C) $(TEST_EMPTY_PMU_EVENTS_C)
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,test)diff -u $^ 2> $@ || (cat $@ && false)
 
-$(PMU_EVENTS_C): $(JSON) $(JSON_TEST) $(JEVENTS_PY) $(METRIC_PY) $(METRIC_TEST_LOG) $(EMPTY_PMU_EVENTS_TEST_LOG)
+ifdef MYPY
+  PMU_EVENTS_PY_TESTS := $(wildcard *.py)
+  PMU_EVENTS_MYPY_TEST_LOGS := $(JEVENTS_PY_TESTS:%=%.mypy_log)
+else
+  PMU_EVENTS_MYPY_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.mypy_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
+
+$(PMU_EVENTS_C): $(JSON) $(JSON_TEST) $(JEVENTS_PY) $(METRIC_PY) $(METRIC_TEST_LOG) \
+    $(EMPTY_PMU_EVENTS_TEST_LOG) $(PMU_EVENTS_MYPY_TEST_LOGS)
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,gen)$(PYTHON) $(JEVENTS_PY) $(JEVENTS_ARCH) $(JEVENTS_MODEL) pmu-events/arch $@
 endif
diff --git a/tools/perf/scripts/Build b/tools/perf/scripts/Build
index 46f0c6f76dbf..a5350dc6ac50 100644
--- a/tools/perf/scripts/Build
+++ b/tools/perf/scripts/Build
@@ -2,3 +2,16 @@ ifeq ($(CONFIG_LIBTRACEEVENT),y)
   perf-util-$(CONFIG_LIBPERL)   += perl/Perf-Trace-Util/
 endif
 perf-util-$(CONFIG_LIBPYTHON) += python/Perf-Trace-Util/
+
+ifdef MYPY
+  PY_TESTS := $(shell find python -type f -name '*.py')
+  MYPY_TEST_LOGS := $(PY_TESTS:python/%=python/%.mypy_log)
+else
+  MYPY_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.mypy_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-y += $(MYPY_TEST_LOGS)
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index a51d28c7431c..4d7c38a2bf6c 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -91,3 +91,16 @@ $(OUTPUT)%.shellcheck_log: %
 	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
 perf-test-y += $(SHELL_TEST_LOGS)
+
+ifdef MYPY
+  PY_TESTS := $(shell find tests/shell -type f -name '*.py')
+  MYPY_TEST_LOGS := $(PY_TESTS:tests/shell/%=shell/%.mypy_log)
+else
+  MYPY_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.mypy_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-test-y += $(MYPY_TEST_LOGS)
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 18cd02ccd3ff..3baa1f41502d 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -409,3 +409,16 @@ $(OUTPUT)%.shellcheck_log: %
 	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
 
 perf-util-y += $(SHELL_TEST_LOGS)
+
+PY_TESTS := setup.py
+ifdef MYPY
+  MYPY_TEST_LOGS := $(PY_TESTS:%=%.mypy_log)
+else
+  MYPY_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.mypy_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-util-y += $(MYPY_TEST_LOGS)
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH v1 4/6] perf build: Add pylint build tests

[Ian Rogers]

If PYLINT=1 is passed to the build then run pylint over python code in
perf. Unlike shellcheck this isn't default on as there are currently
too many errors.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Build            | 13 +++++++++++++
 tools/perf/Makefile.perf    |  7 ++++++-
 tools/perf/pmu-events/Build | 13 ++++++++++++-
 tools/perf/scripts/Build    | 13 +++++++++++++
 tools/perf/tests/Build      | 13 +++++++++++++
 tools/perf/util/Build       | 12 ++++++++++++
 6 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Build b/tools/perf/Build
index 312914994c89..06107f1e1d42 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -89,3 +89,16 @@ $(OUTPUT)%.mypy_log: %
 	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
 
 perf-y += $(MYPY_TEST_LOGS)
+
+ifdef PYLINT
+  PY_TESTS := $(shell find python -type f -name '*.py')
+  PYLINT_TEST_LOGS := $(PY_TESTS:python/%=python/%.pylint_log)
+else
+  PYLINT_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.pylint_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-y += $(PYLINT_TEST_LOGS)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 333afe29b1e2..6b5b420d794a 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -282,8 +282,13 @@ ifeq ($(MYPY),1)
   MYPY := $(shell which mypy 2> /dev/null)
 endif
 
+# Runs pylint on perf python files
+ifeq ($(PYLINT),1)
+  PYLINT := $(shell which pylint 2> /dev/null)
+endif
+
 export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
-export HOSTCC HOSTLD HOSTAR HOSTCFLAGS SHELLCHECK MYPY
+export HOSTCC HOSTLD HOSTAR HOSTCFLAGS SHELLCHECK MYPY PYLINT
 
 include $(srctree)/tools/build/Makefile.include
 
diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
index fc1dc810ffb6..32f387d48908 100644
--- a/tools/perf/pmu-events/Build
+++ b/tools/perf/pmu-events/Build
@@ -52,8 +52,19 @@ $(OUTPUT)%.mypy_log: %
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
 
+ifdef PYLINT
+  PMU_EVENTS_PY_TESTS := $(wildcard *.py)
+  PMU_EVENTS_PYLINT_TEST_LOGS := $(JEVENTS_PY_TESTS:%=%.pylint_log)
+else
+  PMU_EVENTS_PYLINT_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.pylint_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
+
 $(PMU_EVENTS_C): $(JSON) $(JSON_TEST) $(JEVENTS_PY) $(METRIC_PY) $(METRIC_TEST_LOG) \
-    $(EMPTY_PMU_EVENTS_TEST_LOG) $(PMU_EVENTS_MYPY_TEST_LOGS)
+    $(EMPTY_PMU_EVENTS_TEST_LOG) $(PMU_EVENTS_MYPY_TEST_LOGS) $(PMU_EVENTS_PYLINT_TEST_LOGS)
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,gen)$(PYTHON) $(JEVENTS_PY) $(JEVENTS_ARCH) $(JEVENTS_MODEL) pmu-events/arch $@
 endif
diff --git a/tools/perf/scripts/Build b/tools/perf/scripts/Build
index a5350dc6ac50..91229a1fe3ff 100644
--- a/tools/perf/scripts/Build
+++ b/tools/perf/scripts/Build
@@ -15,3 +15,16 @@ $(OUTPUT)%.mypy_log: %
 	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
 
 perf-y += $(MYPY_TEST_LOGS)
+
+ifdef PYLINT
+  PY_TESTS := $(shell find python -type f -name '*.py')
+  PYLINT_TEST_LOGS := $(PY_TESTS:python/%=python/%.pylint_log)
+else
+  PYLINT_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.pylint_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-y += $(PYLINT_TEST_LOGS)
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 4d7c38a2bf6c..5fe011cf897b 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -104,3 +104,16 @@ $(OUTPUT)%.mypy_log: %
 	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
 
 perf-test-y += $(MYPY_TEST_LOGS)
+
+ifdef PYLINT
+  PY_TESTS := $(shell find tests/shell -type f -name '*.py')
+  PYLINT_TEST_LOGS := $(PY_TESTS:tests/shell/%=shell/%.pylint_log)
+else
+  PYLINT_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.pylint_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-test-y += $(PYLINT_TEST_LOGS)
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 3baa1f41502d..c33e2372579e 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -422,3 +422,15 @@ $(OUTPUT)%.mypy_log: %
 	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
 
 perf-util-y += $(MYPY_TEST_LOGS)
+
+ifdef PYLINT
+  PYLINT_TEST_LOGS := $(PY_TESTS:%=%.pylint_log)
+else
+  PYLINT_TEST_LOGS :=
+endif
+
+$(OUTPUT)%.pylint_log: %
+	$(call rule_mkdir)
+	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
+
+perf-util-y += $(PYLINT_TEST_LOGS)
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH v1 5/6] perf test: Address attr.py mypy error

[Ian Rogers]

ConfigParser existed in python2 but not in python3 causing mypy to
fail.
Whilst removing a python2 workaround remove reference to __future__.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/lib/attr.py | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/tools/perf/tests/shell/lib/attr.py b/tools/perf/tests/shell/lib/attr.py
index 3db9a7d78715..bfccc727d9b2 100644
--- a/tools/perf/tests/shell/lib/attr.py
+++ b/tools/perf/tests/shell/lib/attr.py
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-from __future__ import print_function
-
+import configparser
 import os
 import sys
 import glob
@@ -13,11 +12,6 @@ import re
 import shutil
 import subprocess
 
-try:
-    import configparser
-except ImportError:
-    import ConfigParser as configparser
-
 def data_equal(a, b):
     # Allow multiple values in assignment separated by '|'
     a_list = a.split('|')
-- 
2.47.0.163.g1226f6d8fa-goog

[PATCH v1 6/6] perf python: Fix setup.py mypy errors

[Ian Rogers]

getenv may return None, so assert it isn't None for CC and srctree
environmental variables required for the script.
Disable an optional warning related to Popen.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/setup.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index 649550e9b7aa..dd289d15acfd 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -3,6 +3,7 @@ from subprocess import Popen, PIPE
 from re import sub
 
 cc = getenv("CC")
+assert cc, "Environment variable CC not set"
 
 # Check if CC has options, as is the case in yocto, where it uses CC="cc --sysroot..."
 cc_tokens = cc.split()
@@ -12,8 +13,13 @@ if len(cc_tokens) > 1:
 else:
     cc_options = ""
 
+# ignore optional stderr could be None as it is set to PIPE to avoid that.
+# mypy: disable-error-code="union-attr"
 cc_is_clang = b"clang version" in Popen([cc, "-v"], stderr=PIPE).stderr.readline()
-src_feature_tests  = getenv('srctree') + '/tools/build/feature'
+
+srctree = getenv('srctree')
+assert srctree, "Environment variable srctree, for the Linux sources, not set"
+src_feature_tests  = f'{srctree}/tools/build/feature'
 
 def clang_has_option(option):
     cc_output = Popen([cc, cc_options + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
@@ -71,7 +77,7 @@ else:
 # The python headers have mixed code with declarations (decls after asserts, for instance)
 cflags += [ "-Wno-declaration-after-statement" ]
 
-src_perf  = getenv('srctree') + '/tools/perf'
+src_perf  = f'{srctree}/tools/perf'
 build_lib = getenv('PYTHON_EXTBUILD_LIB')
 build_tmp = getenv('PYTHON_EXTBUILD_TMP')
 
-- 
2.47.0.163.g1226f6d8fa-goog

Re: [PATCH v1 0/6] Mypy and pylint build support

[Ian Rogers]

On Fri, Oct 25, 2024 at 10:23 AM Ian Rogers <irogers@google.com> wrote:
>
> Support building perf checking the python code with mypy and/or
> pylint. Currently there are too many errors to make this a default.
>
> Shellcheck generates no output on success, so linking shellcheck files
> doesn't cause `ld` to fail. Mypy and pylint generate output that will
> break `ld` so change the Makefile.build to ignore test log files.
>
> Address some initial mypy errors.
>
> Ian Rogers (6):
>   tools/build: Don't pass test log files to linker
>   perf build: Rename TEST_LOGS to SHELL_TEST_LOGS
>   perf build: Add mypy build tests
>   perf build: Add pylint build tests
>   perf test: Address attr.py mypy error
>   perf python: Fix setup.py mypy errors

Ping.

Thanks,
Ian

>  tools/build/Makefile.build         |  6 +++++-
>  tools/perf/Build                   | 32 +++++++++++++++++++++++++++---
>  tools/perf/Makefile.perf           | 14 +++++++++++--
>  tools/perf/arch/x86/Build          |  6 +++---
>  tools/perf/arch/x86/tests/Build    |  6 +++---
>  tools/perf/pmu-events/Build        | 25 ++++++++++++++++++++++-
>  tools/perf/scripts/Build           | 26 ++++++++++++++++++++++++
>  tools/perf/tests/Build             | 32 +++++++++++++++++++++++++++---
>  tools/perf/tests/shell/lib/attr.py |  8 +-------
>  tools/perf/trace/beauty/Build      |  6 +++---
>  tools/perf/util/Build              | 31 ++++++++++++++++++++++++++---
>  tools/perf/util/setup.py           | 10 ++++++++--
>  12 files changed, 171 insertions(+), 31 deletions(-)
>
> --
> 2.47.0.163.g1226f6d8fa-goog
>

Re: [PATCH v1 0/6] Mypy and pylint build support

[James Clark]

On 25/10/2024 6:22 pm, Ian Rogers wrote:
> Support building perf checking the python code with mypy and/or
> pylint. Currently there are too many errors to make this a default.
> 
> Shellcheck generates no output on success, so linking shellcheck files
> doesn't cause `ld` to fail. Mypy and pylint generate output that will
> break `ld` so change the Makefile.build to ignore test log files.
> 
> Address some initial mypy errors.
> 
> Ian Rogers (6):
>    tools/build: Don't pass test log files to linker
>    perf build: Rename TEST_LOGS to SHELL_TEST_LOGS
>    perf build: Add mypy build tests
>    perf build: Add pylint build tests
>    perf test: Address attr.py mypy error
>    perf python: Fix setup.py mypy errors
> 
>   tools/build/Makefile.build         |  6 +++++-
>   tools/perf/Build                   | 32 +++++++++++++++++++++++++++---
>   tools/perf/Makefile.perf           | 14 +++++++++++--
>   tools/perf/arch/x86/Build          |  6 +++---
>   tools/perf/arch/x86/tests/Build    |  6 +++---
>   tools/perf/pmu-events/Build        | 25 ++++++++++++++++++++++-
>   tools/perf/scripts/Build           | 26 ++++++++++++++++++++++++
>   tools/perf/tests/Build             | 32 +++++++++++++++++++++++++++---
>   tools/perf/tests/shell/lib/attr.py |  8 +-------
>   tools/perf/trace/beauty/Build      |  6 +++---
>   tools/perf/util/Build              | 31 ++++++++++++++++++++++++++---
>   tools/perf/util/setup.py           | 10 ++++++++--
>   12 files changed, 171 insertions(+), 31 deletions(-)
> 

Reviewed-by: James Clark <james.clark@linaro.org>

   $ make PYLINT=1 MYPY=1

   TEST    python/twatch.py.pylint_log
   ...
   Your code has been rated at -5.00/10
   ...

Re: [PATCH v1 0/6] Mypy and pylint build support

[James Clark]

On 16/01/2025 10:56 am, James Clark wrote:
> 
> 
> On 25/10/2024 6:22 pm, Ian Rogers wrote:
>> Support building perf checking the python code with mypy and/or
>> pylint. Currently there are too many errors to make this a default.
>>
>> Shellcheck generates no output on success, so linking shellcheck files
>> doesn't cause `ld` to fail. Mypy and pylint generate output that will
>> break `ld` so change the Makefile.build to ignore test log files.
>>
>> Address some initial mypy errors.
>>
>> Ian Rogers (6):
>>    tools/build: Don't pass test log files to linker
>>    perf build: Rename TEST_LOGS to SHELL_TEST_LOGS
>>    perf build: Add mypy build tests
>>    perf build: Add pylint build tests
>>    perf test: Address attr.py mypy error
>>    perf python: Fix setup.py mypy errors
>>
>>   tools/build/Makefile.build         |  6 +++++-
>>   tools/perf/Build                   | 32 +++++++++++++++++++++++++++---
>>   tools/perf/Makefile.perf           | 14 +++++++++++--
>>   tools/perf/arch/x86/Build          |  6 +++---
>>   tools/perf/arch/x86/tests/Build    |  6 +++---
>>   tools/perf/pmu-events/Build        | 25 ++++++++++++++++++++++-
>>   tools/perf/scripts/Build           | 26 ++++++++++++++++++++++++
>>   tools/perf/tests/Build             | 32 +++++++++++++++++++++++++++---
>>   tools/perf/tests/shell/lib/attr.py |  8 +-------
>>   tools/perf/trace/beauty/Build      |  6 +++---
>>   tools/perf/util/Build              | 31 ++++++++++++++++++++++++++---
>>   tools/perf/util/setup.py           | 10 ++++++++--
>>   12 files changed, 171 insertions(+), 31 deletions(-)
>>
> 
> Reviewed-by: James Clark <james.clark@linaro.org>
> 
>    $ make PYLINT=1 MYPY=1
> 
>    TEST    python/twatch.py.pylint_log
>    ...
>    Your code has been rated at -5.00/10
>    ...

I forgot to add this:

  :(

I didn't know it could go negative.

Re: [PATCH v1 0/6] Mypy and pylint build support

[Ian Rogers]

On Thu, Jan 16, 2025 at 2:58 AM James Clark <james.clark@linaro.org> wrote:
>
>
>
> On 16/01/2025 10:56 am, James Clark wrote:
> >
> >
> > On 25/10/2024 6:22 pm, Ian Rogers wrote:
> >> Support building perf checking the python code with mypy and/or
> >> pylint. Currently there are too many errors to make this a default.
> >>
> >> Shellcheck generates no output on success, so linking shellcheck files
> >> doesn't cause `ld` to fail. Mypy and pylint generate output that will
> >> break `ld` so change the Makefile.build to ignore test log files.
> >>
> >> Address some initial mypy errors.
> >>
> >> Ian Rogers (6):
> >>    tools/build: Don't pass test log files to linker
> >>    perf build: Rename TEST_LOGS to SHELL_TEST_LOGS
> >>    perf build: Add mypy build tests
> >>    perf build: Add pylint build tests
> >>    perf test: Address attr.py mypy error
> >>    perf python: Fix setup.py mypy errors
> >>
> >>   tools/build/Makefile.build         |  6 +++++-
> >>   tools/perf/Build                   | 32 +++++++++++++++++++++++++++---
> >>   tools/perf/Makefile.perf           | 14 +++++++++++--
> >>   tools/perf/arch/x86/Build          |  6 +++---
> >>   tools/perf/arch/x86/tests/Build    |  6 +++---
> >>   tools/perf/pmu-events/Build        | 25 ++++++++++++++++++++++-
> >>   tools/perf/scripts/Build           | 26 ++++++++++++++++++++++++
> >>   tools/perf/tests/Build             | 32 +++++++++++++++++++++++++++---
> >>   tools/perf/tests/shell/lib/attr.py |  8 +-------
> >>   tools/perf/trace/beauty/Build      |  6 +++---
> >>   tools/perf/util/Build              | 31 ++++++++++++++++++++++++++---
> >>   tools/perf/util/setup.py           | 10 ++++++++--
> >>   12 files changed, 171 insertions(+), 31 deletions(-)
> >>
> >
> > Reviewed-by: James Clark <james.clark@linaro.org>
> >
> >    $ make PYLINT=1 MYPY=1
> >
> >    TEST    python/twatch.py.pylint_log
> >    ...
> >    Your code has been rated at -5.00/10
> >    ...
>
> I forgot to add this:
>
>   :(
>
> I didn't know it could go negative.

:-) Shows we have some room to improve.

Thanks for the review!
Ian

Re: [PATCH v1 1/6] tools/build: Don't pass test log files to linker

[Namhyung Kim]

On Fri, Oct 25, 2024 at 10:22:58AM -0700, Ian Rogers wrote:
> Separate test log files from object files. Depend on test log output
> but don't pass to the linker.

I don't know why $(obj-y) contains log files in the first place.  It's
supposed to have .o files only, right?

Thanks,
Namhyung

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/build/Makefile.build | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/build/Makefile.build b/tools/build/Makefile.build
> index 5fb3fb3d97e0..ffe988867703 100644
> --- a/tools/build/Makefile.build
> +++ b/tools/build/Makefile.build
> @@ -149,6 +149,10 @@ objprefix    := $(subst ./,,$(OUTPUT)$(dir)/)
>  obj-y        := $(addprefix $(objprefix),$(obj-y))
>  subdir-obj-y := $(addprefix $(objprefix),$(subdir-obj-y))
>  
> +# Separate out test log files from real build objects.
> +test-y       := $(filter %_log, $(obj-y))
> +obj-y        := $(filter-out %_log, $(obj-y))
> +
>  # Final '$(obj)-in.o' object
>  in-target := $(objprefix)$(obj)-in.o
>  
> @@ -159,7 +163,7 @@ $(subdir-y):
>  
>  $(sort $(subdir-obj-y)): $(subdir-y) ;
>  
> -$(in-target): $(obj-y) FORCE
> +$(in-target): $(obj-y) $(test-y) FORCE
>  	$(call rule_mkdir)
>  	$(call if_changed,$(host)ld_multi)
>  
> -- 
> 2.47.0.163.g1226f6d8fa-goog
> 

Re: [PATCH v1 3/6] perf build: Add mypy build tests

[Namhyung Kim]

On Fri, Oct 25, 2024 at 10:23:00AM -0700, Ian Rogers wrote:
> If MYPY=1 is passed to the build then run mypy over python code in
> perf. Unlike shellcheck this isn't default on as there are currently
> too many errors.

Can you please add an example output in the commit log?  Also you need
to add a comment to describe the build option.

Is it ok to pass the option on a system that doesn't have mypy?

Thanks,
Namhyung

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/Build            | 13 +++++++++++++
>  tools/perf/Makefile.perf    |  7 ++++++-
>  tools/perf/pmu-events/Build | 14 +++++++++++++-
>  tools/perf/scripts/Build    | 13 +++++++++++++
>  tools/perf/tests/Build      | 13 +++++++++++++
>  tools/perf/util/Build       | 13 +++++++++++++
>  6 files changed, 71 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/Build b/tools/perf/Build
> index 5e385f370dd7..312914994c89 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -76,3 +76,16 @@ $(OUTPUT)%.shellcheck_log: %
>  	$(Q)$(call echo-cmd,test)shellcheck -s bash -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>  
>  perf-y += $(SHELL_TEST_LOGS)
> +
> +ifdef MYPY
> +  PY_TESTS := $(shell find python -type f -name '*.py')
> +  MYPY_TEST_LOGS := $(PY_TESTS:python/%=python/%.mypy_log)
> +else
> +  MYPY_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.mypy_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-y += $(MYPY_TEST_LOGS)
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index a9f2a9f6ebf0..333afe29b1e2 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -277,8 +277,13 @@ ifneq ($(SHELLCHECK),)
>    endif
>  endif
>  
> +# Runs mypy on perf python files
> +ifeq ($(MYPY),1)
> +  MYPY := $(shell which mypy 2> /dev/null)
> +endif
> +
>  export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
> -export HOSTCC HOSTLD HOSTAR HOSTCFLAGS SHELLCHECK
> +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS SHELLCHECK MYPY
>  
>  include $(srctree)/tools/build/Makefile.include
>  
> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> index d941bc9d16e9..fc1dc810ffb6 100644
> --- a/tools/perf/pmu-events/Build
> +++ b/tools/perf/pmu-events/Build
> @@ -41,7 +41,19 @@ $(EMPTY_PMU_EVENTS_TEST_LOG): $(EMPTY_PMU_EVENTS_C) $(TEST_EMPTY_PMU_EVENTS_C)
>  	$(call rule_mkdir)
>  	$(Q)$(call echo-cmd,test)diff -u $^ 2> $@ || (cat $@ && false)
>  
> -$(PMU_EVENTS_C): $(JSON) $(JSON_TEST) $(JEVENTS_PY) $(METRIC_PY) $(METRIC_TEST_LOG) $(EMPTY_PMU_EVENTS_TEST_LOG)
> +ifdef MYPY
> +  PMU_EVENTS_PY_TESTS := $(wildcard *.py)
> +  PMU_EVENTS_MYPY_TEST_LOGS := $(JEVENTS_PY_TESTS:%=%.mypy_log)
> +else
> +  PMU_EVENTS_MYPY_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.mypy_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +$(PMU_EVENTS_C): $(JSON) $(JSON_TEST) $(JEVENTS_PY) $(METRIC_PY) $(METRIC_TEST_LOG) \
> +    $(EMPTY_PMU_EVENTS_TEST_LOG) $(PMU_EVENTS_MYPY_TEST_LOGS)
>  	$(call rule_mkdir)
>  	$(Q)$(call echo-cmd,gen)$(PYTHON) $(JEVENTS_PY) $(JEVENTS_ARCH) $(JEVENTS_MODEL) pmu-events/arch $@
>  endif
> diff --git a/tools/perf/scripts/Build b/tools/perf/scripts/Build
> index 46f0c6f76dbf..a5350dc6ac50 100644
> --- a/tools/perf/scripts/Build
> +++ b/tools/perf/scripts/Build
> @@ -2,3 +2,16 @@ ifeq ($(CONFIG_LIBTRACEEVENT),y)
>    perf-util-$(CONFIG_LIBPERL)   += perl/Perf-Trace-Util/
>  endif
>  perf-util-$(CONFIG_LIBPYTHON) += python/Perf-Trace-Util/
> +
> +ifdef MYPY
> +  PY_TESTS := $(shell find python -type f -name '*.py')
> +  MYPY_TEST_LOGS := $(PY_TESTS:python/%=python/%.mypy_log)
> +else
> +  MYPY_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.mypy_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-y += $(MYPY_TEST_LOGS)
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index a51d28c7431c..4d7c38a2bf6c 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -91,3 +91,16 @@ $(OUTPUT)%.shellcheck_log: %
>  	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>  
>  perf-test-y += $(SHELL_TEST_LOGS)
> +
> +ifdef MYPY
> +  PY_TESTS := $(shell find tests/shell -type f -name '*.py')
> +  MYPY_TEST_LOGS := $(PY_TESTS:tests/shell/%=shell/%.mypy_log)
> +else
> +  MYPY_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.mypy_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-test-y += $(MYPY_TEST_LOGS)
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 18cd02ccd3ff..3baa1f41502d 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -409,3 +409,16 @@ $(OUTPUT)%.shellcheck_log: %
>  	$(Q)$(call echo-cmd,test)shellcheck -a -S warning "$<" > $@ || (cat $@ && rm $@ && false)
>  
>  perf-util-y += $(SHELL_TEST_LOGS)
> +
> +PY_TESTS := setup.py
> +ifdef MYPY
> +  MYPY_TEST_LOGS := $(PY_TESTS:%=%.mypy_log)
> +else
> +  MYPY_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.mypy_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-util-y += $(MYPY_TEST_LOGS)
> -- 
> 2.47.0.163.g1226f6d8fa-goog
> 

Re: [PATCH v1 4/6] perf build: Add pylint build tests

[Namhyung Kim]

On Fri, Oct 25, 2024 at 10:23:01AM -0700, Ian Rogers wrote:
> If PYLINT=1 is passed to the build then run pylint over python code in
> perf. Unlike shellcheck this isn't default on as there are currently
> too many errors.

Same as the previous.

Thanks,
Namhyung

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/Build            | 13 +++++++++++++
>  tools/perf/Makefile.perf    |  7 ++++++-
>  tools/perf/pmu-events/Build | 13 ++++++++++++-
>  tools/perf/scripts/Build    | 13 +++++++++++++
>  tools/perf/tests/Build      | 13 +++++++++++++
>  tools/perf/util/Build       | 12 ++++++++++++
>  6 files changed, 69 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/Build b/tools/perf/Build
> index 312914994c89..06107f1e1d42 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -89,3 +89,16 @@ $(OUTPUT)%.mypy_log: %
>  	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
>  
>  perf-y += $(MYPY_TEST_LOGS)
> +
> +ifdef PYLINT
> +  PY_TESTS := $(shell find python -type f -name '*.py')
> +  PYLINT_TEST_LOGS := $(PY_TESTS:python/%=python/%.pylint_log)
> +else
> +  PYLINT_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.pylint_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-y += $(PYLINT_TEST_LOGS)
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 333afe29b1e2..6b5b420d794a 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -282,8 +282,13 @@ ifeq ($(MYPY),1)
>    MYPY := $(shell which mypy 2> /dev/null)
>  endif
>  
> +# Runs pylint on perf python files
> +ifeq ($(PYLINT),1)
> +  PYLINT := $(shell which pylint 2> /dev/null)
> +endif
> +
>  export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
> -export HOSTCC HOSTLD HOSTAR HOSTCFLAGS SHELLCHECK MYPY
> +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS SHELLCHECK MYPY PYLINT
>  
>  include $(srctree)/tools/build/Makefile.include
>  
> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> index fc1dc810ffb6..32f387d48908 100644
> --- a/tools/perf/pmu-events/Build
> +++ b/tools/perf/pmu-events/Build
> @@ -52,8 +52,19 @@ $(OUTPUT)%.mypy_log: %
>  	$(call rule_mkdir)
>  	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
>  
> +ifdef PYLINT
> +  PMU_EVENTS_PY_TESTS := $(wildcard *.py)
> +  PMU_EVENTS_PYLINT_TEST_LOGS := $(JEVENTS_PY_TESTS:%=%.pylint_log)
> +else
> +  PMU_EVENTS_PYLINT_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.pylint_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
> +
>  $(PMU_EVENTS_C): $(JSON) $(JSON_TEST) $(JEVENTS_PY) $(METRIC_PY) $(METRIC_TEST_LOG) \
> -    $(EMPTY_PMU_EVENTS_TEST_LOG) $(PMU_EVENTS_MYPY_TEST_LOGS)
> +    $(EMPTY_PMU_EVENTS_TEST_LOG) $(PMU_EVENTS_MYPY_TEST_LOGS) $(PMU_EVENTS_PYLINT_TEST_LOGS)
>  	$(call rule_mkdir)
>  	$(Q)$(call echo-cmd,gen)$(PYTHON) $(JEVENTS_PY) $(JEVENTS_ARCH) $(JEVENTS_MODEL) pmu-events/arch $@
>  endif
> diff --git a/tools/perf/scripts/Build b/tools/perf/scripts/Build
> index a5350dc6ac50..91229a1fe3ff 100644
> --- a/tools/perf/scripts/Build
> +++ b/tools/perf/scripts/Build
> @@ -15,3 +15,16 @@ $(OUTPUT)%.mypy_log: %
>  	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
>  
>  perf-y += $(MYPY_TEST_LOGS)
> +
> +ifdef PYLINT
> +  PY_TESTS := $(shell find python -type f -name '*.py')
> +  PYLINT_TEST_LOGS := $(PY_TESTS:python/%=python/%.pylint_log)
> +else
> +  PYLINT_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.pylint_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-y += $(PYLINT_TEST_LOGS)
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 4d7c38a2bf6c..5fe011cf897b 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -104,3 +104,16 @@ $(OUTPUT)%.mypy_log: %
>  	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
>  
>  perf-test-y += $(MYPY_TEST_LOGS)
> +
> +ifdef PYLINT
> +  PY_TESTS := $(shell find tests/shell -type f -name '*.py')
> +  PYLINT_TEST_LOGS := $(PY_TESTS:tests/shell/%=shell/%.pylint_log)
> +else
> +  PYLINT_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.pylint_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-test-y += $(PYLINT_TEST_LOGS)
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 3baa1f41502d..c33e2372579e 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -422,3 +422,15 @@ $(OUTPUT)%.mypy_log: %
>  	$(Q)$(call echo-cmd,test)mypy "$<" > $@ || (cat $@ && rm $@ && false)
>  
>  perf-util-y += $(MYPY_TEST_LOGS)
> +
> +ifdef PYLINT
> +  PYLINT_TEST_LOGS := $(PY_TESTS:%=%.pylint_log)
> +else
> +  PYLINT_TEST_LOGS :=
> +endif
> +
> +$(OUTPUT)%.pylint_log: %
> +	$(call rule_mkdir)
> +	$(Q)$(call echo-cmd,test)pylint "$<" > $@ || (cat $@ && rm $@ && false)
> +
> +perf-util-y += $(PYLINT_TEST_LOGS)
> -- 
> 2.47.0.163.g1226f6d8fa-goog
> 

Re: [PATCH v1 0/6] Mypy and pylint build support

[Namhyung Kim]

On Fri, Oct 25, 2024 at 10:22:57AM -0700, Ian Rogers wrote:
> Support building perf checking the python code with mypy and/or
> pylint. Currently there are too many errors to make this a default.
> 
> Shellcheck generates no output on success, so linking shellcheck files
> doesn't cause `ld` to fail. Mypy and pylint generate output that will
> break `ld` so change the Makefile.build to ignore test log files.
> 
> Address some initial mypy errors.
> 
> Ian Rogers (6):
>   tools/build: Don't pass test log files to linker
>   perf build: Rename TEST_LOGS to SHELL_TEST_LOGS
>   perf build: Add mypy build tests
>   perf build: Add pylint build tests
>   perf test: Address attr.py mypy error
>   perf python: Fix setup.py mypy errors

It doesn't apply to the perf-tools-next cleanly.  Can you please rebase?

Thanks,
Namhyung

> 
>  tools/build/Makefile.build         |  6 +++++-
>  tools/perf/Build                   | 32 +++++++++++++++++++++++++++---
>  tools/perf/Makefile.perf           | 14 +++++++++++--
>  tools/perf/arch/x86/Build          |  6 +++---
>  tools/perf/arch/x86/tests/Build    |  6 +++---
>  tools/perf/pmu-events/Build        | 25 ++++++++++++++++++++++-
>  tools/perf/scripts/Build           | 26 ++++++++++++++++++++++++
>  tools/perf/tests/Build             | 32 +++++++++++++++++++++++++++---
>  tools/perf/tests/shell/lib/attr.py |  8 +-------
>  tools/perf/trace/beauty/Build      |  6 +++---
>  tools/perf/util/Build              | 31 ++++++++++++++++++++++++++---
>  tools/perf/util/setup.py           | 10 ++++++++--
>  12 files changed, 171 insertions(+), 31 deletions(-)
> 
> -- 
> 2.47.0.163.g1226f6d8fa-goog
> 

Re: [PATCH v1 1/6] tools/build: Don't pass test log files to linker

[Ian Rogers]

On Fri, Jan 24, 2025 at 3:31 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Fri, Oct 25, 2024 at 10:22:58AM -0700, Ian Rogers wrote:
> > Separate test log files from object files. Depend on test log output
> > but don't pass to the linker.
>
> I don't know why $(obj-y) contains log files in the first place.  It's
> supposed to have .o files only, right?

There's context here:
https://lore.kernel.org/all/20231129213428.2227448-1-irogers@google.com/
$(obj-y) is the set of the dependencies from Build files, generally
directories or .o files. Perf added the test logs as the alternative
is to duplicate all the directory scanning and other logic for a
$(test-y) but it isn't clear how you'd even name targets for a test in
the Build files. Rather than reinvent Makefile.build the choice was
made to work with what we had.

Thanks,
Ian

Re: [PATCH v1 3/6] perf build: Add mypy build tests

[Ian Rogers]

On Fri, Jan 24, 2025 at 3:51 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Fri, Oct 25, 2024 at 10:23:00AM -0700, Ian Rogers wrote:
> > If MYPY=1 is passed to the build then run mypy over python code in
> > perf. Unlike shellcheck this isn't default on as there are currently
> > too many errors.
>
> Can you please add an example output in the commit log?  Also you need
> to add a comment to describe the build option.

It feels more of a developer option, so I'm not sure we should
advertise it. Ideally we'd just default it like the shellcheck case.
I'm not sure of the benefit of snapshotting mypy errors, a quick
internet search will show what mypy does and I'd prefer the patch set
focus on the build infrastructure changes.

> Is it ok to pass the option on a system that doesn't have mypy?

I'd expect the shell/which assignment to MYPY would leave it
undefined, in which case the behavior would match not building with
MYPY=1. It's basically the same logic as with shellchecks. I didn't
test/optimize for people requesting a build option with dependencies
missing.

Thanks,
Ian

Re: [PATCH v1 1/6] tools/build: Don't pass test log files to linker

[Namhyung Kim]

On Fri, Jan 24, 2025 at 05:58:37PM -0800, Ian Rogers wrote:
> On Fri, Jan 24, 2025 at 3:31 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Fri, Oct 25, 2024 at 10:22:58AM -0700, Ian Rogers wrote:
> > > Separate test log files from object files. Depend on test log output
> > > but don't pass to the linker.
> >
> > I don't know why $(obj-y) contains log files in the first place.  It's
> > supposed to have .o files only, right?
> 
> There's context here:
> https://lore.kernel.org/all/20231129213428.2227448-1-irogers@google.com/
> $(obj-y) is the set of the dependencies from Build files, generally
> directories or .o files. Perf added the test logs as the alternative
> is to duplicate all the directory scanning and other logic for a
> $(test-y) but it isn't clear how you'd even name targets for a test in
> the Build files. Rather than reinvent Makefile.build the choice was
> made to work with what we had.

Thanks for the explanation.  I'm ok with it for now but I'll take a look
if there's a better way later.

Thanks,
Namhyung

Re: [PATCH v1 3/6] perf build: Add mypy build tests

[Namhyung Kim]

On Fri, Jan 24, 2025 at 06:10:09PM -0800, Ian Rogers wrote:
> On Fri, Jan 24, 2025 at 3:51 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Fri, Oct 25, 2024 at 10:23:00AM -0700, Ian Rogers wrote:
> > > If MYPY=1 is passed to the build then run mypy over python code in
> > > perf. Unlike shellcheck this isn't default on as there are currently
> > > too many errors.
> >
> > Can you please add an example output in the commit log?  Also you need
> > to add a comment to describe the build option.
> 
> It feels more of a developer option, so I'm not sure we should
> advertise it. Ideally we'd just default it like the shellcheck case.

Yep, but I think we need some (brief) explanation even for a default
developer option.

> I'm not sure of the benefit of snapshotting mypy errors, a quick
> internet search will show what mypy does and I'd prefer the patch set
> focus on the build infrastructure changes.

You are introducing a new thing so you need to explain what it is, how
to use it and how the output look like.

> 
> > Is it ok to pass the option on a system that doesn't have mypy?
> 
> I'd expect the shell/which assignment to MYPY would leave it
> undefined, in which case the behavior would match not building with
> MYPY=1. It's basically the same logic as with shellchecks. I didn't
> test/optimize for people requesting a build option with dependencies
> missing.

Ok, I'm good as long as it handles the case.

Thanks,
Namhyung