On Mon, Jan 13, 2025 at 09:54:07AM -0800, Josh Steadmon wrote:
> As Junio says, a short term fix would be to build with
> LINK_FUZZ_PROGRAMS="". A better solution would be to make
> config.mak.uname smarter about whether to enable this by default. I see
> that we use "detect-compiler" in config.mak.dev, would it make sense to
> check this in config.mak.uname as well?
It feels like the original sin here is defining main() in our library in
the first place, if there are programs that may not want it. But we
don't actually put it into libgit.a; the object file is just mentioned
in the $(GITLIBS) variable of the Makefile.
We could split that out like this:
diff --git a/Makefile b/Makefile
index 97e8385b66..f098ca5a5c 100644
--- a/Makefile
+++ b/Makefile
@@ -1371,7 +1371,8 @@ UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o
UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o
# xdiff and reftable libs may in turn depend on what is in libgit.a
-GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)
+GITLIBS = $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)
+PROGRAM_GITLIBS = common-main.o $(GITLIBS)
EXTLIBS =
GIT_USER_AGENT = git/$(GIT_VERSION)
and then depend on $(PROGRAM_GITLIBS) as appropriate. Or if oss-fuzz is
the only special case here, then perhaps we could just teach it to
suppress the extra main:
diff --git a/Makefile b/Makefile
index 97e8385b66..06431170de 100644
--- a/Makefile
+++ b/Makefile
@@ -3852,9 +3852,8 @@ FUZZ_CXXFLAGS ?= $(ALL_CFLAGS)
.PHONY: fuzz-all
fuzz-all: $(FUZZ_PROGRAMS)
-$(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS
+$(FUZZ_PROGRAMS): %: %.o $(filter-out common-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)
$(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_OBJS) \
You'd need two fixes to go with that:
1. common-main was originally supposed to just be about overriding
main(). But it has since grown common_exit(), which does not have
the same linking properties (we override exit() calls at the macro
layer instead). That should be defined in libgit.a somewhere.
2. When building the test-compile fuzzers, now you'll need to
provide an actual main() function. I guess we can determine that by
the presence of LIB_FUZZING_ENGINE? So maybe:
diff --git a/Makefile b/Makefile
index 97e8385b66..ba3faf9b9e 100644
--- a/Makefile
+++ b/Makefile
@@ -3852,9 +3852,15 @@ FUZZ_CXXFLAGS ?= $(ALL_CFLAGS)
.PHONY: fuzz-all
fuzz-all: $(FUZZ_PROGRAMS)
-$(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS
+ifdef LIB_FUZZING_ENGINE
+# assume the fuzzing engine supplies main()
+FUZZ_GITLIBS = $(filter-out common-main.o, $(GITLIBS))
+else
+FUZZ_GITLIBS = oss-fuzz/dummy-cmd-main.o $(GITLIBS)
+endif
+
+$(FUZZ_PROGRAMS): %: %.o $(FUZZ_GITLIBS) GIT-LDFLAGS
$(QUIET_LINK)$(FUZZ_CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \
- -Wl,--allow-multiple-definition \
$(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)
$(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_OBJS) \
-Peff
