"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
> -	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)

I think the reason why the pattern to use only the .o files among
the prerequisites and then use only the .a files among the same
prerequisites (both filters $^) is used here is to make sure that the
linker sees object files first before library archives, so that by
the time its left-to-right scan sees the first library archive, all
the missing symbols in the object files are known.  The above change
depends on LIBS being a strict superset of all the library archive
files ($GITLIBS in the current code, but that can be updated in the
future) listed as prerequisites for the rule, but there is nothing to
guarantee that, so it looks brittle.

Exact same comment applies to the other two rules touched by this patch.

On Thu, Jun 4, 2026 at 2:33 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> >  t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
> > -     $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> > +     $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>
> I think the reason why the pattern to use only the .o files among
> the prerequisites and then use only the .a files among the same
> prerequisites (both filters $^) is used here is to make sure that the
> linker sees object files first before library archives, so that by
> the time its left-to-right scan sees the first library archive, all
> the missing symbols in the object files are known.  The above change
> depends on LIBS being a strict superset of all the library archive
> files ($GITLIBS in the current code, but that can be updated in the
> future) listed as prerequisites for the rule, but there is nothing to
> guarantee that, so it looks brittle.
>
> Exact same comment applies to the other two rules touched by this patch.

Hmm, there are other constructs like this that rely on $(LIBS) being a
superset of the archives, so the three rules changed here align with
the trend rather than introduce a new trend.

Not saying we shouldn't find a way to handle the overall brittleness.


Harald

Maybe we can do this to get around the brittleness for all ~10 places:

```
-LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
+LIBS = $(filter %.a,$^) $(filter-out $(filter %.a,$^),$(filter-out
%.o,$(GITLIBS)) $(EXTLIBS))

 BASIC_CFLAGS += $(COMPAT_CFLAGS)
 LIB_OBJS += $(COMPAT_OBJS)
@@ -3392,7 +3395,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)
- $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter
%.o,$^) $(filter %.a,$^) $(LIBS)
+ $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)

 check-sha1:: t/helper/test-tool$X
  t/helper/test-sha1.sh
@@ -4015,13 +4018,13 @@ fuzz-all: $(FUZZ_PROGRAMS)
 $(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS
  $(QUIET_LINK)$(FUZZ_CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \
  -Wl,--allow-multiple-definition \
- $(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)
+ $(filter %.o,$^) $(LIBS) $(LIB_FUZZING_ENGINE)

 $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o
$(UNIT_TEST_OBJS) \
  $(GITLIBS) GIT-LDFLAGS
  $(call mkdir_p_parent_template)
  $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
- $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
+ $(filter %.o,$^) $(LIBS)

 GIT-TEST-SUITES: FORCE
  @FLAGS='$(CLAR_TEST_SUITES)'; \
```


Harald

On Thu, Jun 4, 2026 at 9:06 AM Harald Nordgren <haraldnordgren@gmail.com> wrote:
>
> On Thu, Jun 4, 2026 at 2:33 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> > >  t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
> > > -     $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> > > +     $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
> >
> > I think the reason why the pattern to use only the .o files among
> > the prerequisites and then use only the .a files among the same
> > prerequisites (both filters $^) is used here is to make sure that the
> > linker sees object files first before library archives, so that by
> > the time its left-to-right scan sees the first library archive, all
> > the missing symbols in the object files are known.  The above change
> > depends on LIBS being a strict superset of all the library archive
> > files ($GITLIBS in the current code, but that can be updated in the
> > future) listed as prerequisites for the rule, but there is nothing to
> > guarantee that, so it looks brittle.
> >
> > Exact same comment applies to the other two rules touched by this patch.
>
> Hmm, there are other constructs like this that rely on $(LIBS) being a
> superset of the archives, so the three rules changed here align with
> the trend rather than introduce a new trend.
>
> Not saying we shouldn't find a way to handle the overall brittleness.
>
>
> Harald