[PATCH 1/5] General configuration portability (Resend)

[PATCH 1/5] General configuration portability (Resend)

[Gary V. Vaughan]

Resending some patches that didn't make it, in the correct format
and taking into account the feedback I already had on one of the
later patches...

This patch is in part to deal with peculiarities of our build
environment (software packages are installed in their own directory
tree, for example, and oftentimes we need to set additional -I options
in CPPFLAGS, or -L options in LDFLAGS, among others).

However this is also where we put configuration changes to add
additional tests, or fix Makefile problems so that everything builds
on all of our supported architectures.  Probably, much of this patch
is useful upstream.
---
 Makefile             |   50 +++++++++++++++++++++++++++++++++++++++---------
 aclocal.m4           |   41 ++++++++++++++++++++++++++++++++++++++++
 config.mak.in        |   10 ++++++++-
 configure.ac         |   51 ++++++++++++++++++++++++++++++++++++++++++-------
 git-compat-util.h    |    8 +++++-
 perl/Makefile        |    3 +-
 perl/Makefile.PL     |    2 +-
 t/t7610-mergetool.sh |   25 +++++++++++++----------
 8 files changed, 156 insertions(+), 34 deletions(-)
 create mode 100644 aclocal.m4

diff --git a/Makefile b/Makefile
index 8fdc421..6058805 100644
--- a/Makefile
+++ b/Makefile
@@ -243,7 +243,7 @@ endif
 
 CFLAGS = -g -O2 -Wall
 LDFLAGS =
-ALL_CFLAGS = $(CFLAGS)
+ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 STRIP ?= strip
 
@@ -264,7 +264,7 @@ bindir_relative = bin
 bindir = $(prefix)/$(bindir_relative)
 mandir = share/man
 infodir = share/info
-gitexecdir = libexec/git-core
+gitexecdir = libexec
 sharedir = $(prefix)/share
 template_dir = share/git-core/templates
 htmldir = share/doc/git-doc
@@ -294,6 +294,7 @@ RPMBUILD = rpmbuild
 TCL_PATH = tclsh
 TCLTK_PATH = wish
 PTHREAD_LIBS = -lpthread
+PTHREAD_CFLAGS =
 
 export TCL_PATH TCLTK_PATH
 
@@ -725,6 +726,12 @@ EXTLIBS =
 # because maintaining the nesting to match is a pain.  If
 # we had "elif" things would have been much nicer...
 
+ifeq ($(uname_S),OSF1)
+        # Need this for u_short definitions et al
+        BASIC_CFLAGS += -D_OSF_SOURCE
+        NO_STRTOULL = YesPlease
+	NO_NSEC = YesPlease
+endif
 ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
 	NO_MKSTEMPS = YesPlease
@@ -797,8 +804,10 @@ ifeq ($(uname_S),SunOS)
 	NO_MKDTEMP = YesPlease
 	NO_MKSTEMPS = YesPlease
 	NO_REGEX = YesPlease
+	ifeq ($(uname_R),5.6)
+		NO_HSTRERROR = YesPlease
+	endif
 	ifeq ($(uname_R),5.7)
-		NEEDS_RESOLV = YesPlease
 		NO_IPV6 = YesPlease
 		NO_SOCKADDR_STORAGE = YesPlease
 		NO_UNSETENV = YesPlease
@@ -888,6 +897,8 @@ ifeq ($(uname_S),AIX)
 	BASIC_CFLAGS += -D_LARGE_FILES
 	ifeq ($(shell expr "$(uname_V)" : '[1234]'),1)
 		NO_PTHREADS = YesPlease
+	else
+		PTHREAD_LIBS = -lpthread
 	endif
 endif
 ifeq ($(uname_S),GNU)
@@ -941,8 +952,14 @@ ifeq ($(uname_S),HP-UX)
 	NO_MKDTEMP = YesPlease
 	NO_UNSETENV = YesPlease
 	NO_HSTRERROR = YesPlease
+	NO_INET_NTOP = YesPlease
+	NO_INET_PTON = YesPlease
+	ifeq ($(uname_R),B.10.20)
+		NO_PREAD = YesPlease
+	endif
 	NO_SYS_SELECT_H = YesPlease
 	SNPRINTF_RETURNS_BOGUS = YesPlease
+	NO_NSEC = YesPlease
 endif
 ifeq ($(uname_S),Windows)
 	GIT_VERSION := $(GIT_VERSION).MSVC
