* [PATCH] Makefile: drop duplicate %.a from link recipes
@ 2026-05-31 23:16 Harald Nordgren via GitGitGadget
2026-06-04 0:33 ` Junio C Hamano
2026-06-04 22:03 ` [PATCH v2] Makefile: dedup archives in $(LIBS) so link recipes don't repeat them Harald Nordgren via GitGitGadget
0 siblings, 2 replies; 5+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-05-31 23:16 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
Three link recipes list archive files twice on the link line: once
via $(filter %.a,$^) and again through $(LIBS), which expands to
$(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the linker warns
about the duplicates:
ld: warning: ignoring duplicate libraries: 'libgit.a', 'target/release/libgitcore.a'
Drop the redundant filter from the test-helper, fuzz-program, and
unit-test recipes so they match the pattern used by other link
recipes in the file.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
Makefile: drop duplicate %.a from test-helper link rule
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2314%2FHaraldNordgren%2Fmakefile-test-helper-dedup-libs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2314/HaraldNordgren/makefile-test-helper-dedup-libs-v1
Pull-Request: https://github.com/git/git/pull/2314
Makefile | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index b31ecb0756..309d1d1e74 100644
--- a/Makefile
+++ b/Makefile
@@ -3392,7 +3392,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 +4015,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)'; \
base-commit: 1666c1265231b0bc5f613fbbf3f0a9896cdef76e
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Makefile: drop duplicate %.a from link recipes
2026-05-31 23:16 [PATCH] Makefile: drop duplicate %.a from link recipes Harald Nordgren via GitGitGadget
@ 2026-06-04 0:33 ` Junio C Hamano
2026-06-04 7:06 ` Harald Nordgren
2026-06-04 22:03 ` [PATCH v2] Makefile: dedup archives in $(LIBS) so link recipes don't repeat them Harald Nordgren via GitGitGadget
1 sibling, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2026-06-04 0:33 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
"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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Makefile: drop duplicate %.a from link recipes
2026-06-04 0:33 ` Junio C Hamano
@ 2026-06-04 7:06 ` Harald Nordgren
2026-06-04 7:15 ` Harald Nordgren
0 siblings, 1 reply; 5+ messages in thread
From: Harald Nordgren @ 2026-06-04 7:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Makefile: drop duplicate %.a from link recipes
2026-06-04 7:06 ` Harald Nordgren
@ 2026-06-04 7:15 ` Harald Nordgren
0 siblings, 0 replies; 5+ messages in thread
From: Harald Nordgren @ 2026-06-04 7:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] Makefile: dedup archives in $(LIBS) so link recipes don't repeat them
2026-05-31 23:16 [PATCH] Makefile: drop duplicate %.a from link recipes Harald Nordgren via GitGitGadget
2026-06-04 0:33 ` Junio C Hamano
@ 2026-06-04 22:03 ` Harald Nordgren via GitGitGadget
1 sibling, 0 replies; 5+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-06-04 22:03 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
A handful of link recipes listed archive files twice: once explicitly
via $(filter %.a,$^) and again implicitly through $(LIBS), which
expanded to $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the
linker warned about the duplicates:
ld: warning: ignoring duplicate libraries: 'libgit.a', 'target/release/libgitcore.a'
Redefine $(LIBS) to list archive prerequisites from $^ first, then
the rest of the library list with those archives filtered out so each
appears only once.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
Makefile: drop duplicate %.a from test-helper link rule
Redefine $(LIBS) to list archive prerequisites from $^ first, then the
rest of the library list to avoid brittleness in the future.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2314%2FHaraldNordgren%2Fmakefile-test-helper-dedup-libs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2314/HaraldNordgren/makefile-test-helper-dedup-libs-v2
Pull-Request: https://github.com/git/git/pull/2314
Range-diff vs v1:
1: f6166450b0 ! 1: 0ef442ea05 Makefile: drop duplicate %.a from link recipes
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail•com>
## Commit message ##
- Makefile: drop duplicate %.a from link recipes
+ Makefile: dedup archives in $(LIBS) so link recipes don't repeat them
- Three link recipes list archive files twice on the link line: once
- via $(filter %.a,$^) and again through $(LIBS), which expands to
- $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the linker warns
- about the duplicates:
+ A handful of link recipes listed archive files twice: once explicitly
+ via $(filter %.a,$^) and again implicitly through $(LIBS), which
+ expanded to $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the
+ linker warned about the duplicates:
ld: warning: ignoring duplicate libraries: 'libgit.a', 'target/release/libgitcore.a'
- Drop the redundant filter from the test-helper, fuzz-program, and
- unit-test recipes so they match the pattern used by other link
- recipes in the file.
+ Redefine $(LIBS) to list archive prerequisites from $^ first, then
+ the rest of the library list with those archives filtered out so each
+ appears only once.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
## Makefile ##
+@@ Makefile: endif
+ #
+ # where we use it as a dependency. Since we also pull object files
+ # from the dependency list, that would make each entry appear twice.
+-LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
++# Archives from $^ come first, then the rest with those archives
++# filtered out so each appears only once.
++LIBS = $(filter %.a,$^) $(filter-out $(filter %.a,$^),$(filter-out %.o,$(GITLIBS)) $(EXTLIBS))
+
+ BASIC_CFLAGS += $(COMPAT_CFLAGS)
+ LIB_OBJS += $(COMPAT_OBJS)
@@ Makefile: perf: all
t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o
Makefile | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index b31ecb0756..a828a66f28 100644
--- a/Makefile
+++ b/Makefile
@@ -2503,7 +2503,9 @@ endif
#
# where we use it as a dependency. Since we also pull object files
# from the dependency list, that would make each entry appear twice.
-LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
+# Archives from $^ come first, then the rest with those archives
+# filtered out so each appears only once.
+LIBS = $(filter %.a,$^) $(filter-out $(filter %.a,$^),$(filter-out %.o,$(GITLIBS)) $(EXTLIBS))
BASIC_CFLAGS += $(COMPAT_CFLAGS)
LIB_OBJS += $(COMPAT_OBJS)
@@ -3392,7 +3394,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 +4017,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)'; \
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-04 22:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-31 23:16 [PATCH] Makefile: drop duplicate %.a from link recipes Harald Nordgren via GitGitGadget
2026-06-04 0:33 ` Junio C Hamano
2026-06-04 7:06 ` Harald Nordgren
2026-06-04 7:15 ` Harald Nordgren
2026-06-04 22:03 ` [PATCH v2] Makefile: dedup archives in $(LIBS) so link recipes don't repeat them Harald Nordgren via GitGitGadget
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox