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:
<TAB> ...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.

diff --git a/.github/workflows/dev-long-tests.yml b/.github/workflows/dev-long-tests.yml
index 779ba1c7c19deef51155c0509423708cf91df7fd..3e6fb1e8fd5c9230aefea346a516afd5cec712d5 100644 (file)
--- 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 9f51508415c6184f1c3b17a4f569b877e7e65398..b32c21594ed69305aabf91792bd6dc566abb6a51 100644 (file)
--- 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 32b133b67bfdbd38130cd127e4c2797e562bf681..df842cc941190cb20cd318bff88162597babfa25 100644 (file)
--- 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 cb77b0160a85a160effc984f2cfd449830a452b2..f4318ebfb6ff3388c7c8763b6d7bf259a6d567d8 100644 (file)
--- 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 6fd5ac3bbec8c2f871a5e833f248e8e1e0e20d33..830b294bb6f46a4e4e19d6a15c3b7c7fc50442b8 100644 (file)
--- 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