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

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

[Serge van den Boom]

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

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

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

[Junio C Hamano]

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.

> 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

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

[Serge van den Boom]

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:
> 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.

> 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.

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
 

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

[Junio C Hamano]

Thanks.