| Message ID | 20190716193837.2808971-1-andriin@fb.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf,1/2] selftests/bpf: fix test_verifier/test_maps make dependencies | expand |
On 07/16, Andrii Nakryiko wrote: > e46fc22e60a4 ("selftests/bpf: make directory prerequisites order-only") > exposed existing problem in Makefile for test_verifier and test_maps tests: > their dependency on auto-generated header file with a list of all tests wasn't > recorded explicitly. This patch fixes these issues. Why adding it explicitly fixes it? At least for test_verifier, we have the following rule: test_verifier.c: $(VERIFIER_TESTS_H) And there should be implicit/builtin test_verifier -> test_verifier.c dependency rule. Same for maps, I guess: $(OUTPUT)/test_maps: map_tests/*.c test_maps.c: $(MAP_TESTS_H) So why is it not working as is? What I'm I missing?
On Tue, Jul 16, 2019 at 12:55 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > On 07/16, Andrii Nakryiko wrote: > > e46fc22e60a4 ("selftests/bpf: make directory prerequisites order-only") > > exposed existing problem in Makefile for test_verifier and test_maps tests: > > their dependency on auto-generated header file with a list of all tests wasn't > > recorded explicitly. This patch fixes these issues. > Why adding it explicitly fixes it? At least for test_verifier, we have > the following rule: > > test_verifier.c: $(VERIFIER_TESTS_H) > > And there should be implicit/builtin test_verifier -> test_verifier.c > dependency rule. > > Same for maps, I guess: > > $(OUTPUT)/test_maps: map_tests/*.c > test_maps.c: $(MAP_TESTS_H) > > So why is it not working as is? What I'm I missing? I don't know exactly why it's not working, but it's clearly because of that. It's the only difference between how test_progs are set up, which didn't break, and test_maps/test_verifier, which did. Feel free to figure it out through a maze of Makefiles why it didn't work as expected, but this definitely fixed a breakage (at least for me).
On 07/16, Andrii Nakryiko wrote: > On Tue, Jul 16, 2019 at 12:55 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > On 07/16, Andrii Nakryiko wrote: > > > e46fc22e60a4 ("selftests/bpf: make directory prerequisites order-only") > > > exposed existing problem in Makefile for test_verifier and test_maps tests: > > > their dependency on auto-generated header file with a list of all tests wasn't > > > recorded explicitly. This patch fixes these issues. > > Why adding it explicitly fixes it? At least for test_verifier, we have > > the following rule: > > > > test_verifier.c: $(VERIFIER_TESTS_H) > > > > And there should be implicit/builtin test_verifier -> test_verifier.c > > dependency rule. > > > > Same for maps, I guess: > > > > $(OUTPUT)/test_maps: map_tests/*.c > > test_maps.c: $(MAP_TESTS_H) > > > > So why is it not working as is? What I'm I missing? > > I don't know exactly why it's not working, but it's clearly because of > that. It's the only difference between how test_progs are set up, > which didn't break, and test_maps/test_verifier, which did. > > Feel free to figure it out through a maze of Makefiles why it didn't > work as expected, but this definitely fixed a breakage (at least for > me). Agreed on not wasting time. I took a brief look (with make -qp) and I don't have any clue. By default implicit matching doesn't work: # makefile (from 'Makefile', line 261) /linux/tools/testing/selftests/bpf/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) /linux/tools/testing/selftests/bpf/test_maps: map_tests/sk_storage_map.c /linux/tools/testing/selftests/bpf/test_stub.o /linux/tools/testing/selftests/bpf/libbpf.a # Implicit rule search has not been done. # File is an intermediate prerequisite. # Modification time never checked. # File has not been updated. # variable set hash-table stats: # Load=1/32=3%, Rehash=0, Collisions=0/2=0% If I comment out the following line: $(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ) Then it works: # makefile (from 'Makefile', line 261) /linux/tools/testing/selftests/bpf/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) /linux/tools/testing/selftests/bpf/test_maps: test_maps.c map_tests/sk_storage_map.c # Implicit rule search has been done. # Implicit/static pattern stem: 'test_maps' # File is an intermediate prerequisite. # File does not exist. # File has not been updated. # variable set hash-table stats: # Load=1/32=3%, Rehash=0, Collisions=0/2=0% # recipe to execute (from '../lib.mk', line 138): $(LINK.c) $^ $(LDLIBS) -o $@ It's because "File is an intermediate prerequisite.", but I don't see how it's is a intermediate prerequisite for anything... One other optional suggestion I have to your second patch: maybe drop all those dependencies on the directories altogether? Why not do the following instead, for example (same for test_progs/test_maps)? diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 1296253b3422..c2d087ce6d4b 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -277,12 +277,9 @@ VERIFIER_TESTS_H := $(OUTPUT)/verifier/tests.h test_verifier.c: $(VERIFIER_TESTS_H) $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS) -VERIFIER_TESTS_DIR = $(OUTPUT)/verifier -$(VERIFIER_TESTS_DIR): - mkdir -p $@ - VERIFIER_TEST_FILES := $(wildcard verifier/*.c) -$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR) +$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) + mkdir -p $(dir $@) $(shell ( cd verifier/; \ echo '/* Generated header, do not edit */'; \ echo '#ifdef FILL_ARRAY'; \
On Tue, Jul 16, 2019 at 3:57 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > On 07/16, Andrii Nakryiko wrote: > > On Tue, Jul 16, 2019 at 12:55 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > On 07/16, Andrii Nakryiko wrote: > > > > e46fc22e60a4 ("selftests/bpf: make directory prerequisites order-only") > > > > exposed existing problem in Makefile for test_verifier and test_maps tests: > > > > their dependency on auto-generated header file with a list of all tests wasn't > > > > recorded explicitly. This patch fixes these issues. > > > Why adding it explicitly fixes it? At least for test_verifier, we have > > > the following rule: > > > > > > test_verifier.c: $(VERIFIER_TESTS_H) > > > > > > And there should be implicit/builtin test_verifier -> test_verifier.c > > > dependency rule. > > > > > > Same for maps, I guess: > > > > > > $(OUTPUT)/test_maps: map_tests/*.c > > > test_maps.c: $(MAP_TESTS_H) > > > > > > So why is it not working as is? What I'm I missing? > > > > I don't know exactly why it's not working, but it's clearly because of > > that. It's the only difference between how test_progs are set up, > > which didn't break, and test_maps/test_verifier, which did. > > > > Feel free to figure it out through a maze of Makefiles why it didn't > > work as expected, but this definitely fixed a breakage (at least for > > me). > Agreed on not wasting time. I took a brief look (with make -qp) and I > don't have any clue. Oh, -qp is cool, noted :) > > By default implicit matching doesn't work: > # makefile (from 'Makefile', line 261) > /linux/tools/testing/selftests/bpf/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) > /linux/tools/testing/selftests/bpf/test_maps: map_tests/sk_storage_map.c /linux/tools/testing/selftests/bpf/test_stub.o /linux/tools/testing/selftests/bpf/libbpf.a > # Implicit rule search has not been done. > # File is an intermediate prerequisite. > # Modification time never checked. > # File has not been updated. > # variable set hash-table stats: > # Load=1/32=3%, Rehash=0, Collisions=0/2=0% > > If I comment out the following line: > $(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ) > > Then it works: > # makefile (from 'Makefile', line 261) > /linux/tools/testing/selftests/bpf/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) > /linux/tools/testing/selftests/bpf/test_maps: test_maps.c map_tests/sk_storage_map.c > # Implicit rule search has been done. > # Implicit/static pattern stem: 'test_maps' > # File is an intermediate prerequisite. > # File does not exist. > # File has not been updated. > # variable set hash-table stats: > # Load=1/32=3%, Rehash=0, Collisions=0/2=0% > # recipe to execute (from '../lib.mk', line 138): > $(LINK.c) $^ $(LDLIBS) -o $@ > > It's because "File is an intermediate prerequisite.", but I > don't see how it's is a intermediate prerequisite for anything... Well, it's "File is an intermediate prerequisite." in both cases, so I don't know if that's it. Makefiles is not my strong suit, honestly, and definitely not an area of passion, so no idea > > > One other optional suggestion I have to your second patch: maybe drop all > those dependencies on the directories altogether? Why not do the following > instead, for example (same for test_progs/test_maps)? Some of those _TEST_DIR's are dependencies of multiple targets, so you'd need to replicate that `mkdir -p $@` in multiple places, which is annoying. But I also don't think we need to worry about creating them, because there is always at least one test (otherwise those tests are useless anyways) in those directories, so we might as well just remove those dependencies, I guess. TBH, those explicit dependencies are good to specify anyways, so I think this fix is good. Second patch just makes the layout of dependencies similar, so it's easier to spot differences like this one. As usual, I'll let Alexei and Daniel decide which one to apply (or if we need to take some other approach to fixing this). > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 1296253b3422..c2d087ce6d4b 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -277,12 +277,9 @@ VERIFIER_TESTS_H := $(OUTPUT)/verifier/tests.h > test_verifier.c: $(VERIFIER_TESTS_H) > $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS) > > -VERIFIER_TESTS_DIR = $(OUTPUT)/verifier > -$(VERIFIER_TESTS_DIR): > - mkdir -p $@ > - > VERIFIER_TEST_FILES := $(wildcard verifier/*.c) > -$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR) > +$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) > + mkdir -p $(dir $@) > $(shell ( cd verifier/; \ > echo '/* Generated header, do not edit */'; \ > echo '#ifdef FILL_ARRAY'; \
On 07/16, Andrii Nakryiko wrote: > On Tue, Jul 16, 2019 at 3:57 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > On 07/16, Andrii Nakryiko wrote: > > > On Tue, Jul 16, 2019 at 12:55 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > > > On 07/16, Andrii Nakryiko wrote: > > > > > e46fc22e60a4 ("selftests/bpf: make directory prerequisites order-only") > > > > > exposed existing problem in Makefile for test_verifier and test_maps tests: > > > > > their dependency on auto-generated header file with a list of all tests wasn't > > > > > recorded explicitly. This patch fixes these issues. > > > > Why adding it explicitly fixes it? At least for test_verifier, we have > > > > the following rule: > > > > > > > > test_verifier.c: $(VERIFIER_TESTS_H) > > > > > > > > And there should be implicit/builtin test_verifier -> test_verifier.c > > > > dependency rule. > > > > > > > > Same for maps, I guess: > > > > > > > > $(OUTPUT)/test_maps: map_tests/*.c > > > > test_maps.c: $(MAP_TESTS_H) > > > > > > > > So why is it not working as is? What I'm I missing? > > > > > > I don't know exactly why it's not working, but it's clearly because of > > > that. It's the only difference between how test_progs are set up, > > > which didn't break, and test_maps/test_verifier, which did. > > > > > > Feel free to figure it out through a maze of Makefiles why it didn't > > > work as expected, but this definitely fixed a breakage (at least for > > > me). > > Agreed on not wasting time. I took a brief look (with make -qp) and I > > don't have any clue. > > Oh, -qp is cool, noted :) > > > > > By default implicit matching doesn't work: > > # makefile (from 'Makefile', line 261) > > /linux/tools/testing/selftests/bpf/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) > > /linux/tools/testing/selftests/bpf/test_maps: map_tests/sk_storage_map.c /linux/tools/testing/selftests/bpf/test_stub.o /linux/tools/testing/selftests/bpf/libbpf.a > > # Implicit rule search has not been done. > > # File is an intermediate prerequisite. > > # Modification time never checked. > > # File has not been updated. > > # variable set hash-table stats: > > # Load=1/32=3%, Rehash=0, Collisions=0/2=0% > > > > If I comment out the following line: > > $(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ) > > > > Then it works: > > # makefile (from 'Makefile', line 261) > > /linux/tools/testing/selftests/bpf/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) > > /linux/tools/testing/selftests/bpf/test_maps: test_maps.c map_tests/sk_storage_map.c > > # Implicit rule search has been done. > > # Implicit/static pattern stem: 'test_maps' > > # File is an intermediate prerequisite. > > # File does not exist. > > # File has not been updated. > > # variable set hash-table stats: > > # Load=1/32=3%, Rehash=0, Collisions=0/2=0% > > # recipe to execute (from '../lib.mk', line 138): > > $(LINK.c) $^ $(LDLIBS) -o $@ > > > > It's because "File is an intermediate prerequisite.", but I > > don't see how it's is a intermediate prerequisite for anything... > > Well, it's "File is an intermediate prerequisite." in both cases, so I > don't know if that's it. > Makefiles is not my strong suit, honestly, and definitely not an area > of passion, so no idea > > > > > > > One other optional suggestion I have to your second patch: maybe drop all > > those dependencies on the directories altogether? Why not do the following > > instead, for example (same for test_progs/test_maps)? > > Some of those _TEST_DIR's are dependencies of multiple targets, so > you'd need to replicate that `mkdir -p $@` in multiple places, which > is annoying. Agreed, but one single "mkdir -p $@" might be better than having three lines that define XXX_TEST_DIR and then add a target that mkdir's it. Up to you, I can give it a try once your changes are in; the Makefile becomes hairier by the day, thx for cleaning it up a bit :-) > But I also don't think we need to worry about creating them, because > there is always at least one test (otherwise those tests are useless > anyways) in those directories, so we might as well just remove those > dependencies, I guess. We probably care about them for "make -C tools/selftests O=$PWD/xyz" case where OUTPUT would point to a fresh/clean directory. > TBH, those explicit dependencies are good to specify anyways, so I > think this fix is good. Second patch just makes the layout of > dependencies similar, so it's easier to spot differences like this > one. > > As usual, I'll let Alexei and Daniel decide which one to apply (or if > we need to take some other approach to fixing this). I agree, both of your changes look good. I was just trying to understand what's happening because I was assuming implicit rules to always kick in. > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > index 1296253b3422..c2d087ce6d4b 100644 > > --- a/tools/testing/selftests/bpf/Makefile > > +++ b/tools/testing/selftests/bpf/Makefile > > @@ -277,12 +277,9 @@ VERIFIER_TESTS_H := $(OUTPUT)/verifier/tests.h > > test_verifier.c: $(VERIFIER_TESTS_H) > > $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS) > > > > -VERIFIER_TESTS_DIR = $(OUTPUT)/verifier > > -$(VERIFIER_TESTS_DIR): > > - mkdir -p $@ > > - > > VERIFIER_TEST_FILES := $(wildcard verifier/*.c) > > -$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR) > > +$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) > > + mkdir -p $(dir $@) > > $(shell ( cd verifier/; \ > > echo '/* Generated header, do not edit */'; \ > > echo '#ifdef FILL_ARRAY'; \
On Tue, Jul 16, 2019 at 5:15 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > On 07/16, Andrii Nakryiko wrote: > > On Tue, Jul 16, 2019 at 3:57 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > On 07/16, Andrii Nakryiko wrote: > > > > On Tue, Jul 16, 2019 at 12:55 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > > > > > > > On 07/16, Andrii Nakryiko wrote: > > > > > > e46fc22e60a4 ("selftests/bpf: make directory prerequisites order-only") > > > > > > exposed existing problem in Makefile for test_verifier and test_maps tests: > > > > > > their dependency on auto-generated header file with a list of all tests wasn't > > > > > > recorded explicitly. This patch fixes these issues. > > > > > Why adding it explicitly fixes it? At least for test_verifier, we have > > > > > the following rule: > > > > > > > > > > test_verifier.c: $(VERIFIER_TESTS_H) > > > > > > > > > > And there should be implicit/builtin test_verifier -> test_verifier.c > > > > > dependency rule. > > > > > > > > > > Same for maps, I guess: > > > > > > > > > > $(OUTPUT)/test_maps: map_tests/*.c > > > > > test_maps.c: $(MAP_TESTS_H) > > > > > > > > > > So why is it not working as is? What I'm I missing? > > > > > > > > I don't know exactly why it's not working, but it's clearly because of > > > > that. It's the only difference between how test_progs are set up, > > > > which didn't break, and test_maps/test_verifier, which did. > > > > > > > > Feel free to figure it out through a maze of Makefiles why it didn't > > > > work as expected, but this definitely fixed a breakage (at least for > > > > me). > > > Agreed on not wasting time. I took a brief look (with make -qp) and I > > > don't have any clue. > > > > Oh, -qp is cool, noted :) > > > > > > > > By default implicit matching doesn't work: > > > # makefile (from 'Makefile', line 261) > > > /linux/tools/testing/selftests/bpf/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) > > > /linux/tools/testing/selftests/bpf/test_maps: map_tests/sk_storage_map.c /linux/tools/testing/selftests/bpf/test_stub.o /linux/tools/testing/selftests/bpf/libbpf.a > > > # Implicit rule search has not been done. > > > # File is an intermediate prerequisite. > > > # Modification time never checked. > > > # File has not been updated. > > > # variable set hash-table stats: > > > # Load=1/32=3%, Rehash=0, Collisions=0/2=0% > > > > > > If I comment out the following line: > > > $(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ) > > > > > > Then it works: > > > # makefile (from 'Makefile', line 261) > > > /linux/tools/testing/selftests/bpf/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) > > > /linux/tools/testing/selftests/bpf/test_maps: test_maps.c map_tests/sk_storage_map.c > > > # Implicit rule search has been done. > > > # Implicit/static pattern stem: 'test_maps' > > > # File is an intermediate prerequisite. > > > # File does not exist. > > > # File has not been updated. > > > # variable set hash-table stats: > > > # Load=1/32=3%, Rehash=0, Collisions=0/2=0% > > > # recipe to execute (from '../lib.mk', line 138): > > > $(LINK.c) $^ $(LDLIBS) -o $@ > > > > > > It's because "File is an intermediate prerequisite.", but I > > > don't see how it's is a intermediate prerequisite for anything... > > > > Well, it's "File is an intermediate prerequisite." in both cases, so I > > don't know if that's it. > > Makefiles is not my strong suit, honestly, and definitely not an area > > of passion, so no idea > > > > > > > > > > > One other optional suggestion I have to your second patch: maybe drop all > > > those dependencies on the directories altogether? Why not do the following > > > instead, for example (same for test_progs/test_maps)? > > > > Some of those _TEST_DIR's are dependencies of multiple targets, so > > you'd need to replicate that `mkdir -p $@` in multiple places, which > > is annoying. > Agreed, but one single "mkdir -p $@" might be better than having three > lines that define XXX_TEST_DIR and then add a target that mkdir's it. > Up to you, I can give it a try once your changes are in; the > Makefile becomes hairier by the day, thx for cleaning it up a bit :-) > > > But I also don't think we need to worry about creating them, because > > there is always at least one test (otherwise those tests are useless > > anyways) in those directories, so we might as well just remove those > > dependencies, I guess. > We probably care about them for "make -C tools/selftests O=$PWD/xyz" case > where OUTPUT would point to a fresh/clean directory. Oh, yes, out-of-tree builds, forgot about that, so yeah, we need that. > > > TBH, those explicit dependencies are good to specify anyways, so I > > think this fix is good. Second patch just makes the layout of > > dependencies similar, so it's easier to spot differences like this > > one. > > > > As usual, I'll let Alexei and Daniel decide which one to apply (or if > > we need to take some other approach to fixing this). > I agree, both of your changes look good. I was just trying to understand > what's happening because I was assuming implicit rules to always kick > in. > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > > index 1296253b3422..c2d087ce6d4b 100644 > > > --- a/tools/testing/selftests/bpf/Makefile > > > +++ b/tools/testing/selftests/bpf/Makefile > > > @@ -277,12 +277,9 @@ VERIFIER_TESTS_H := $(OUTPUT)/verifier/tests.h > > > test_verifier.c: $(VERIFIER_TESTS_H) > > > $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS) > > > > > > -VERIFIER_TESTS_DIR = $(OUTPUT)/verifier > > > -$(VERIFIER_TESTS_DIR): > > > - mkdir -p $@ > > > - > > > VERIFIER_TEST_FILES := $(wildcard verifier/*.c) > > > -$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR) > > > +$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) > > > + mkdir -p $(dir $@) > > > $(shell ( cd verifier/; \ > > > echo '/* Generated header, do not edit */'; \ > > > echo '#ifdef FILL_ARRAY'; \
On Tue, Jul 16, 2019 at 5:22 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > Makefile becomes hairier by the day, thx for cleaning it up a bit :-) > > > > > But I also don't think we need to worry about creating them, because > > > there is always at least one test (otherwise those tests are useless > > > anyways) in those directories, so we might as well just remove those > > > dependencies, I guess. > > We probably care about them for "make -C tools/selftests O=$PWD/xyz" case > > where OUTPUT would point to a fresh/clean directory. > > Oh, yes, out-of-tree builds, forgot about that, so yeah, we need that. Anyhow the patches fixed the issue I was seeing. Hence both applied.
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 1296253b3422..9bc68d8abc5f 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -86,8 +86,6 @@ $(OUTPUT)/urandom_read: $(OUTPUT)/%: %.c $(OUTPUT)/test_stub.o: test_stub.c $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) -c -o $@ $< -$(OUTPUT)/test_maps: map_tests/*.c - BPFOBJ := $(OUTPUT)/libbpf.a $(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ) @@ -257,9 +255,10 @@ MAP_TESTS_DIR = $(OUTPUT)/map_tests $(MAP_TESTS_DIR): mkdir -p $@ MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h +MAP_TESTS_FILES := $(wildcard map_tests/*.c) test_maps.c: $(MAP_TESTS_H) $(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS) -MAP_TESTS_FILES := $(wildcard map_tests/*.c) +$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_H) $(MAP_TESTS_FILES) $(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR) $(shell ( cd map_tests/; \ echo '/* Generated header, do not edit */'; \ @@ -276,6 +275,7 @@ $(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR) VERIFIER_TESTS_H := $(OUTPUT)/verifier/tests.h test_verifier.c: $(VERIFIER_TESTS_H) $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS) +$(OUTPUT)/test_verifier: test_verifier.c $(VERIFIER_TESTS_H) VERIFIER_TESTS_DIR = $(OUTPUT)/verifier $(VERIFIER_TESTS_DIR):
e46fc22e60a4 ("selftests/bpf: make directory prerequisites order-only") exposed existing problem in Makefile for test_verifier and test_maps tests: their dependency on auto-generated header file with a list of all tests wasn't recorded explicitly. This patch fixes these issues. Fixes: 51a0e301a563 ("bpf: Add BPF_MAP_TYPE_SK_STORAGE test to test_maps") Fixes: 6b7b6995c43e ("selftests: bpf: tests.h should depend on .c files, not the output") Cc: Ilya Leoshkevich <iii@linux.ibm.com> Cc: Stanislav Fomichev <sdf@google.com> Cc: Martin KaFai Lau <kafai@fb.com> Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/testing/selftests/bpf/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)