{"thread":{"id":"62093","subject":"[PATCH] Makefile: rename clar-related variables to avoid confusion","startedAt":"2024-09-10T06:23:53Z","lastAt":"2024-09-10T17:35:46Z","messageCount":2,"participants":["Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"502537","messageId":"0ee5282c64c51a2c8c06582510b3ca7ec7173c42.1725949388.git.ps@pks.im","threadId":"62093","inReplyTo":null,"subject":"[PATCH] Makefile: rename clar-related variables to avoid confusion","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-10T06:23:45Z","receivedAt":"2024-09-10T06:23:53Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The Makefile variables related to the recently-introduced clar testing\nframework have a `UNIT_TESTS_` prefix. This prefix is extremely similar\nto the prefix used by our other unit tests that use our homegrown unit\ntesting framework, which is `UNIT_TEST_`. The consequence is that it is\neasy to misread the names and confuse them with each other.\n\nRename the clar-related variables to instead have a `CLAR_TEST_` prefix\nto address this.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n\nAs discussed in <xmqqfrq8r8s8.fsf@gitster.g>. This is based on top of\nJunio's ps/clar-unit-test at 894deb76a0 (clar: add CMake support,\n2024-09-04).\n\nPatrick\n\n Makefile | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c841cf7006..4ef1f1dfe3 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1336,12 +1336,12 @@ THIRD_PARTY_SOURCES += sha1dc/%\n THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/%\n THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%\n \n-UNIT_TESTS_SUITES += ctype\n-UNIT_TESTS_SUITES += strvec\n-UNIT_TESTS_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n-UNIT_TESTS_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TESTS_SUITES))\n-UNIT_TESTS_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n-UNIT_TESTS_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n+CLAR_TEST_SUITES += ctype\n+CLAR_TEST_SUITES += strvec\n+CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)\n+CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))\n+CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o\n+CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o\n \n UNIT_TEST_PROGRAMS += t-example-decorate\n UNIT_TEST_PROGRAMS += t-hash\n@@ -2721,7 +2721,7 @@ OBJECTS += $(XDIFF_OBJS)\n OBJECTS += $(FUZZ_OBJS)\n OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)\n OBJECTS += $(UNIT_TEST_OBJS)\n-OBJECTS += $(UNIT_TESTS_OBJS)\n+OBJECTS += $(CLAR_TEST_OBJS)\n \n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n@@ -3224,7 +3224,7 @@ endif\n \n test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n \n-all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(UNIT_TESTS_PROG)\n+all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)\n \n bin-wrappers/%: wrap-for-bin.sh\n \t$(call mkdir_p_parent_template)\n@@ -3656,7 +3656,7 @@ endif\n \n artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \\\n \t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \\\n-\t\t$(UNIT_TEST_PROGS) $(UNIT_TESTS_PROG) $(MOFILES)\n+\t\t$(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) $(MOFILES)\n \t$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \\\n \t\tSHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'\n \ttest -n \"$(ARTIFACTS_DIRECTORY)\"\n@@ -3873,25 +3873,25 @@ $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o \\\n \t\t$(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n \n GIT-TEST-SUITES: FORCE\n-\t@FLAGS='$(UNIT_TESTS_SUITES)'; \\\n+\t@FLAGS='$(CLAR_TEST_SUITES)'; \\\n \t    if test x\"$$FLAGS\" != x\"`cat GIT-TEST-SUITES 2>/dev/null`\" ; then \\\n \t\techo >&2 \"    * new test suites\"; \\\n \t\techo \"$$FLAGS\" >GIT-TEST-SUITES; \\\n             fi\n \n-$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(UNIT_TESTS_SUITES)) GIT-TEST-SUITES\n-\t$(QUIET_GEN)for suite in $(UNIT_TESTS_SUITES); do \\\n+$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n+\t$(QUIET_GEN)for suite in $(CLAR_TEST_SUITES); do \\\n \t\tsed -ne \"s/^\\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$$\\)/extern \\1;/p\" $(UNIT_TEST_DIR)/$$suite.c; \\\n \tdone >$@\n $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h\n \t$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite\n-$(UNIT_TESTS_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n-$(UNIT_TESTS_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)\n-$(UNIT_TESTS_PROG): $(UNIT_TEST_DIR)/clar.suite $(UNIT_TESTS_OBJS) $(GITLIBS) GIT-LDFLAGS\n+$(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n+$(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)\n+$(CLAR_TEST_PROG): $(UNIT_TEST_DIR)/clar.suite $(CLAR_TEST_OBJS) $(GITLIBS) GIT-LDFLAGS\n \t$(call mkdir_p_parent_template)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n \n .PHONY: build-unit-tests unit-tests\n-build-unit-tests: $(UNIT_TEST_PROGS) $(UNIT_TESTS_PROG)\n-unit-tests: $(UNIT_TEST_PROGS) $(UNIT_TESTS_PROG) t/helper/test-tool$X\n+build-unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)\n+unit-tests: $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) t/helper/test-tool$X\n \t$(MAKE) -C t/ unit-tests\n-- \n2.46.0.519.g2e7b89e038.dirty\n\n"},{"id":"502578","messageId":"xmqqbk0vfm34.fsf@gitster.g","threadId":"62093","inReplyTo":"0ee5282c64c51a2c8c06582510b3ca7ec7173c42.1725949388.git.ps@pks.im","subject":"Re: [PATCH] Makefile: rename clar-related variables to avoid confusion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-10T17:35:43Z","receivedAt":"2024-09-10T17:35:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The Makefile variables related to the recently-introduced clar testing\n> framework have a `UNIT_TESTS_` prefix. This prefix is extremely similar\n> to the prefix used by our other unit tests that use our homegrown unit\n> testing framework, which is `UNIT_TEST_`. The consequence is that it is\n> easy to misread the names and confuse them with each other.\n>\n> Rename the clar-related variables to instead have a `CLAR_TEST_` prefix\n> to address this.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>\n> As discussed in <xmqqfrq8r8s8.fsf@gitster.g>. This is based on top of\n> Junio's ps/clar-unit-test at 894deb76a0 (clar: add CMake support,\n> 2024-09-04).\n\nThanks, looking good.  Let me queue it directly on top of that\ntopic, and have it graduate at the same time as the rest of the\ntopic.\n\n"}]}