@@ -1060,6 +1077,12 @@ ifdef COMPUTE_HEADER_DEPENDENCIES
 USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
 endif
 
+ifeq ($(uname_S),HP-UX)
+	NEEDS_RESOLV =
+	NEEDS_LIBGEN =
+endif
+
+
 ifdef SANE_TOOL_PATH
 SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
 BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
@@ -1068,6 +1091,14 @@ else
 BROKEN_PATH_FIX = '/^\# @@BROKEN_PATH_FIX@@$$/d'
 endif
 
+ifneq (inline,$(INLINE))
+	BASIC_CFLAGS += -Dinline=$(INLINE)
+endif
+
+ifneq (socklen_t,$(SOCKLEN_T))
+	BASIC_CFLAGS += -Dsocklen_t=$(SOCKLEN_T)
+endif
+
 ifeq ($(uname_S),Darwin)
 	ifndef NO_FINK
 		ifeq ($(shell test -d /sw/lib && echo y),y)
@@ -1085,13 +1116,9 @@ ifeq ($(uname_S),Darwin)
 endif
 
 ifndef CC_LD_DYNPATH
-	ifdef NO_R_TO_GCC_LINKER
 		# Some gcc does not accept and pass -R to the linker to specify
 		# the runtime dynamic library path.
 		CC_LD_DYNPATH = -Wl,-rpath,
-	else
-		CC_LD_DYNPATH = -R
-	endif
 endif
 
 ifdef NO_LIBGEN_H
@@ -1339,6 +1366,7 @@ endif
 ifdef NO_PTHREADS
 	BASIC_CFLAGS += -DNO_PTHREADS
 else
+	BASIC_CFLAGS += $(PTHREAD_CFLAGS)
 	EXTLIBS += $(PTHREAD_LIBS)
 	LIB_OBJS += thread-utils.o
 endif
@@ -1527,11 +1555,10 @@ ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
 perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL
-	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
+	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' libdir='$(libdir)' $(@F)
 
 $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
 	$(QUIET_GEN)$(RM) $@ $@+ && \
-	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
 	sed -e '1{' \
 	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
 	    -e '	h' \
@@ -1539,7 +1566,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
 	    -e '	H' \
 	    -e '	x' \
 	    -e '}' \
-	    -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \
+	    -e 's|@@INSTLIBDIR@@|'"$(libdir)/perl"'|g' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    $@.perl >$@+ && \
 	chmod +x $@+ && \
@@ -1985,6 +2012,9 @@ endif
 install-doc:
 	$(MAKE) -C Documentation install
 
+install-html:
+	$(MAKE) -C Documentation install-html
+
 install-man:
 	$(MAKE) -C Documentation install-man
 
diff --git a/aclocal.m4 b/aclocal.m4
new file mode 100644
index 0000000..e1fdfe3
--- /dev/null
+++ b/aclocal.m4
@@ -0,0 +1,41 @@
+dnl Check for socklen_t: historically on BSD it is an int, and in
+dnl POSIX 1g it is a type of its own, but some platforms use different
+dnl types for the argument to getsockopt, getpeername, etc.  So we
+dnl have to test to find something that will work.
+AC_DEFUN([TYPE_SOCKLEN_T],
+[
+   AC_CHECK_TYPE([socklen_t], ,[
+      AC_MSG_CHECKING([for socklen_t equivalent])
+      AC_CACHE_VAL([git_cv_socklen_t_equiv],
+      [
+         # Systems have either "struct sockaddr *" or
+         # "void *" as the second argument to getpeername
+         git_cv_socklen_t_equiv=
+         for arg2 in "struct sockaddr" void; do
+            for t in int size_t unsigned long "unsigned long"; do
+               AC_TRY_COMPILE([
+                  #include <sys/types.h>
+                  #include <sys/socket.h>
+
+                  int getpeername (int, $arg2 *, $t *);
+               ],[
+                  $t len;
+                  getpeername(0,0,&len);
+               ],[
+                  git_cv_socklen_t_equiv="$t"
+                  break 2
+               ])
+            done
+         done
+
+         if test "x$git_cv_socklen_t_equiv" = x; then
+            AC_MSG_ERROR([Cannot find a type to use in place of socklen_t])
+         fi
+      ])
+      AC_MSG_RESULT($git_cv_socklen_t_equiv)
+      AC_DEFINE_UNQUOTED(socklen_t, $git_cv_socklen_t_equiv,
+			[type to use in place of socklen_t if not defined])],
+      [#include <sys/types.h>
+#include <sys/socket.h>])
+])
+
diff --git a/config.mak.in b/config.mak.in
index 6008ac9..a6d20b8 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -3,6 +3,7 @@
 
 CC = @CC@
 CFLAGS = @CFLAGS@
+CPPFLAGS = @CPPFLAGS@
 LDFLAGS = @LDFLAGS@
 CC_LD_DYNPATH = @CC_LD_DYNPATH@
 AR = @AR@
@@ -13,9 +14,10 @@ TCLTK_PATH = @TCLTK_PATH@
 prefix = @prefix@
 exec_prefix = @exec_prefix@
 bindir = @bindir@
-gitexecdir = @libexecdir@/git-core
+gitexecdir = @libexecdir@
 datarootdir = @datarootdir@
 template_dir = @datadir@/git-core/templates
+libdir = @libdir@
 
 mandir=@mandir@
 
@@ -50,10 +52,16 @@ NO_SETENV=@NO_SETENV@
 NO_UNSETENV=@NO_UNSETENV@
 NO_MKDTEMP=@NO_MKDTEMP@
 NO_MKSTEMPS=@NO_MKSTEMPS@
+NO_INET_NTOP=@NO_INET_NTOP@
+NO_INET_PTON=@NO_INET_PTON@
 NO_ICONV=@NO_ICONV@
 OLD_ICONV=@OLD_ICONV@
 NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
+INLINE=@INLINE@
+SOCKLEN_T=@SOCKLEN_T@
 FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@
 SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@
 NO_PTHREADS=@NO_PTHREADS@
+PTHREAD_CFLAGS=@PTHREAD_CFLAGS@
 PTHREAD_LIBS=@PTHREAD_LIBS@
+ICONVDIR=@ICONVDIR@
diff --git a/configure.ac b/configure.ac
index 914ae57..d914cb9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -244,6 +244,8 @@ AS_HELP_STRING([--with-iconv=PATH],
 [PATH is prefix for libiconv library and headers])
 AS_HELP_STRING([],
 [used only if you need linking with libiconv]),
+NEEDS_LIBICONV=YesPlease
+OLD_ICONV=
 GIT_PARSE_WITH(iconv))
 
 ## --enable-FEATURE[=ARG] and --disable-FEATURE
@@ -307,6 +309,13 @@ GIT_PARSE_WITH(tcltk))
 AC_MSG_NOTICE([CHECKS for programs])
 #
 AC_PROG_CC([cc gcc])
