Makefile checks for DarwinPorts / Fink

Makefile checks for DarwinPorts / Fink

[Stefan Pfetzing]

Hi,

while I was updating the DarwinPorts Portfile for git, I saw some
really suspicious lines in the Makefile of Git for DarwinPorts/Fink.

--- snip ---
        ## fink
        ifeq ($(shell test -d /sw/lib && echo y),y)
                ALL_CFLAGS += -I/sw/include
                ALL_LDFLAGS += -L/sw/lib
        endif
        ## darwinports
        ifeq ($(shell test -d /opt/local/lib && echo y),y)
                ALL_CFLAGS += -I/opt/local/include
                ALL_LDFLAGS += -L/opt/local/lib
        endif
--- snap ---

IMHO, Git should definetely not include /sw/include and /sw/lib, just
if it *exists*.

Think of a situation, when somebody has Fink and DarwinPorts installed
on one machine (possible). Then if you would build Git from
DarwinPorts, the git Makefile would link against Fink libraries! IMHO
the DarwinPorts / Fink build process should set LDFLAGS and CFLAGS
accordingly.

Also, maybe you want to create a DarwinPorts / Fink independent Mac OS
X pkg which contains Git and its deps.

I know this just appends to CFLAGS/LDFLAGS, but if for example
DarwinPorts has broken build-deps, then the Fink stuff would get
sucked in, and you would not notice. (on a box with both, DP and Fink
installed)

bye

Stefan

  --
       http://www.dreamind.de/
Oroborus and Debian GNU/Linux Developer.

Re: Makefile checks for DarwinPorts / Fink

[Junio C Hamano]

"Stefan Pfetzing" <stefan.pfetzing@gmail.com> writes:

> while I was updating the DarwinPorts Portfile for git, I saw some
> really suspicious lines in the Makefile of Git for DarwinPorts/Fink.
>
> --- snip ---
>        ## fink
>        ifeq ($(shell test -d /sw/lib && echo y),y)
>                ALL_CFLAGS += -I/sw/include
>                ALL_LDFLAGS += -L/sw/lib
>        endif
>        ## darwinports
>        ifeq ($(shell test -d /opt/local/lib && echo y),y)
>                ALL_CFLAGS += -I/opt/local/include
>                ALL_LDFLAGS += -L/opt/local/lib
>        endif
> --- snap ---
>
> IMHO, Git should definetely not include /sw/include and /sw/lib, just
> if it *exists*.

Could you make a concrete suggestion (I am not on Darwin)?  

If I am reading you correctly, your suggestion is that
DarwinPorts and/or Fink build procedure, which drive our
Makefile from outside, should set up CFLAGS and LDFLAGS to have
the correct paths for local libraries and headers.  It is not
clear to me if having these defaults there makes it hard (or
cumbersome) to override them in such a setup and you are
proposing to remove them (or commenting them out), or if you can
live with them being there.

These were made as "quick relatively sane defaults for help
people with simple configuration when people build git
themselves" initially and it may be the case that they now could
use improvements.  I dunno (I am not on Darwin).

But I suspect that the "official" portfile (or whatever it is
called in the Darwin world) should be able to override whatever
is done in there --- otherwise we would need to remove them or
comment them out, but I am hoping it does not have to come to
that; I think they serve as good hint to help people who are
building from the source.

Re: Makefile checks for DarwinPorts / Fink

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> "Stefan Pfetzing" <stefan.pfetzing@gmail.com> writes:
> 
> > while I was updating the DarwinPorts Portfile for git, I saw some
> > really suspicious lines in the Makefile of Git for DarwinPorts/Fink.
> >
> > --- snip ---
> >        ## fink
> >        ifeq ($(shell test -d /sw/lib && echo y),y)
> >                ALL_CFLAGS += -I/sw/include
> >                ALL_LDFLAGS += -L/sw/lib
> >        endif
> >        ## darwinports
> >        ifeq ($(shell test -d /opt/local/lib && echo y),y)
> >                ALL_CFLAGS += -I/opt/local/include
> >                ALL_LDFLAGS += -L/opt/local/lib
> >        endif
> > --- snap ---
> >
> > IMHO, Git should definetely not include /sw/include and /sw/lib, just
> > if it *exists*.
[snip]
> But I suspect that the "official" portfile (or whatever it is
> called in the Darwin world) should be able to override whatever
> is done in there --- otherwise we would need to remove them or
> comment them out, but I am hoping it does not have to come to
> that; I think they serve as good hint to help people who are
> building from the source.

The quoted section of the Makefile was mostly my fault.  GIT used
to build only against Fink but when I switched to DarwinPorts it
was first not even looking for them and second when I removed Fink
the Mac OS X linker was warning about /sw/lib not existing.

I completely agree that its incorrect to be doing this all of the
time as a higher-level build driver (e.g. Portfile) should be able
to have more direct control CFLAGS and LDFLAGS.  Perhaps something
like this?

diff --git a/Makefile b/Makefile
index a1666e2..0a48c32 100644
--- a/Makefile
+++ b/Makefile
@@ -268,14 +268,18 @@ ifeq ($(uname_S),Darwin)
 	NEEDS_LIBICONV = YesPlease
 	NO_STRLCPY = YesPlease
 	## fink
-	ifeq ($(shell test -d /sw/lib && echo y),y)
-		BASIC_CFLAGS += -I/sw/include
-		BASIC_LDFLAGS += -L/sw/lib
+	ifndef NO_FINK
+		ifeq ($(shell test -d /sw/lib && echo y),y)
+			BASIC_CFLAGS += -I/sw/include
+			BASIC_LDFLAGS += -L/sw/lib
+		endif
 	endif
