Subject: Re: [GSoC][PATCH v5 0/7] t: port reftable/stack_test.c to the unit testing framework

From: Junio C Hamano <[email-masked]>
Date: Fri, 06 Sep 2024 16:57:37 -0700

Junio C Hamano <[email-masked]> writes:

> Chandra Pratap <[email-masked]> writes:
>
>> Changes in v5:
>> - Edit the commit messages in patches 3 and 4 to reflect the changes
>>   and the motivation behind those changes better.
>> - Add newlines after variable declarations in patch 6.
>> - Introduce patch 7 which removes leftover cruft from the previous
>>   reftable testing scheme.
>
> Hmph, the end-result looks good to me, but the structure of the
> series is a bit curious.  I didn't expect there will be a separate
> step for removal.  Shouldn't these "leftover cruft" be removed *in*
> the same step that they become cruft (which I am assuming is when
> reftable/stack_test.c and all references to it gets removed in an
> early part of the series)?
>
> Other than that, looking good.
>
> Thanks.

There is another issue.

It is unique to this among the reftable/*_test topics, simply
because this one happens to be the last and needs to clean up a bit
more.

Apparently, GNU "ar" can be invoked without any .o object files and
happily creates an empty archive, but on BSD may not be that may be
the case.  macOS CI jobs seem to be hard failing due to this.

You'd need to squeeze in the following, in addition to [7/7], to
earlier patch(es).

Thanks.

 Makefile | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git c/Makefile w/Makefile
index 64ccb1433f..bdea061971 100644
--- c/Makefile
+++ w/Makefile
@@ -912,7 +912,6 @@ TEST_SHELL_PATH = $(SHELL_PATH)
 LIB_FILE = libgit.a
 XDIFF_LIB = xdiff/lib.a
 REFTABLE_LIB = reftable/libreftable.a
-REFTABLE_TEST_LIB = reftable/libreftable_test.a
 
 GENERATED_H += command-list.h
 GENERATED_H += config-list.h
@@ -2866,9 +2865,6 @@ $(XDIFF_LIB): $(XDIFF_OBJS)
 $(REFTABLE_LIB): $(REFTABLE_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
 
-$(REFTABLE_TEST_LIB): $(REFTABLE_TEST_OBJS)
-	$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
-
 export DEFAULT_EDITOR DEFAULT_PAGER
 
 Documentation/GIT-EXCLUDED-PROGRAMS: FORCE
@@ -3248,7 +3244,7 @@ perf: all
 
 t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o
 
-t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS) $(REFTABLE_TEST_LIB)
+t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
 
 check-sha1:: t/helper/test-tool$X
@@ -3709,7 +3705,7 @@ clean: profile-clean coverage-clean cocciclean
 	$(RM) git.res
 	$(RM) $(OBJECTS)
 	$(RM) headless-git.o
-	$(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(REFTABLE_TEST_LIB)
+	$(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS)
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) $(FUZZ_PROGRAMS)