+AC_C_INLINE
+case $ac_cv_c_inline in
+  no)		AC_SUBST([INLINE], []) ;;
+  inline | yes)	AC_SUBST([INLINE], [inline]) ;;
+  *)		AC_SUBST([INLINE], [$ac_cv_c_inline]) ;;
+esac
+
 # which switch to pass runtime path to dynamic libraries to the linker
 AC_CACHE_CHECK([if linker supports -R], git_cv_ld_dashr, [
    SAVE_LDFLAGS="${LDFLAGS}"
@@ -531,13 +540,11 @@ AC_CHECK_LIB([c], [hstrerror],
 [NEEDS_RESOLV=],
 [NEEDS_RESOLV=YesPlease])
 AC_SUBST(NEEDS_RESOLV)
-test -n "$NEEDS_RESOLV" && LIBS="$LIBS -lresolv"
 
 AC_CHECK_LIB([c], [basename],
 [NEEDS_LIBGEN=],
 [NEEDS_LIBGEN=YesPlease])
 AC_SUBST(NEEDS_LIBGEN)
-test -n "$NEEDS_LIBGEN" && LIBS="$LIBS -lgen"
 
 ## Checks for header files.
 AC_MSG_NOTICE([CHECKS for header files])
@@ -578,6 +585,12 @@ AC_SUBST(OLD_ICONV)
 ## Checks for typedefs, structures, and compiler characteristics.
 AC_MSG_NOTICE([CHECKS for typedefs, structures, and compiler characteristics])
 #
+TYPE_SOCKLEN_T
+case $ac_cv_type_socklen_t in
+  yes) 		AC_SUBST([SOCKLEN_T], [socklen_t]) ;;
+  *)  		AC_SUBST([SOCKLEN_T], [$git_cv_socklen_t_equiv]) ;;
+esac
+
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 AC_CHECK_MEMBER(struct dirent.d_ino,
 [NO_D_INO_IN_DIRENT=],
@@ -761,6 +774,19 @@ GIT_CHECK_FUNC(mkstemps,
 AC_SUBST(NO_MKSTEMPS)
 #
 #
+# Define NO_INET_NTOP if you don't have inet_ntop
+AC_CHECK_FUNC(inet_ntop,
+[NO_INET_NTOP=],
+[NO_INET_NTOP=YesPlease])
+AC_SUBST(NO_INET_NTOP)
+#
+# Define NO_INET_PTON if you don't have inet_ntop
+AC_CHECK_FUNC(inet_pton,
+[NO_INET_PTON=],
+[NO_INET_PTON=YesPlease])
+AC_SUBST(NO_INET_PTON)
+
+#
 # Define NO_MMAP if you want to avoid mmap.
 #
 # Define NO_ICONV if your libc does not properly support iconv.
@@ -782,7 +808,11 @@ AC_DEFUN([PTHREADTEST_SRC], [
 int main(void)
 {
 	pthread_mutex_t test_mutex;
-	return (0);
+	int retcode = 0;
+	retcode |= pthread_mutex_init(&test_mutex,(void*)0);
+	retcode |= pthread_mutex_lock(&test_mutex);
+	retcode |= pthread_mutex_unlock(&test_mutex);
+	return retcode;
 }
 ])
 
@@ -799,7 +829,8 @@ if test -n "$USER_NOPTHREAD"; then
 # handle these separately since PTHREAD_CFLAGS could be '-lpthreads
 # -D_REENTRANT' or some such.
 elif test -z "$PTHREAD_CFLAGS"; then
-  for opt in -pthread -lpthread; do
+  threads_found=no
+  for opt in -mt -pthread -lpthread; do
      old_CFLAGS="$CFLAGS"
      CFLAGS="$opt $CFLAGS"
      AC_MSG_CHECKING([Checking for POSIX Threads with '$opt'])
@@ -807,11 +838,18 @@ elif test -z "$PTHREAD_CFLAGS"; then
 	[AC_MSG_RESULT([yes])
 		NO_PTHREADS=
 		PTHREAD_LIBS="$opt"
+		PTHREAD_CFLAGS="$opt"
+		threads_found=yes
 		break
 	],
 	[AC_MSG_RESULT([no])])
       CFLAGS="$old_CFLAGS"
   done
+  if test $threads_found != yes; then
+    AC_CHECK_LIB([pthread], [pthread_create],
+	[PTHREAD_LIBS="-lpthread"],
+	[NO_PTHREADS=UnfortunatelyYes])
+  fi
 else
   old_CFLAGS="$CFLAGS"
   CFLAGS="$PTHREAD_CFLAGS $CFLAGS"
@@ -828,13 +866,10 @@ fi
 
 CFLAGS="$old_CFLAGS"
 
+AC_SUBST(PTHREAD_CFLAGS)
 AC_SUBST(PTHREAD_LIBS)
 AC_SUBST(NO_PTHREADS)
 
 ## Output files
 AC_CONFIG_FILES(["${config_file}":"${config_in}":"${config_append}"])
 AC_OUTPUT
-
-
-## Cleanup
-rm -f "${config_append}"
diff --git a/git-compat-util.h b/git-compat-util.h
index a3c4537..31057fe 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -55,13 +55,14 @@
 # else
 # define _XOPEN_SOURCE 500
 # endif
-#elif !defined(__APPLE__) && !defined(__FreeBSD__)  && !defined(__USLC__) && !defined(_M_UNIX) && !defined(sgi)
+#elif !defined(__APPLE__) && !defined(__FreeBSD__)  && !defined(__USLC__) && !defined(_M_UNIX) && !defined(__sgi)
 #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
 #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
 #endif
 #define _ALL_SOURCE 1
 #define _GNU_SOURCE 1
 #define _BSD_SOURCE 1
+#define _BSD_TYPES 1 /* IRIX needs this for u_short et al */
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1
 
@@ -216,7 +217,6 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
 #define PROT_READ 1
 #define PROT_WRITE 2
 #define MAP_PRIVATE 1
-#define MAP_FAILED ((void*)-1)
 #endif
 
 #define mmap git_mmap
@@ -245,6 +245,10 @@ extern int git_munmap(void *start, size_t length);
 
 #endif /* NO_MMAP */
 
+#ifndef MAP_FAILED
+#define MAP_FAILED ((void*)-1)
+#endif
+
 #ifdef NO_ST_BLOCKS_IN_STRUCT_STAT
 #define on_disk_bytes(st) ((st).st_size)
 #else
diff --git a/perl/Makefile b/perl/Makefile
index 4ab21d6..7e4a407 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -5,6 +5,7 @@ makfile:=perl.mak
 
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 prefix_SQ = $(subst ','\'',$(prefix))
+libdir_SQ = $(subst ','\'',$(libdir))
 
 ifndef V
 	QUIET = @
@@ -38,7 +39,7 @@ $(makfile): ../GIT-CFLAGS Makefile
 	echo '	echo $(instdir_SQ)' >> $@
 else
 $(makfile): Makefile.PL ../GIT-CFLAGS
-	$(PERL_PATH) $< PREFIX='$(prefix_SQ)'
+	$(PERL_PATH) $< PREFIX='$(prefix_SQ)' INSTALLARCHLIB='$(libdir)/perl' INSTALLSITEARCH='$(libdir)/perl' INSTALLSITELIB='$(libdir)/perl'
 endif
 
 # this is just added comfort for calling make directly in perl dir
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 0b9deca..c3ce5f6 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -34,5 +34,5 @@ WriteMakefile(
 	VERSION_FROM    => 'Git.pm',
 	PM		=> \%pm,
 	MAKEFILE	=> 'perl.mak',
-	INSTALLSITEMAN3DIR => '$(SITEPREFIX)/share/man/man3'
+	INSTALLSITEMAN3DIR => '$(SITEPREFIX)/man/man3'
 );
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index e768c3e..25ce0c4 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -39,17 +39,20 @@ test_expect_success 'setup' '
     git config mergetool.mytool.trustExitCode true
 '
 
-test_expect_success 'custom mergetool' '
-    git checkout -b test1 branch1 &&
-    test_must_fail git merge master >/dev/null 2>&1 &&
-    ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
-    ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
-    ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
-    test "$(cat file1)" = "master updated" &&
-    test "$(cat file2)" = "master new" &&
-    test "$(cat subdir/file3)" = "master new sub" &&
-    git commit -m "branch1 resolved with mergetool"
-'
+# Disabled this test because it hangs forever, due to 'yes' never
+# exiting when git exits on some systems
+#
+# test_expect_success 'custom mergetool' '
+#     git checkout -b test1 branch1 &&
+#     test_must_fail git merge master >/dev/null 2>&1 &&
+#     ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
+#     ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
+#     ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
+#     test "$(cat file1)" = "master updated" &&
+#     test "$(cat file2)" = "master new" &&
+#     test "$(cat subdir/file3)" = "master new sub" &&
+#     git commit -m "branch1 resolved with mergetool"
+# '
 
 test_expect_success 'mergetool crlf' '
     git config core.autocrlf true &&
-- 
1.7.0.2

-- 
Gary V. Vaughan (gary@thewrittenword.com)

Re: [PATCH 1/5] General configuration portability (Resend)

[Johannes Schindelin]

Hi,

On Thu, 11 Mar 2010, Gary V. Vaughan wrote:

> Resending some patches that didn't make it, in the correct format
> and taking into account the feedback I already had on one of the
> later patches...
> 
> This patch is in part to deal with peculiarities of our build
> environment (software packages are installed in their own directory
> tree, for example, and oftentimes we need to set additional -I options
> in CPPFLAGS, or -L options in LDFLAGS, among others).
> 
> However this is also where we put configuration changes to add
> additional tests, or fix Makefile problems so that everything builds
> on all of our supported architectures.  Probably, much of this patch
> is useful upstream.
> ---
>  Makefile             |   50 +++++++++++++++++++++++++++++++++++++++---------
>  aclocal.m4           |   41 ++++++++++++++++++++++++++++++++++++++++
>  config.mak.in        |   10 ++++++++-
>  configure.ac         |   51 ++++++++++++++++++++++++++++++++++++++++++-------
>  git-compat-util.h    |    8 +++++-
>  perl/Makefile        |    3 +-
>  perl/Makefile.PL     |    2 +-
>  t/t7610-mergetool.sh |   25 +++++++++++++----------
>  8 files changed, 156 insertions(+), 34 deletions(-)
>  create mode 100644 aclocal.m4

Whoa.

Pretty large patch, no? And looking at things like this:

> diff --git a/Makefile b/Makefile
> index 8fdc421..6058805 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -243,7 +243,7 @@ endif
>  
>  CFLAGS = -g -O2 -Wall
>  LDFLAGS =
> -ALL_CFLAGS = $(CFLAGS)
> +ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)

