threads / patch / 17357

patchMakefile: Make 'configure --with-expat=path' actually work

Subject: [PATCH] Makefile: Make 'configure --with-expat=path' actually work

## tl;dr

4 messages between Jan 25, 2009 and Jan 28, 2009. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Serge van den Boom· Jan 25, 2009, 19:24 UTC · lore

The prefix specified with the --with-expat option of configure was not actually used.

Signed-off-by: Serge van den Boom <svdb@stack.nl>
---
Show changes to Makefile +6 −1
diff --git a/Makefile b/Makefile
index b4d9cb4..e7218cb 100644
--- a/Makefile
+++ b/Makefile
@@ -849,7 +849,12 @@ else
  		endif
  	endif
  	ifndef NO_EXPAT
-		EXPAT_LIBEXPAT = -lexpat
+		ifdef EXPATDIR
+			BASIC_CFLAGS += -I$(EXPATDIR)/include
+			EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat
+		else
+			EXPAT_LIBEXPAT = -lexpat
+		endif
  	endif
  endif
Junio C Hamano· Jan 28, 2009, 19:13 UTC · re: Serge van den Boom · lore

Re: [PATCH] Makefile: Make 'configure --with-expat=path' actually work

Serge van den Boom <svdb@stack.nl> writes:
> The prefix specified with the --with-expat option of configure was not
> actually used.

I see configure.ac already has support for autodetection but I realized it only after running "git grep EXPATDIR". "Even though the configure script knows how to autodetect presence of the expat library and set EXPATDIR to the prefix of the location it was found, the Makefile ignored it and only honoured NO_EXPAT" might have been a better way to describe the breakage the patch fixes.

If you look at the Makefile, you will notice a sequence of comments like this:

    # Define NO_CURL if you do not have libcurl installed.  git-http-pull and
    # git-http-push are not built, and you cannot use http:// and https://
    # transports.
    #
    # Define CURLDIR=/foo/bar if your curl header and library files are in
    # /foo/bar/include and /foo/bar/lib directories.
    #

Please add one for EXPATDIR, just after "Define NO_EXPAT if ...". People who do not run ./configure but add their own customizations in config.mak should benefit from your patch as well.

You might want to add a logic to drop NO_EXPAT when EXPATDIR is specified to the Makefile as well, but I didn't check.

Please do *not* send a patch in 'text/plain; format="flowed"' content-type. You will get a whitespace mangled patch that I have to fix up by hand.

Thanks.
Show 19 quoted lines
> Signed-off-by: Serge van den Boom <svdb@stack.nl>
> ---
> diff --git a/Makefile b/Makefile
> index b4d9cb4..e7218cb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -849,7 +849,12 @@ else
>  		endif
>  	endif
>  	ifndef NO_EXPAT
> -		EXPAT_LIBEXPAT = -lexpat
> +		ifdef EXPATDIR
> +			BASIC_CFLAGS += -I$(EXPATDIR)/include
> +			EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat
> +		else
> +			EXPAT_LIBEXPAT = -lexpat
> +		endif
>  	endif
>  endif
Serge van den Boom· Jan 28, 2009, 20:43 UTC · re: Junio C Hamano · lore

[PATCH v2] Makefile: Make 'configure --with-expat=path' actually work

While the configure script sets the EXPATDIR environment variable to whatever value was passed to its option --with-expat as the prefix of the location of the expat library and headers, the Makefile ignored it. This patch fixes this bug.

Signed-off-by: Serge van den Boom <svdb@stack.nl>
---
On Wed, 28 Jan 2009, Junio C Hamano wrote:
Show 10 quoted lines
> Serge van den Boom <svdb@stack.nl> writes:
> > The prefix specified with the --with-expat option of configure was not
> > actually used.
> 
> I see configure.ac already has support for autodetection but I realized it
> only after running "git grep EXPATDIR".  "Even though the configure script
> knows how to autodetect presence of the expat library and set EXPATDIR to
> the prefix of the location it was found, the Makefile ignored it and only
> honoured NO_EXPAT" might have been a better way to describe the breakage
> the patch fixes.

That's not entirely right, unless I'm missing something. The configure script does not try to detect expat itself, though it passes on the argument to --with-expat to the Makefile, via the EXPATDIR environment variable.

Show 14 quoted lines
> If you look at the Makefile, you will notice a sequence of comments like
> this:
> 
>     # Define NO_CURL if you do not have libcurl installed.  git-http-pull and
>     # git-http-push are not built, and you cannot use http:// and https://
>     # transports.
>     #
>     # Define CURLDIR=/foo/bar if your curl header and library files are in
>     # /foo/bar/include and /foo/bar/lib directories.
>     #
> 
> Please add one for EXPATDIR, just after "Define NO_EXPAT if ...".  People
> who do not run ./configure but add their own customizations in config.mak
> should benefit from your patch as well.
Ok.
Show changes to Makefile +9 −1
diff --git a/Makefile b/Makefile
index 9d451cf..a7310f2 100644
--- a/Makefile
+++ b/Makefile
@@ -23,6 +23,9 @@ all::
 # Define NO_EXPAT if you do not have expat installed.  git-http-push is
 # not built, and you cannot push using http:// and https:// transports.
 #
+# Define EXPATDIR=/foo/bar if your expat header and library files are in
+# /foo/bar/include and /foo/bar/lib directories.
+#
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 #
 # Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks
@@ -850,7 +853,12 @@ else
 		endif
 	endif
 	ifndef NO_EXPAT
-		EXPAT_LIBEXPAT = -lexpat
+		ifdef EXPATDIR
+			BASIC_CFLAGS += -I$(EXPATDIR)/include
+			EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat
+		else
+			EXPAT_LIBEXPAT = -lexpat
+		endif
 	endif
 endif

← back to recent threads