{"thread":{"id":"17357","subject":"[PATCH] Makefile: Make 'configure --with-expat=path' actually work","startedAt":"2009-01-25T19:24:41Z","lastAt":"2009-01-28T21:30:24Z","messageCount":4,"participants":["Serge van den Boom","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"101860","messageId":"alpine.BSF.2.00.0901251938120.97940@toad.stack.nl","threadId":"17357","inReplyTo":null,"subject":"[PATCH] Makefile: Make 'configure --with-expat=path' actually work","fromName":"Serge van den Boom","fromEmail":"svdb@stack.nl","sentAt":"2009-01-25T19:24:41Z","receivedAt":"2009-01-25T19:24:41Z","isPatch":true,"sender":{"key":"svdb@stack.nl","avatar":null},"body":"The prefix specified with the --with-expat option of configure was not\nactually used.\n\nSigned-off-by: Serge van den Boom <svdb@stack.nl>\n---\ndiff --git a/Makefile b/Makefile\nindex b4d9cb4..e7218cb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -849,7 +849,12 @@ else\n  \t\tendif\n  \tendif\n  \tifndef NO_EXPAT\n-\t\tEXPAT_LIBEXPAT = -lexpat\n+\t\tifdef EXPATDIR\n+\t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n+\t\t\tEXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat\n+\t\telse\n+\t\t\tEXPAT_LIBEXPAT = -lexpat\n+\t\tendif\n  \tendif\n  endif\n"},{"id":"102339","messageId":"7vocxrqm57.fsf@gitster.siamese.dyndns.org","threadId":"17357","inReplyTo":"alpine.BSF.2.00.0901251938120.97940@toad.stack.nl","subject":"Re: [PATCH] Makefile: Make 'configure --with-expat=path' actually work","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-28T19:13:40Z","receivedAt":"2009-01-28T19:13:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Serge van den Boom <svdb@stack.nl> writes:\n\n> The prefix specified with the --with-expat option of configure was not\n> actually used.\n\nI see configure.ac already has support for autodetection but I realized it\nonly after running \"git grep EXPATDIR\".  \"Even though the configure script\nknows how to autodetect presence of the expat library and set EXPATDIR to\nthe prefix of the location it was found, the Makefile ignored it and only\nhonoured NO_EXPAT\" might have been a better way to describe the breakage\nthe patch fixes.\n\nIf you look at the Makefile, you will notice a sequence of comments like\nthis:\n\n    # Define NO_CURL if you do not have libcurl installed.  git-http-pull and\n    # git-http-push are not built, and you cannot use http:// and https://\n    # transports.\n    #\n    # Define CURLDIR=/foo/bar if your curl header and library files are in\n    # /foo/bar/include and /foo/bar/lib directories.\n    #\n\nPlease add one for EXPATDIR, just after \"Define NO_EXPAT if ...\".  People\nwho do not run ./configure but add their own customizations in config.mak\nshould benefit from your patch as well.\n\nYou might want to add a logic to drop NO_EXPAT when EXPATDIR is specified\nto the Makefile as well, but I didn't check.\n\nPlease do *not* send a patch in 'text/plain; format=\"flowed\"' content-type.\nYou will get a whitespace mangled patch that I have to fix up by hand.\n\nThanks.\n\n> Signed-off-by: Serge van den Boom <svdb@stack.nl>\n> ---\n> diff --git a/Makefile b/Makefile\n> index b4d9cb4..e7218cb 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -849,7 +849,12 @@ else\n>  \t\tendif\n>  \tendif\n>  \tifndef NO_EXPAT\n> -\t\tEXPAT_LIBEXPAT = -lexpat\n> +\t\tifdef EXPATDIR\n> +\t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n> +\t\t\tEXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat\n> +\t\telse\n> +\t\t\tEXPAT_LIBEXPAT = -lexpat\n> +\t\tendif\n>  \tendif\n>  endif\n"},{"id":"102347","messageId":"alpine.BSF.2.00.0901282120240.74552@toad.stack.nl","threadId":"17357","inReplyTo":"7vocxrqm57.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2] Makefile: Make 'configure --with-expat=path' actually work","fromName":"Serge van den Boom","fromEmail":"svdb@stack.nl","sentAt":"2009-01-28T20:43:57Z","receivedAt":"2009-01-28T20:43:57Z","isPatch":true,"sender":{"key":"svdb@stack.nl","avatar":null},"body":"While the configure script sets the EXPATDIR environment variable to\nwhatever value was passed to its option --with-expat as the prefix of\nthe location of the expat library and headers, the Makefile ignored it.\nThis patch fixes this bug.\n\nSigned-off-by: Serge van den Boom <svdb@stack.nl>\n---\n\nOn Wed, 28 Jan 2009, Junio C Hamano wrote:\n> Serge van den Boom <svdb@stack.nl> writes:\n> > The prefix specified with the --with-expat option of configure was not\n> > actually used.\n> \n> I see configure.ac already has support for autodetection but I realized it\n> only after running \"git grep EXPATDIR\".  \"Even though the configure script\n> knows how to autodetect presence of the expat library and set EXPATDIR to\n> the prefix of the location it was found, the Makefile ignored it and only\n> honoured NO_EXPAT\" might have been a better way to describe the breakage\n> the patch fixes.\n\nThat's not entirely right, unless I'm missing something. The configure script\ndoes not try to detect expat itself, though it passes on the argument\nto --with-expat to the Makefile, via the EXPATDIR environment variable.\n\n> If you look at the Makefile, you will notice a sequence of comments like\n> this:\n> \n>     # Define NO_CURL if you do not have libcurl installed.  git-http-pull and\n>     # git-http-push are not built, and you cannot use http:// and https://\n>     # transports.\n>     #\n>     # Define CURLDIR=/foo/bar if your curl header and library files are in\n>     # /foo/bar/include and /foo/bar/lib directories.\n>     #\n> \n> Please add one for EXPATDIR, just after \"Define NO_EXPAT if ...\".  People\n> who do not run ./configure but add their own customizations in config.mak\n> should benefit from your patch as well.\n\nOk.\n\ndiff --git a/Makefile b/Makefile\nindex 9d451cf..a7310f2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -23,6 +23,9 @@ all::\n # Define NO_EXPAT if you do not have expat installed.  git-http-push is\n # not built, and you cannot push using http:// and https:// transports.\n #\n+# Define EXPATDIR=/foo/bar if your expat header and library files are in\n+# /foo/bar/include and /foo/bar/lib directories.\n+#\n # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.\n #\n # Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks\n@@ -850,7 +853,12 @@ else\n \t\tendif\n \tendif\n \tifndef NO_EXPAT\n-\t\tEXPAT_LIBEXPAT = -lexpat\n+\t\tifdef EXPATDIR\n+\t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n+\t\t\tEXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat\n+\t\telse\n+\t\t\tEXPAT_LIBEXPAT = -lexpat\n+\t\tendif\n \tendif\n endif\n \n"},{"id":"102352","messageId":"7v63jzm83z.fsf@gitster.siamese.dyndns.org","threadId":"17357","inReplyTo":"alpine.BSF.2.00.0901282120240.74552@toad.stack.nl","subject":"Re: [PATCH v2] Makefile: Make 'configure --with-expat=path' actually work","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-28T21:30:24Z","receivedAt":"2009-01-28T21:30:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"}]}