It seems to me that this wants to be a relatively large patch series of 
well-contained, easy-to-review, tiny patches.

Am I correct?
Dscho

Re: [PATCH 1/5] General configuration portability (Resend)

[Brandon Casey]

On 03/11/2010 10:51 AM, Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 11 Mar 2010, Gary V. Vaughan wrote:
> 
>> Resending some patches that didn't make it, in the correct format
>> and taking into account the feedback I already had on one of the
>> later patches...
>>
>> This patch is in part to deal with peculiarities of our build
>> environment (software packages are installed in their own directory
>> tree, for example, and oftentimes we need to set additional -I options
>> in CPPFLAGS, or -L options in LDFLAGS, among others).
>>
>> However this is also where we put configuration changes to add
>> additional tests, or fix Makefile problems so that everything builds
>> on all of our supported architectures.  Probably, much of this patch
>> is useful upstream.
>> ---
>>  Makefile             |   50 +++++++++++++++++++++++++++++++++++++++---------
>>  aclocal.m4           |   41 ++++++++++++++++++++++++++++++++++++++++
>>  config.mak.in        |   10 ++++++++-
>>  configure.ac         |   51 ++++++++++++++++++++++++++++++++++++++++++-------
>>  git-compat-util.h    |    8 +++++-
>>  perl/Makefile        |    3 +-
>>  perl/Makefile.PL     |    2 +-
>>  t/t7610-mergetool.sh |   25 +++++++++++++----------
>>  8 files changed, 156 insertions(+), 34 deletions(-)
>>  create mode 100644 aclocal.m4
> 
> Whoa.
> 
> Pretty large patch, no? And looking at things like this:
> 
>> diff --git a/Makefile b/Makefile
>> index 8fdc421..6058805 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -243,7 +243,7 @@ endif
>>  
>>  CFLAGS = -g -O2 -Wall
>>  LDFLAGS =
>> -ALL_CFLAGS = $(CFLAGS)
>> +ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
> 
> It seems to me that this wants to be a relatively large patch series of 
> well-contained, easy-to-review, tiny patches.
> 
> Am I correct?

