{"thread":{"id":"43849","subject":"[PATCH 1/1] do not add common-main to lib","startedAt":"2016-08-15T08:01:29Z","lastAt":"2016-08-15T12:31:29Z","messageCount":5,"participants":["Christian Hesse","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"299321","messageId":"20160815075207.31280-1-list@eworm.de","threadId":"43849","inReplyTo":null,"subject":"[PATCH 1/1] do not add common-main to lib","fromName":"Christian Hesse","fromEmail":"list@eworm.de","sentAt":"2016-08-15T07:52:07Z","receivedAt":"2016-08-15T08:01:29Z","isPatch":true,"sender":{"key":"list@eworm.de","avatar":"https://gravatar.com/avatar/ec9a78d63ae8bf8efdc06867449c0a3e763066c462c2c2f9f103ac4675109e14?d=mp&s=160"},"body":"From: Christian Hesse <mail@eworm.de>\n\nCommit 08aade70 (mingw: declare main()'s argv as const) changed\ndeclaration of main function. This breaks linking external projects\n(e.g. cgit) to libgit.a with:\n\nerror: Multiple definition of `main'\n\nSo do not add common-main to lib and let projects have their own\nmain function.\n\nSigned-off-by: Christian Hesse <mail@eworm.de>\n---\n Makefile | 25 +++++++++++++------------\n 1 file changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex d96ecb7..1aea768 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -953,7 +953,8 @@ BUILTIN_OBJS += builtin/verify-tag.o\n BUILTIN_OBJS += builtin/worktree.o\n BUILTIN_OBJS += builtin/write-tree.o\n \n-GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB)\n+GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\n+GITCOMMON = common-main.o $(GITLIBS)\n EXTLIBS =\n \n GIT_USER_AGENT = git/$(GIT_VERSION)\n@@ -1593,15 +1594,15 @@ TCLTK_PATH_SQ = $(subst ','\\'',$(TCLTK_PATH))\n DIFF_SQ = $(subst ','\\'',$(DIFF))\n PERLLIB_EXTRA_SQ = $(subst ','\\'',$(PERLLIB_EXTRA))\n \n-# We must filter out any object files from $(GITLIBS),\n+# We must filter out any object files from $(GITCOMMON),\n # as it is typically used like:\n #\n-#   foo: foo.o $(GITLIBS)\n+#   foo: foo.o $(GITCOMMON)\n #\t$(CC) $(filter %.o,$^) $(LIBS)\n #\n # where we use it as a dependency. Since we also pull object files\n # from the dependency list, that would make each entry appear twice.\n-LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)\n+LIBS = $(filter-out %.o, $(GITCOMMON)) $(EXTLIBS)\n \n BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \\\n \t$(COMPAT_CFLAGS)\n@@ -1741,7 +1742,7 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n \n-git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n+git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITCOMMON)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n \t\t$(filter %.o,$^) $(LIBS)\n \n@@ -2033,21 +2034,21 @@ compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \\\n compat/nedmalloc/nedmalloc.sp: SPARSE_FLAGS += -Wno-non-pointer-null\n endif\n \n-git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n+git-%$X: %.o GIT-LDFLAGS $(GITCOMMON)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n \n-git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITCOMMON)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(LIBS) $(IMAP_SEND_LDFLAGS)\n \n-git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n+git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITCOMMON)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(CURL_LIBCURL) $(LIBS)\n-git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)\n+git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITCOMMON)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)\n \n-git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)\n+git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITCOMMON) $(VCSSVN_LIB)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \\\n \t$(VCSSVN_LIB)\n \n@@ -2057,7 +2058,7 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n \tln -s $< $@ 2>/dev/null || \\\n \tcp $< $@\n \n-$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS)\n+$(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITCOMMON)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)\n \n@@ -2271,7 +2272,7 @@ t/helper/test-svn-fe$X: $(VCSSVN_LIB)\n \n .PRECIOUS: $(TEST_OBJS)\n \n-t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)\n+t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITCOMMON)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n \n check-sha1:: t/helper/test-sha1$X\n-- \n2.9.3\n\n"},{"id":"299326","messageId":"20160815120223.4lr23aiqmqzjprch@sigill.intra.peff.net","threadId":"43849","inReplyTo":"20160815075207.31280-1-list@eworm.de","subject":"Re: [PATCH 1/1] do not add common-main to lib","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-15T12:02:23Z","receivedAt":"2016-08-15T12:02:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 15, 2016 at 09:52:07AM +0200, Christian Hesse wrote:\n\n> From: Christian Hesse <mail@eworm.de>\n> \n> Commit 08aade70 (mingw: declare main()'s argv as const) changed\n> declaration of main function. This breaks linking external projects\n> (e.g. cgit) to libgit.a with:\n> \n> error: Multiple definition of `main'\n\nI'd expect the culprit is actually 3f2e229 (add an extra level of\nindirection to main(), 2016-07-01).\n\n> So do not add common-main to lib and let projects have their own\n> main function.\n\nThat is certainly an option, but I think it means that those projects\nare potentially buggy in the same way that some git commands were prior\nto the common-main series. Namely, the common main() may do some\nrun-time setup that parts of libgit.a assume has been done.\n\nI would not be surprised if cgit crashes on Windows, for instance, for\nthe reasons detailed in 650c449 (common-main: call\ngit_extract_argv0_path(), 2016-07-01). I would also not be surprised if\nnobody actually builds cgit on Windows. :)\n\nThe \"right\" way to do it (according to the way libgit.a views the world)\nis for cgit's main to become cmd_main(), and let libgit.a do its\nrun-time startup before getting there.\n\n-Peff\n"},{"id":"299329","messageId":"alpine.DEB.2.20.1608151418150.4924@virtualbox","threadId":"43849","inReplyTo":"20160815075207.31280-1-list@eworm.de","subject":"Re: [PATCH 1/1] do not add common-main to lib","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-15T12:20:51Z","receivedAt":"2016-08-15T12:21:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Christian,\n\nOn Mon, 15 Aug 2016, Christian Hesse wrote:\n\n> From: Christian Hesse <mail@eworm.de>\n> \n> Commit 08aade70 (mingw: declare main()'s argv as const) changed\n> declaration of main function. This breaks linking external projects\n> (e.g. cgit) to libgit.a with:\n> \n> error: Multiple definition of `main'\n> \n> So do not add common-main to lib and let projects have their own\n> main function.\n\nI am opposed to this change.\n\nFor one, libgit.a is *not* a library with an API, for a good reason:\nnothing in Git's development guarantees any kind of stable API. For that\nreason, libgit.a is not installed, either, and neither are any headers.\n\nAnd even more importantly: *iff* you *insist* on using libgit.a in your\nproject *despite* having been told not to, it is your responsibility to\nstay up-to-date with the requirements of it.\n\nOne such requirement is that you now implement cmd_main() instead of\nmain().\n\nSo if you want to continue to have an out-of-tree project that links\nagainst the (private) libgit.a, it is your out-of-tree project that needs\nchanging, not libgit.a.\n\nCiao,\nJohannes\n"},{"id":"299332","messageId":"20160815143007.2225c655@leda.localdomain","threadId":"43849","inReplyTo":"alpine.DEB.2.20.1608151418150.4924@virtualbox","subject":"Re: [PATCH 1/1] do not add common-main to lib","fromName":"Christian Hesse","fromEmail":"list@eworm.de","sentAt":"2016-08-15T12:30:07Z","receivedAt":"2016-08-15T12:30:19Z","isPatch":true,"sender":{"key":"list@eworm.de","avatar":"https://gravatar.com/avatar/ec9a78d63ae8bf8efdc06867449c0a3e763066c462c2c2f9f103ac4675109e14?d=mp&s=160"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> on Mon, 2016/08/15 14:20:\n> Hi Christian,\n> \n> On Mon, 15 Aug 2016, Christian Hesse wrote:\n> \n> > From: Christian Hesse <mail@eworm.de>\n> > \n> > Commit 08aade70 (mingw: declare main()'s argv as const) changed\n> > declaration of main function. This breaks linking external projects\n> > (e.g. cgit) to libgit.a with:\n> > \n> > error: Multiple definition of `main'\n> > \n> > So do not add common-main to lib and let projects have their own\n> > main function.  \n> \n> I am opposed to this change.\n\nMe too. :-p\n\n> For one, libgit.a is *not* a library with an API, for a good reason:\n> nothing in Git's development guarantees any kind of stable API. For that\n> reason, libgit.a is not installed, either, and neither are any headers.\n> \n> And even more importantly: *iff* you *insist* on using libgit.a in your\n> project *despite* having been told not to, it is your responsibility to\n> stay up-to-date with the requirements of it.\n\ncgit pulls in the git tree as a subproject. We are aware that the API changes\nall the time and that's fine. Usually we just fix it, this time I missed the\nbackground information of the change.\n\n> One such requirement is that you now implement cmd_main() instead of\n> main().\n> \n> So if you want to continue to have an out-of-tree project that links\n> against the (private) libgit.a, it is your out-of-tree project that needs\n> changing, not libgit.a.\n\nAlready updated my code. ;)\nThanks!\n-- \nmain(a){char*c=/*    Schoene Gruesse                         */\"B?IJj;MEH\"\n\"CX:;\",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)\nputchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}\n"},{"id":"299333","messageId":"20160815142125.0ca30e0f@leda.localdomain","threadId":"43849","inReplyTo":"20160815120223.4lr23aiqmqzjprch@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] do not add common-main to lib","fromName":"Christian Hesse","fromEmail":"list@eworm.de","sentAt":"2016-08-15T12:21:25Z","receivedAt":"2016-08-15T12:31:29Z","isPatch":true,"sender":{"key":"list@eworm.de","avatar":"https://gravatar.com/avatar/ec9a78d63ae8bf8efdc06867449c0a3e763066c462c2c2f9f103ac4675109e14?d=mp&s=160"},"body":"Jeff King <peff@peff.net> on Mon, 2016/08/15 08:02:\n> On Mon, Aug 15, 2016 at 09:52:07AM +0200, Christian Hesse wrote:\n> \n> > From: Christian Hesse <mail@eworm.de>\n> > \n> > Commit 08aade70 (mingw: declare main()'s argv as const) changed\n> > declaration of main function. This breaks linking external projects\n> > (e.g. cgit) to libgit.a with:\n> > \n> > error: Multiple definition of `main'  \n> \n> I'd expect the culprit is actually 3f2e229 (add an extra level of\n> indirection to main(), 2016-07-01).\n\nAh, probably you are right...\n\n> > So do not add common-main to lib and let projects have their own\n> > main function.  \n> \n> That is certainly an option, but I think it means that those projects\n> are potentially buggy in the same way that some git commands were prior\n> to the common-main series. Namely, the common main() may do some\n> run-time setup that parts of libgit.a assume has been done.\n\nOk, got it.\n\n> I would not be surprised if cgit crashes on Windows, for instance, for\n> the reasons detailed in 650c449 (common-main: call\n> git_extract_argv0_path(), 2016-07-01). I would also not be surprised if\n> nobody actually builds cgit on Windows. :)\n\nI never tried and probably nobody else did. :-p\n\n> The \"right\" way to do it (according to the way libgit.a views the world)\n> is for cgit's main to become cmd_main(), and let libgit.a do its\n> run-time startup before getting there.\n\nLooks like that does the job. I will give it some more testing.\n\nPlease ignore my patch... ;)\nThanks a lot!\n-- \nmain(a){char*c=/*    Schoene Gruesse                         */\"B?IJj;MEH\"\n\"CX:;\",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)\nputchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}\n"}]}