Makefile: rename clar-related variables to avoid confusion

Commit Message

Patrick Steinhardt Sept. 10, 2024, 6:23 a.m. UTC

The Makefile variables related to the recently-introduced clar testing
framework have a `UNIT_TESTS_` prefix. This prefix is extremely similar
to the prefix used by our other unit tests that use our homegrown unit
testing framework, which is `UNIT_TEST_`. The consequence is that it is
easy to misread the names and confuse them with each other.

Rename the clar-related variables to instead have a `CLAR_TEST_` prefix
to address this.

Signed-off-by: Patrick Steinhardt <[e-mail]>
---

As discussed in <xmqqfrq8r8s8.fsf@gitster.g>. This is based on top of
Junio's ps/clar-unit-test at 894deb76a0 (clar: add CMake support,
2024-09-04).

Patrick

 Makefile | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Comments

Junio C Hamano Sept. 10, 2024, 5:35 p.m. UTC | #1

Patrick Steinhardt <[e-mail]> writes:

> The Makefile variables related to the recently-introduced clar testing
> framework have a `UNIT_TESTS_` prefix. This prefix is extremely similar
> to the prefix used by our other unit tests that use our homegrown unit
> testing framework, which is `UNIT_TEST_`. The consequence is that it is
> easy to misread the names and confuse them with each other.
>
> Rename the clar-related variables to instead have a `CLAR_TEST_` prefix
> to address this.
>
> Signed-off-by: Patrick Steinhardt <[e-mail]>
> ---
>
> As discussed in <xmqqfrq8r8s8.fsf@gitster.g>. This is based on top of
> Junio's ps/clar-unit-test at 894deb76a0 (clar: add CMake support,
> 2024-09-04).

Thanks, looking good.  Let me queue it directly on top of that
topic, and have it graduate at the same time as the rest of the
topic.

Patch

diff --git a/Makefile b/Makefile
index c841cf7006..4ef1f1dfe3 100644
--- a/Makefile
+++ b/Makefile
@@ -1336,12 +1336,12 @@  THIRD_PARTY_SOURCES += sha1dc/%
 THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%
 THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%
 
-UNIT_TESTS_SUITES += ctype
-UNIT_TESTS_SUITES += strvec
-UNIT_TESTS_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)
-UNIT_TESTS_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TESTS_SUITES))
-UNIT_TESTS_OBJS += $(UNIT_TEST_DIR)/clar/clar.o
-UNIT_TESTS_OBJS += $(UNIT_TEST_DIR)/unit-test.o
+CLAR_TEST_SUITES += ctype
+CLAR_TEST_SUITES += strvec
+CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)
+CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))
+CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o
+CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
 
 UNIT_TEST_PROGRAMS += t-example-decorate
 UNIT_TEST_PROGRAMS += t-hash
@@ -2721,7 +2721,7 @@  OBJECTS += $(XDIFF_OBJS)
 OBJECTS += $(FUZZ_OBJS)
 OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
 OBJECTS += $(UNIT_TEST_OBJS)
-OBJECTS += $(UNIT_TESTS_OBJS)
+OBJECTS += $(CLAR_TEST_OBJS)
 
 ifndef NO_CURL
 	OBJECTS += http.o http-walker.o remote-curl.o
@@ -3224,7 +3224,7 @@  endif
 
 test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 
-all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(UNIT_TESTS_PROG)
+all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)
 
 bin-wrappers/%: wrap-for-bin.sh
 	$(call mkdir_p_parent_template)
@@ -3656,7 +3656,7 @@  endif
 
 artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
 		GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
-		$(UNIT_TEST_PROGS) $(UNIT_TESTS_PROG) $(MOFILES)
+		$(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) $(MOFILES)
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
 		SHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'
 	test -n "$(ARTIFACTS_DIRECTORY)"
@@ -3873,25 +3873,25 @@  $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o \
 		$(filter %.o,$^) $(filter %.a,$^) $(LIBS)
 
 GIT-TEST-SUITES: FORCE
-	@FLAGS='$(UNIT_TESTS_SUITES)'; \
+	@FLAGS='$(CLAR_TEST_SUITES)'; \
 	    if test x"$$FLAGS" != x"`cat GIT-TEST-SUITES 2>/dev/null`" ; then \
 		echo >&2 "    * new test suites"; \
 		echo "$$FLAGS" >GIT-TEST-SUITES; \
             fi
 
-$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(UNIT_TESTS_SUITES)) GIT-TEST-SUITES
-	$(QUIET_GEN)for suite in $(UNIT_TESTS_SUITES); do \
+$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES
+	$(QUIET_GEN)for suite in $(CLAR_TEST_SUITES); do \
 		sed -ne "s/^\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$$\)/extern \1;/p" $(UNIT_TEST_DIR)/$$suite.c; \
 	done >$@
 $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
 	$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite
-$(UNIT_TESTS_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
-$(UNIT_TESTS_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)
-$(UNIT_TESTS_PROG): $(UNIT_TEST_DIR)/clar.suite $(UNIT_TESTS_OBJS) $(GITLIBS) GIT-LDFLAGS
+$(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
+$(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)
+$(CLAR_TEST_PROG): $(UNIT_TEST_DIR)/clar.suite $(CLAR_TEST_OBJS) $(GITLIBS) GIT-LDFLAGS
 	$(call mkdir_p_parent_template)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
 .PHONY: build-unit-tests unit-tests
-build-unit-tests: $(UNIT_TEST_PROGS) $(UNIT_TESTS_PROG)
-unit-tests: $(UNIT_TEST_PROGS) $(UNIT_TESTS_PROG) t/helper/test-tool$X
+build-unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)
+unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) t/helper/test-tool$X
 	$(MAKE) -C t/ unit-tests