Yes, please.

-brandon

Re: [PATCH 1/5] General configuration portability (Resend)

[Gary V. Vaughan]

Hi Ben,

On Thu, Mar 11, 2010 at 12:01:51PM -0500, Ben Walton wrote:
> Excerpts from Gary V. Vaughan's message of Thu Mar 11 11:27:50 -0500 2010:
> > -gitexecdir = libexec/git-core
> > +gitexecdir = libexec
> 
> You're proposing to change the execdir?

>From the OP:

  This patch is in part to deal with peculiarities of our build                 
  environment [[...]] Probably, much of this patch is useful upstream.

We install each package in it's own directory, and remove redundant
directories as a local policy.  So /opt/fsw/git17/libexec/git-core
becomes /opt/fsw/git17/libexec.

My employer kindly agreed to donate a little of my paid time to push
the git patches we require to build on our supported hosts back
upstream.  There is certainly some good stuff in here that is useful
outside of our environment, but I have another 1600 packages to port
too...

Cheers,
    Gary
-- 
Gary V. Vaughan (gary@thewrittenword.com)

Re: [PATCH 1/5] General configuration portability (Resend)

[Sverre Rabbelier]

Heya,

On Fri, Mar 12, 2010 at 07:23, Gary V. Vaughan
<git@mlists.thewrittenword.com> wrote:
> My employer kindly agreed to donate a little of my paid time to push
> the git patches we require to build on our supported hosts back
> upstream.  There is certainly some good stuff in here that is useful
> outside of our environment, but I have another 1600 packages to port
> too...

While understandable, I hope your employer also understands that just
dumping a large patch upstream and tell them "have fun with it" does
not a good open source contribution make? Consider the time balance:
the investment required for us to figure out whether this patch is any
good for git, (not being familiar with the changes you're introducing,
and probably unable to test them since we lack the setup), and the
time it will cost you to split up the patches into an easily
reviewable series. I hope you (and your employer?) will agree that
it'd be less time for you to do so, than for us :).

Of course, if your employer is more like "pointy-haired boss" there's
not much you can do either.

-- 
Cheers,

Sverre Rabbelier