Re: [PATCH 8/8] selftests/bpf: factor out MKDIR rule

[Yauheni Kaliuta <yauheni.kaliuta@redhat.com>]

Hi, Andrii!

>>>>> On Tue, 26 May 2020 15:29:04 -0700, Andrii Nakryiko  wrote:

 > On Thu, May 21, 2020 at 9:14 PM Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> 
 >> Do not repeat youself, move common mkdir code (message and action)
 >> to a variable.
 >> 
 >> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
 >> ---
 >> tools/testing/selftests/bpf/Makefile | 14 ++++++++------
 >> 1 file changed, 8 insertions(+), 6 deletions(-)
 >> 
 >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
 >> index bade24e29a1a..26497d8869ea 100644
 >> --- a/tools/testing/selftests/bpf/Makefile
 >> +++ b/tools/testing/selftests/bpf/Makefile
 >> @@ -252,6 +252,11 @@ define COMPILE_C_RULE
 >> $(CC) $(CFLAGS) -c $(filter %.c,$^) $(LDLIBS) -o $@
 >> endef
 >> 
 >> +define MKDIR_RULE
 >> +       $(call msg,MKDIR,,$@)
 >> +       mkdir -p $@
 >> +endef

 > I don't think you save much with this, especially by combining
 > dir creation rules together. Let's not do this, just adds
 > extra level of "rule nestedness", if I may say so...

Ok

 >> +
 >> SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
 >> 
 >> # Set up extra TRUNNER_XXX "temporary" variables in the environment (relies on
 >> @@ -294,8 +299,7 @@ define DEFINE_TEST_RUNNER_RULES
 >> ifeq ($($(TRUNNER_OUTPUT)-dir),)
 >> $(TRUNNER_OUTPUT)-dir := y
 >> $(TRUNNER_OUTPUT):
 >> -       $$(call msg,MKDIR,,$$@)
 >> -       mkdir -p $$@
 >> +       $$(MKDIR_RULE)
 >> 
 >> ifneq ($2,)
 >> EXTRA_CLEAN +=$(TRUNNER_OUTPUT)
 >> @@ -337,8 +341,7 @@ $(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c |
 >> $(dir $(TRUNNER_TESTS_HDR))
 >> EXTRA_CLEAN += $(TRUNNER_TESTS_HDR)
 >> 
 >> $(dir $(TRUNNER_TESTS_HDR)):

 > combine this rule with $(TRUNNER_OUTPUT) above?

 >> -       $$(call msg,MKDIR,,$$@)
 >> -       mkdir -p $$@
 >> +       $$(MKDIR_RULE)
 >> endif
 >> 
 >> # compile individual test files
 >> @@ -425,8 +428,7 @@ $(OUTPUT)/verifier/tests.h: verifier/*.c | $(OUTPUT)/verifier
 >> EXTRA_CLEAN += $(OUTPUT)/verifier/tests.h
 >> 
 >> $(OUTPUT)/verifier:
 >> -       $(call msg,MKDIR,,$@)
 >> -       mkdir -p $@
 >> +       $(MKDIR_RULE)

 > This should go together with libbpf, bpftool and $(INCLUDE_DIR) rule
 > at line 176.

 >> 
 >> $(OUTPUT)/test_verifier: CFLAGS += -I$(abspath verifier)
 >> $(OUTPUT)/test_verifier: test_verifier.c $(OUTPUT)/verifier/tests.h $(BPFOBJ) \
 >> --
 >> 2.26.2
 >> 


-- 
WBR,
Yauheni Kaliuta