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.

Author: Cyan4973

.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
 

.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

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

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)

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