+	ifndef NO_DARWIN_PORTS
 	## darwinports
-	ifeq ($(shell test -d /opt/local/lib && echo y),y)
-		BASIC_CFLAGS += -I/opt/local/include
-		BASIC_LDFLAGS += -L/opt/local/lib
+		ifeq ($(shell test -d /opt/local/lib && echo y),y)
+			BASIC_CFLAGS += -I/opt/local/include
+			BASIC_LDFLAGS += -L/opt/local/lib
+		endif
 	endif
 endif
 ifeq ($(uname_S),SunOS)

-- 
Shawn.

Re: Makefile checks for DarwinPorts / Fink

[Shawn Pearce]

This is a slightly cleaner version of my prior patch.  :-)

-->8--
Disable linking with Fink or DarwinPorts.

It may be desirable for the compiler to disable linking against Fink
or DarwinPorts, especially if both are installed on the system and
the user wants GIT to be linked specifically to only one of them.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Makefile |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index a1666e2..99a84cc 100644
--- a/Makefile
+++ b/Makefile
@@ -267,15 +267,17 @@ ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_STRLCPY = YesPlease
-	## fink
-	ifeq ($(shell test -d /sw/lib && echo y),y)
-		BASIC_CFLAGS += -I/sw/include
-		BASIC_LDFLAGS += -L/sw/lib
+	ifndef NO_FINK
+		ifeq ($(shell test -d /sw/lib && echo y),y)
+			BASIC_CFLAGS += -I/sw/include
+			BASIC_LDFLAGS += -L/sw/lib
+		endif
 	endif
-	## darwinports
-	ifeq ($(shell test -d /opt/local/lib && echo y),y)
-		BASIC_CFLAGS += -I/opt/local/include
-		BASIC_LDFLAGS += -L/opt/local/lib
+	ifndef NO_DARWIN_PORTS
+		ifeq ($(shell test -d /opt/local/lib && echo y),y)
+			BASIC_CFLAGS += -I/opt/local/include
+			BASIC_LDFLAGS += -L/opt/local/lib
+		endif
 	endif
 endif
 ifeq ($(uname_S),SunOS)
-- 
1.4.2.rc1.ge711

Re: Makefile checks for DarwinPorts / Fink

[Jakub Narebski]

Shawn Pearce wrote:

> This is a slightly cleaner version of my prior patch.  :-)
> 
> -->8--
> Disable linking with Fink or DarwinPorts.
> 
> It may be desirable for the compiler to disable linking against Fink
> or DarwinPorts, especially if both are installed on the system and
> the user wants GIT to be linked specifically to only one of them.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

Could you _please_ document this option in commentary part in main Makefile?
TIA. 

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: Makefile checks for DarwinPorts / Fink

[Shawn Pearce]

Jakub Narebski <jnareb@gmail.com> wrote:
> Could you _please_ document this option in commentary part in main
> Makefile?

OK.

I left them out originally as it seemed like other platform specific
items weren't in the main commentary part, but since these are
new defines I guess it makes perfect sense that they should be
documented before they get included into the main Makefile.  :-)

Hopefully this is the final version of this patch...

-->8--
Disable linking with Fink or DarwinPorts.

It may be desirable for the compiler to disable linking against Fink
or DarwinPorts, especially if both are installed on the system and
the user wants GIT to be linked specifically to only one of them.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Makefile |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index a1666e2..5432636 100644
--- a/Makefile
+++ b/Makefile
@@ -32,6 +32,18 @@ # Define NO_SVN_TESTS if you want to ski
 # tests.  These tests take up a significant amount of the total test time
 # but are not needed unless you plan to talk to SVN repos.
 #
+# Define NO_FINK if you are building on Darwin/Mac OS X, have Fink
+# installed in /sw, but don't want GIT to link against any libraries
+# installed there.  If defined you may specify your own (or Fink's)
+# include directories and library directories by defining CFLAGS
+# and LDFLAGS appropriately.
+#
+# Define NO_DARWIN_PORTS if you are building on Darwin/Mac OS X,
+# have DarwinPorts installed in /opt/local, but don't want GIT to
+# link against any libraries installed there.  If defined you may
+# specify your own (or DarwinPort's) include directories and
+# library directories by defining CFLAGS and LDFLAGS appropriately.
+#
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
@@ -267,15 +279,17 @@ ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_STRLCPY = YesPlease
-	## fink
-	ifeq ($(shell test -d /sw/lib && echo y),y)
-		BASIC_CFLAGS += -I/sw/include
-		BASIC_LDFLAGS += -L/sw/lib
+	ifndef NO_FINK
+		ifeq ($(shell test -d /sw/lib && echo y),y)
+			BASIC_CFLAGS += -I/sw/include
+			BASIC_LDFLAGS += -L/sw/lib
+		endif
 	endif
-	## darwinports
-	ifeq ($(shell test -d /opt/local/lib && echo y),y)
-		BASIC_CFLAGS += -I/opt/local/include
-		BASIC_LDFLAGS += -L/opt/local/lib
+	ifndef NO_DARWIN_PORTS
+		ifeq ($(shell test -d /opt/local/lib && echo y),y)
+			BASIC_CFLAGS += -I/opt/local/include
+			BASIC_LDFLAGS += -L/opt/local/lib
+		endif
 	endif
 endif
 ifeq ($(uname_S),SunOS)
-- 
1.4.2.rc1.ge711

Re: Makefile checks for DarwinPorts / Fink

[Junio C Hamano]

Thanks for the fix and doc.