From c0b46738b4bae20ff0c316fa3f1c4b02c9c2b088 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 7 Sep 2022 16:10:13 -0700 Subject: [PATCH] streamline `make clean` list maintenance When creating a new `Makefile` target to build, it's also necessary to update the `clean` target, which purpose is to remove built targets when they are present. This process is simple, but it's also easy to forget : since there is a large distance between the position in the `Makefile` where the new built target is added, and the place where the list of files to `clean` is defined. Moreover, the list of files becomes pretty long over time, hence it's difficult to visually ensure that all built targets are present there, or that no old target (no longer produced) is no longer in the list This PR tries to improve this process by adding a CLEAN variable. Now, when a new built target is added to the `Makefile`, it should preceded by : ``` CLEAN += newTarget newTarget: ...recipe... ``` This new requirement is somewhat similar to `.PHONY: newTarget` for non-built targets. This new method offers the advantage of locality : there is no separate place in the file to maintain a list of files to clean. This makes maintenance of `make clean` easier. --- .github/workflows/dev-long-tests.yml | 2 +- .github/workflows/dev-short-tests.yml | 2 +- TESTING.md | 2 +- tests/Makefile | 66 +++++++++++++++++---------- zlibWrapper/Makefile | 7 +-- 5 files changed, 49 insertions(+), 30 deletions(-) diff --git a/.github/workflows/dev-long-tests.yml b/.github/workflows/dev-long-tests.yml index 779ba1c7c..3e6fb1e8f 100644 --- a/.github/workflows/dev-long-tests.yml +++ b/.github/workflows/dev-long-tests.yml @@ -157,7 +157,7 @@ jobs: run: | sudo apt-get -qqq update make valgrindinstall - make -C tests valgrindTest + make -C tests test-valgrind make clean make -C tests test-fuzzer-stackmode diff --git a/.github/workflows/dev-short-tests.yml b/.github/workflows/dev-short-tests.yml index 9f5150841..b32c21594 100644 --- a/.github/workflows/dev-short-tests.yml +++ b/.github/workflows/dev-short-tests.yml @@ -127,7 +127,7 @@ jobs: sudo apt-get -qqq update make valgrindinstall make -C zlibWrapper test - make -C zlibWrapper valgrindTest + make -C zlibWrapper test-valgrind lz4-threadpool-libs: runs-on: ubuntu-latest diff --git a/TESTING.md b/TESTING.md index 32b133b67..df842cc94 100644 --- a/TESTING.md +++ b/TESTING.md @@ -22,7 +22,7 @@ They consist of the following tests: - `tests/playTests.sh --test-large-data` - Fuzzer tests: `tests/fuzzer.c`, `tests/zstreamtest.c`, and `tests/decodecorpus.c` - `tests/zstreamtest.c` under Tsan (streaming mode, including multithreaded mode) -- Valgrind Test (`make -C tests valgrindTest`) (testing CLI and fuzzer under valgrind) +- Valgrind Test (`make -C tests test-valgrind`) (testing CLI and fuzzer under `valgrind`) - Fuzzer tests (see above) on ARM, AArch64, PowerPC, and PowerPC64 Long Tests diff --git a/tests/Makefile b/tests/Makefile index cb77b0160..f4318ebfb 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -1,5 +1,5 @@ - # ################################################################ +# ################################################################ # Copyright (c) Yann Collet, Facebook, Inc. # All rights reserved. # @@ -37,7 +37,7 @@ TESTARTEFACT := versionsTest DEBUGFLAGS += -g -Wno-c++-compat CPPFLAGS += -I$(ZSTDDIR) -I$(ZSTDDIR)/common -I$(ZSTDDIR)/compress \ -I$(ZSTDDIR)/dictBuilder -I$(ZSTDDIR)/deprecated -I$(PRGDIR) \ - -DZSTD_WINDOW_OVERFLOW_CORRECT_FREQUENTLY=1 + -DZSTD_WINDOW_OVERFLOW_CORRECT_FREQUENTLY=1 ZSTDCOMMON_FILES := $(sort $(ZSTD_COMMON_FILES)) ZSTDCOMP_FILES := $(sort $(ZSTD_COMPRESS_FILES)) @@ -134,14 +134,17 @@ zstdmt_d_%.o : $(ZSTDDIR)/decompress/%.c zstdmt_d_%.o : $(ZSTDDIR)/decompress/%.S $(CC) -c $(CPPFLAGS) $(ASFLAGS) $< -o $@ +FULLBENCHS := fullbench fullbench32 +CLEAN += $(FULLBENCHS) fullbench32: CPPFLAGS += -m32 -fullbench fullbench32 : CPPFLAGS += $(MULTITHREAD_CPP) -Wno-deprecated-declarations -fullbench fullbench32 : LDFLAGS += $(MULTITHREAD_LD) -fullbench fullbench32 : DEBUGFLAGS = -DNDEBUG # turn off assert() for speed measurements -fullbench fullbench32 : $(ZSTD_FILES) -fullbench fullbench32 : $(PRGDIR)/datagen.c $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/benchfn.c fullbench.c +$(FULLBENCHS) : CPPFLAGS += $(MULTITHREAD_CPP) -Wno-deprecated-declarations +$(FULLBENCHS) : LDFLAGS += $(MULTITHREAD_LD) +$(FULLBENCHS) : DEBUGFLAGS = -DNDEBUG # turn off assert() for speed measurements +$(FULLBENCHS) : $(ZSTD_FILES) +$(FULLBENCHS) : $(PRGDIR)/datagen.c $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/benchfn.c fullbench.c $(LINK.c) $^ -o $@$(EXT) +CLEAN += fullbench-lib fullbench-lib : CPPFLAGS += -DXXH_NAMESPACE=ZSTD_ fullbench-lib : $(PRGDIR)/datagen.c $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/benchfn.c $(ZSTDDIR)/libzstd.a fullbench.c $(LINK.c) $^ -o $@$(EXT) @@ -151,6 +154,7 @@ fullbench-dll: $(PRGDIR)/datagen.c $(PRGDIR)/util.c $(PRGDIR)/benchfn.c $(PRGDIR # $(CC) $(FLAGS) $(filter %.c,$^) -o $@$(EXT) -DZSTD_DLL_IMPORT=1 $(ZSTDDIR)/dll/libzstd.dll $(LINK.c) $^ $(LDLIBS) -o $@$(EXT) +CLEAN += fuzzer fuzzer32 fuzzer : CPPFLAGS += $(MULTITHREAD_CPP) -Wno-deprecated-declarations fuzzer : LDFLAGS += $(MULTITHREAD_LD) fuzzer : $(ZSTDMT_OBJECTS) @@ -164,6 +168,7 @@ fuzzer32 : $(ZSTD_FILES) fuzzer-dll : $(ZSTDDIR)/common/xxhash.c $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/datagen.c fuzzer.c $(CC) $(CPPFLAGS) $(CFLAGS) $(filter %.c,$^) $(LDFLAGS) -o $@$(EXT) +CLEAN += zstreamtest zstreamtest32 ZSTREAM_LOCAL_FILES := $(PRGDIR)/datagen.c $(PRGDIR)/util.c $(PRGDIR)/timefn.c seqgen.c zstreamtest.c ZSTREAM_PROPER_FILES := $(ZDICT_FILES) $(ZSTREAM_LOCAL_FILES) ZSTREAMFILES := $(ZSTD_FILES) $(ZSTREAM_PROPER_FILES) @@ -175,10 +180,12 @@ zstreamtest32 : $(ZSTREAMFILES) zstreamtest zstreamtest32 : $(LINK.c) $^ -o $@$(EXT) +CLEAN += zstreamtest_asan zstreamtest_asan : CFLAGS += -fsanitize=address zstreamtest_asan : $(ZSTREAMFILES) $(LINK.c) $(MULTITHREAD) $^ -o $@$(EXT) +CLEAN += zstreamtest_tsan zstreamtest_tsan : CFLAGS += -fsanitize=thread zstreamtest_tsan : $(ZSTREAMFILES) $(LINK.c) $(MULTITHREAD) $^ -o $@$(EXT) @@ -188,29 +195,38 @@ zstreamtest-dll : $(ZSTDDIR)/common/xxhash.c # xxh symbols not exposed from dll zstreamtest-dll : $(ZSTREAM_LOCAL_FILES) $(CC) $(CPPFLAGS) $(CFLAGS) $(filter %.c,$^) $(LDFLAGS) -o $@$(EXT) +CLEAN += paramgrill paramgrill : DEBUGFLAGS = # turn off debug for speed measurements paramgrill : LDLIBS += -lm paramgrill : $(ZSTD_FILES) $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/benchfn.c $(PRGDIR)/benchzstd.c $(PRGDIR)/datagen.c paramgrill.c +CLEAN += datagen datagen : $(PRGDIR)/datagen.c datagencli.c $(LINK.c) $^ -o $@$(EXT) +CLEAN += roundTripCrash roundTripCrash: CFLAGS += $(MULTITHREAD) roundTripCrash : $(ZSTD_OBJECTS) roundTripCrash.c +CLEAN += longmatch longmatch : $(ZSTD_OBJECTS) longmatch.c +CLEAN += bigdict bigdict: CFLAGS += $(MULTITHREAD) bigdict: $(ZSTDMT_OBJECTS) $(PRGDIR)/datagen.c bigdict.c +CLEAN += invalidDictionaries invalidDictionaries : $(ZSTD_OBJECTS) invalidDictionaries.c +CLEAN += legacy legacy : CPPFLAGS += -I$(ZSTDDIR)/legacy -UZSTD_LEGACY_SUPPORT -DZSTD_LEGACY_SUPPORT=4 legacy : $(ZSTD_FILES) $(sort $(wildcard $(ZSTDDIR)/legacy/*.c)) legacy.c +CLEAN += decodecorpus decodecorpus : LDLIBS += -lm decodecorpus : $(filter-out zstdc_zstd_compress.o, $(ZSTD_OBJECTS)) $(ZDICT_FILES) $(PRGDIR)/util.c $(PRGDIR)/timefn.c decodecorpus.c +CLEAN += poolTests poolTests : $(PRGDIR)/util.c $(PRGDIR)/timefn.c poolTests.c $(ZSTDDIR)/common/pool.c $(ZSTDDIR)/common/threading.c $(ZSTDDIR)/common/zstd_common.c $(ZSTDDIR)/common/error_private.c $(LINK.c) $(MULTITHREAD) $^ -o $@$(EXT) @@ -222,7 +238,8 @@ versionsTest: clean automated_benchmarking: clean $(PYTHON) automated_benchmarking.py -# make checkTag +# make checkTag : check that release tag corresponds to release version +CLEAN += checkTag checkTag.o : $(ZSTDDIR)/zstd.h .PHONY: clean @@ -231,28 +248,22 @@ clean: $(MAKE) -C $(PRGDIR) clean $(RM) -fR $(TESTARTEFACT) $(RM) -rf tmp* # some test directories are named tmp* - $(RM) core *.o *.tmp result* *.gcda dictionary *.zst \ + $(RM) $(CLEAN) core *.o *.tmp result* *.gcda dictionary *.zst \ $(PRGDIR)/zstd$(EXT) $(PRGDIR)/zstd32$(EXT) \ - fullbench$(EXT) fullbench32$(EXT) \ - fullbench-lib$(EXT) fullbench-dll$(EXT) \ - fuzzer$(EXT) fuzzer32$(EXT) \ - fuzzer-dll$(EXT) zstreamtest-dll$(EXT) \ - zstreamtest$(EXT) zstreamtest32$(EXT) \ - datagen$(EXT) paramgrill$(EXT) roundTripCrash$(EXT) longmatch$(EXT) \ - symbols$(EXT) invalidDictionaries$(EXT) legacy$(EXT) poolTests$(EXT) \ - decodecorpus$(EXT) checkTag$(EXT) bigdict$(EXT) + fullbench-dll$(EXT) fuzzer-dll$(EXT) zstreamtest-dll$(EXT) @echo Cleaning completed #---------------------------------------------------------------------------------- -# valgrind tests are validated only for some posix platforms +# valgrind tests validated only for some posix platforms #---------------------------------------------------------------------------------- UNAME := $(shell uname) ifneq (,$(filter $(UNAME),Linux Darwin GNU/kFreeBSD GNU OpenBSD FreeBSD NetBSD DragonFly SunOS AIX)) HOST_OS = POSIX -valgrindTest: VALGRIND = valgrind --leak-check=full --show-leak-kinds=all --error-exitcode=1 -valgrindTest: zstd datagen fuzzer fullbench +.PHONY: test-valgrind +test-valgrind: VALGRIND = valgrind --leak-check=full --show-leak-kinds=all --error-exitcode=1 +test-valgrind: zstd datagen fuzzer fullbench @echo "\n ---- valgrind tests : memory analyzer ----" $(VALGRIND) ./datagen -g50M > $(VOID) $(VALGRIND) $(PRGDIR)/zstd ; if [ $$? -eq 0 ] ; then echo "zstd without argument should have failed"; false; fi @@ -261,7 +272,7 @@ valgrindTest: zstd datagen fuzzer fullbench ./datagen -g2930KB | $(VALGRIND) $(PRGDIR)/zstd -5 -vf - -o tmp $(VALGRIND) $(PRGDIR)/zstd -vdf tmp -c > $(VOID) ./datagen -g64MB | $(VALGRIND) $(PRGDIR)/zstd -vf - -c > $(VOID) - @rm tmp + $(RM) tmp $(VALGRIND) ./fuzzer -T1mn -t1 $(VALGRIND) ./fullbench -i1 @@ -306,9 +317,9 @@ endif test32: test-zstd32 test-fullbench32 test-fuzzer32 test-zstream32 .PHONY: test-all -test-all: test test32 valgrindTest test-decodecorpus-cli +test-all: test test32 test-decodecorpus-cli -.PHONY: test-zstd test-zstd32 test-zstd-nolegacy test-zstdgrep +.PHONY: test-zstd test-zstd32 test-zstd-nolegacy test-zstd: ZSTD = $(PRGDIR)/zstd test-zstd: zstd @@ -322,29 +333,36 @@ test-zstd test-zstd32 test-zstd-nolegacy: datagen file $(ZSTD) EXE_PREFIX="$(QEMU_SYS)" ZSTD_BIN="$(ZSTD)" DATAGEN_BIN=./datagen ./playTests.sh $(ZSTDRTTEST) +.PHONY: test-cli-tests test-cli-tests: ZSTD = $(PRGDIR)/zstd test-cli-tests: zstd datagen file $(ZSTD) ./cli-tests/run.py --exec-prefix="$(QEMU_SYS)" --zstd="$(ZSTD)" --datagen=./datagen - +.PHONY: test-fullbench test-fullbench: fullbench datagen $(QEMU_SYS) ./fullbench -i1 $(QEMU_SYS) ./fullbench -i1 -P0 +.PHONY: test-fullbench32 test-fullbench32: fullbench32 datagen $(QEMU_SYS) ./fullbench32 -i1 $(QEMU_SYS) ./fullbench32 -i1 -P0 +.PHONY: test-fuzzer test-fuzzer: fuzzer $(QEMU_SYS) ./fuzzer -v $(FUZZERTEST) $(FUZZER_FLAGS) +# Note : this test presumes `fuzzer` will be built +.PHONY: test-fuzzer-stackmode test-fuzzer-stackmode: MOREFLAGS += -DZSTD_HEAPMODE=0 test-fuzzer-stackmode: test-fuzzer +.PHONY: test-fuzzer32 test-fuzzer32: fuzzer32 $(QEMU_SYS) ./fuzzer32 -v $(FUZZERTEST) $(FUZZER_FLAGS) +.PHONY: test-zstream test-zstream: zstreamtest $(QEMU_SYS) ./zstreamtest -v $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) $(QEMU_SYS) ./zstreamtest --newapi -t1 $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) diff --git a/zlibWrapper/Makefile b/zlibWrapper/Makefile index 6fd5ac3bb..830b294bb 100644 --- a/zlibWrapper/Makefile +++ b/zlibWrapper/Makefile @@ -66,9 +66,10 @@ test: example fitblk example_zstd fitblk_zstd zwrapbench minigzip minigzip_zstd ./zwrapbench -qi1b3B1K $(TEST_FILE) ./zwrapbench -rqi1b1e3 ../lib -#valgrindTest: ZSTDLIBRARY = $(ZSTDLIBDIR)/libzstd.so -valgrindTest: VALGRIND = LD_LIBRARY_PATH=$(ZSTDLIBDIR) valgrind --track-origins=yes --leak-check=full --error-exitcode=1 -valgrindTest: clean example fitblk example_zstd fitblk_zstd zwrapbench +.PHONY: test-valgrind +#test-valgrind: ZSTDLIBRARY = $(ZSTDLIBDIR)/libzstd.so +test-valgrind: VALGRIND = LD_LIBRARY_PATH=$(ZSTDLIBDIR) valgrind --track-origins=yes --leak-check=full --error-exitcode=1 +test-valgrind: clean example fitblk example_zstd fitblk_zstd zwrapbench @echo "\n ---- valgrind tests ----" $(VALGRIND) ./example $(VALGRIND) ./example_zstd