From c0b46738b4bae20ff0c316fa3f1c4b02c9c2b088 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 7 Sep 2022 16:10:13 -0700 Subject: [PATCH 1/2] 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 From 5129b4ab101165ee04104436a7202a6a4be6106a Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 12 Sep 2022 12:18:51 -0700 Subject: [PATCH 2/2] simplify clean target maintenance within programs/ --- programs/Makefile | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index a5f5b45f2..3e705e84d 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -129,10 +129,10 @@ SET_CACHE_DIRECTORY = \ .PHONY: all -all: zstd +all: zstd zstd-compress zstd-decompress zstd-small .PHONY: allVariants -allVariants: zstd zstd-compress zstd-decompress zstd-small zstd-frugal zstd-nolegacy zstd-dictBuilder +allVariants: all zstd-frugal zstd-nolegacy zstd-dictBuilder .PHONY: zstd # must always be run zstd : CPPFLAGS += $(THREAD_CPP) $(ZLIBCPP) $(LZMACPP) $(LZ4CPP) @@ -180,11 +180,13 @@ zstd : $(BUILD_DIR)/zstd endif # BUILD_DIR +CLEAN += zstd .PHONY: zstd-release zstd-release: DEBUGFLAGS := -DBACKTRACE_ENABLE=0 zstd-release: DEBUGFLAGS_LD := zstd-release: zstd +CLEAN += zstd32 zstd32 : CPPFLAGS += $(THREAD_CPP) zstd32 : LDFLAGS += $(THREAD_LD) zstd32 : CPPFLAGS += -DZSTD_LEGACY_SUPPORT=$(ZSTD_LEGACY_SUPPORT) @@ -194,7 +196,8 @@ endif zstd32 : $(ZSTDLIB_FULL_SRC) $(ZSTD_CLI_SRC) $(CC) -m32 $(FLAGS) $^ -o $@$(EXT) -## zstd-nolegacy: same scope as zstd, with just support of legacy formats removed +## zstd-nolegacy: same scope as zstd, with removed support of legacy formats +CLEAN += zstd-nolegacy zstd-nolegacy : LDFLAGS += $(THREAD_LD) $(ZLIBLD) $(LZMALD) $(LZ4LD) $(DEBUGFLAGS_LD) zstd-nolegacy : CPPFLAGS += -UZSTD_LEGACY_SUPPORT -DZSTD_LEGACY_SUPPORT=0 zstd-nolegacy : $(ZSTDLIB_CORE_SRC) $(ZDICT_SRC) $(ZSTD_CLI_OBJ) @@ -242,20 +245,25 @@ zstd-pgo : $(MAKE) zstd MOREFLAGS=-fprofile-use ## zstd-small: minimal target, supporting only zstd compression and decompression. no bench. no legacy. no other format. +CLEAN += zstd-small zstd-frugal zstd-small: CFLAGS = -Os -Wl,-s zstd-frugal zstd-small: $(ZSTDLIB_CORE_SRC) zstdcli.c util.c timefn.c fileio.c fileio_asyncio.c $(CC) $(FLAGS) -DZSTD_NOBENCH -DZSTD_NODICT -DZSTD_NOTRACE -UZSTD_LEGACY_SUPPORT -DZSTD_LEGACY_SUPPORT=0 $^ -o $@$(EXT) +CLEAN += zstd-decompress zstd-decompress: $(ZSTDLIB_COMMON_SRC) $(ZSTDLIB_DECOMPRESS_SRC) zstdcli.c util.c timefn.c fileio.c fileio_asyncio.c $(CC) $(FLAGS) -DZSTD_NOBENCH -DZSTD_NODICT -DZSTD_NOCOMPRESS -DZSTD_NOTRACE -UZSTD_LEGACY_SUPPORT -DZSTD_LEGACY_SUPPORT=0 $^ -o $@$(EXT) +CLEAN += zstd-compress zstd-compress: $(ZSTDLIB_COMMON_SRC) $(ZSTDLIB_COMPRESS_SRC) zstdcli.c util.c timefn.c fileio.c fileio_asyncio.c $(CC) $(FLAGS) -DZSTD_NOBENCH -DZSTD_NODICT -DZSTD_NODECOMPRESS -DZSTD_NOTRACE -UZSTD_LEGACY_SUPPORT -DZSTD_LEGACY_SUPPORT=0 $^ -o $@$(EXT) ## zstd-dictBuilder: executable supporting dictionary creation and compression (only) +CLEAN += zstd-dictBuilder zstd-dictBuilder: $(ZSTDLIB_COMMON_SRC) $(ZSTDLIB_COMPRESS_SRC) $(ZDICT_SRC) zstdcli.c util.c timefn.c fileio.c fileio_asyncio.c dibio.c $(CC) $(FLAGS) -DZSTD_NOBENCH -DZSTD_NODECOMPRESS -DZSTD_NOTRACE $^ -o $@$(EXT) +CLEAN += zstdmt zstdmt: zstd ln -sf zstd zstdmt @@ -273,12 +281,8 @@ endif .PHONY: clean clean: - $(RM) core *.o tmp* result* *.gcda dictionary *.zst \ - zstd$(EXT) zstd32$(EXT) zstd-dll$(EXT) \ - zstd-compress$(EXT) zstd-decompress$(EXT) \ - zstd-small$(EXT) zstd-frugal$(EXT) zstd-nolegacy$(EXT) zstd4$(EXT) \ - zstd-dictBuilder$(EXT) \ - *.gcda default*.profraw default.profdata have_zlib$(EXT) + $(RM) $(CLEAN) core *.o tmp* result* dictionary *.zst \ + *.gcda default*.profraw default.profdata have_zlib $(RM) -r obj/* @echo Cleaning completed