Rewrite setup.py and associated logic to fix multiple build problems.

The recent distcheck revealed that paths were set incorrectly for the setup.py
script.  Also, as I dug deeper, it turned out that built files were placed in
the src directory (thanks to os.path.join I found in the distutil's source code,
that trips up the location if relative paths contains too many '..'s.).  [The
distccheck revealed this!]  I didn't like either that setup was run in the
source directory and not from the build directory (that's asking for trouble).
I did't like either that the search path, the list of includes, in setup.py was
handcoded and not simply that of CPPFLAGS.  Finally, I was confused by _builddir
being another build directory under $(buildir).  I renamed the former
_include_server to better explain its role.

So, I rewrote most of setup.py (correcting some of the documentation strings
along the way). Also, I partly removed the 'jumping through the hoops'
gymnastics from the Makefile.

TESTING: 'make distcheck'
REVIEWERS: csilvers, fergus

4 files changed, 189 insertions, 95 deletions

diff --git a/Makefile.in b/Makefile.in
index 6e7d937..908ffdf 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -48,7 +48,7 @@ oldincludedir = /usr/include
 docdir = @docdir@
 pkgdatadir = $(datadir)/@PACKAGE_NAME@
 
-include_server_builddir = $(builddir)/_builddir
+include_server_builddir = $(builddir)/_include_server
 
 # These must be done from here, not from autoconf, because they can 
 # contain variable expansions written in Make syntax.  Ew.
@@ -541,21 +541,21 @@ distccmon-gnome@EXEEXT@: $(mon_obj) $(gnome_obj)
 		$(mon_obj) $(gnome_obj) \
 		$(LIBS) $(GNOME_CFLAGS) $(GNOME_LIBS)
 
-# The include-server is a python app, so we use Python's build system.
-# The jumping through hoops with cd is to normalize builddir if it's relative.
-# And we always set BUILDDIR and DISTCC_VERSION since setup.py looks for them.
+# The include-server is a python app, so we use Python's build system.  We pass
+# the distcc version, the source location, the CPP flags (for location of the
+# includes), and the build location.
 include-server:
 	if test -z "$(INCLUDESERVER_PYTHON)"; then	\
 	  echo "Not building $@: No suitable python found"; \
 	else						\
-	  CURDIR=`pwd` && mkdir -p "$(include_server_builddir)" && \
-	  cd "$(include_server_builddir)" && BUILDDIR=`pwd` && export BUILDDIR && \
-	  cd "$$CURDIR" && cd "$(srcdir)/include_server" &&	\
+	  mkdir -p "$(include_server_builddir)" &&      \
 	  DISTCC_VERSION="$(VERSION)"			\
-	  $(INCLUDESERVER_PYTHON) ./setup.py		\
+	  SRCDIR="$(srcdir)"                            \
+	  CPPFLAGS="$(CPPFLAGS)"                        \
+	  $(INCLUDESERVER_PYTHON) "$(srcdir)/include_server/setup.py" \
 	      build 					\
-	         --build-base="$$BUILDDIR" 		\
-	         --build-temp="$$BUILDDIR";             \
+	        --build-base="$(include_server_builddir)"  \
+	        --build-temp="$(include_server_builddir)"; \
 	fi
 
 
@@ -867,17 +867,16 @@ clean-autoconf:
 clean-lzo:
 	rm -f lzo/*.[od] lzo/testmini
 
-# The jumping through hoops with cd is to normalize build/destdir if relative.
-# And we always set BUILDDIR and DISTCC_VERSION since setup.py looks for them.
+
 clean-include-server:
 	if test -n "$(INCLUDESERVER_PYTHON)"; then	\
-	  CURDIR=`pwd` && mkdir -p "$(include_server_builddir)" && \
-	  cd "$(include_server_builddir)" && BUILDDIR=`pwd` && export BUILDDIR && \
-	  cd "$$CURDIR" && cd "$(srcdir)/include_server" &&	\
 	  DISTCC_VERSION="$(VERSION)"			\
-	  $(INCLUDESERVER_PYTHON) ./setup.py clean	\
-	     --build-base="$$BUILDDIR" 			\
-	     --build-temp="$$BUILDDIR";			\
+	  SRCDIR="$(srcdir)"                            \
+	  CPPFLAGS="$(CPPFLAGS)"                        \
+	  $(INCLUDESERVER_PYTHON) "$(srcdir)/include_server/setup.py" \
+	      clean	\
+	         --build-base="$(include_server_builddir)"  \
+	         --build-temp="$(include_server_builddir)"; \
 	fi
 
 maintainer-clean: distclean \
@@ -966,25 +965,28 @@ install-programs: $(bin_PROGRAMS) pump
 	  $(INSTALL_PROGRAM) "$$p" "$(DESTDIR)$(bindir)" || exit 1; \
 	done
 
-# The jumping through hoops with cd is to normalize build/destdir if relative.
-# And we always set BUILDDIR and DISTCC_VERSION since setup.py looks for them.
+# See comments for the include-server target.  Also, we work around an issue in
+# the change_root function of distutils/utils.py that turns the absolute prefix
+# into a relative reference if the root is the empty string: we absolutize the
+# destination directory in order to substitute '/' for the empty prefix (the
+# default) when defining the '--root' parameter.
 install-include-server: include-server
 	if test -z "$(INCLUDESERVER_PYTHON)"; then	\
 	  echo "Not building $@: No suitable python found"; \
 	else						\
-	  CURDIR=`pwd` && 				\
-	  cd "$(include_server_builddir)" && BUILDDIR=`pwd` && export BUILDDIR && \
-	  cd "$(DESTDIR)" && DESTDIR=`pwd` && export DESTDIR && cd "$$CURDIR" && \
-	  cd "$(srcdir)/include_server" && 		\
+	  mkdir -p "$(include_server_builddir)" &&      \
+	  DESTDIR=`cd "$(DESTDIR)/" && pwd` &&            \
 	  DISTCC_VERSION="$(VERSION)"			\
-	  $(INCLUDESERVER_PYTHON) ./setup.py 		\
+	  SRCDIR="$(srcdir)"                            \
+	  CPPFLAGS="$(CPPFLAGS)"                        \
+	  $(INCLUDESERVER_PYTHON) "$(srcdir)/include_server/setup.py" \
 	      build 					\
-	         --build-base="$$BUILDDIR" 		\
-	         --build-temp="$$BUILDDIR"		\
+	        --build-base="$(include_server_builddir)" \
+	        --build-temp="$(include_server_builddir)" \
 	      install 					\
 	         --prefix="$(prefix)" 			\
 	         --record="$(PYTHON_INSTALL_RECORD)"   	\
-	         --root="$$DESTDIR";			\
+	         --root="$$DESTDIR";                    \
 	fi
 
 install-man: $(man1_MEN)
diff --git a/find_c_extension.sh b/find_c_extension.sh
index 0dbc3cd..77aa838 100755
--- a/find_c_extension.sh
+++ b/find_c_extension.sh
@@ -6,14 +6,14 @@
 #
 # More precisely, locate the unique directory of the form:
 #
-#   _builddir/lib.*/include_server/
+#   _include_server/lib.*/include_server/
 #
 # that contains file 'distcc_pump_c_extensions.so'. Write the path of this 
 # directory to stdout and exit with status 0. If such a path does not exist 
 # then write error message to stderr and exit with status 1.
 
 builddir=$1
-so_files=`ls $builddir/_builddir/lib.*/include_server/\
+so_files=`ls $builddir/_include_server/lib.*/include_server/\
 distcc_pump_c_extensions.so`
 if [ -z "$so_files" ]; then
   echo \
diff --git a/include_server/setup.py b/include_server/setup.py
index 47ec623..7eed92b 100755
--- a/include_server/setup.py
+++ b/include_server/setup.py
@@ -1,4 +1,4 @@
-#! /usr/bin/python2.4
+#!/usr/bin/python2.4
 
 # Copyright 2007 Google Inc.
 # 
@@ -18,78 +18,170 @@
 
 """Build the include server module.
 
-Note: the version number should be passed to this script through
-the environment variable DISTCC_VERSION.
+The version number should be passed to this script through the environment
+variable DISTCC_VERSION.  Also, the CPPFLAGS of the Makefile must be passed
+through the enviroment.  This is how we figure out the locations of include
+directories. SRCDIR must be passed as well; it explains where to find the C
+sources and the include_server directory of Python source files and C
+extensions.  Because SRCDIR is appended to build location and we don't want to
+end up outside this location through a relative SRCDIR path with '..'s, SRCDIR
+is absolutized.
 """
 
-__author__ = "Manos Renieris"
+__author__ = 'Manos Renieris and Nils Klarlund'
 
-import distutils
+import distutils.core
+import distutils.extension
+import doctest
 import os
-from distutils.core import setup
-from distutils.extension import Extension
-
-ext = Extension(
-    name="include_server.distcc_pump_c_extensions",
-    sources=[
-             '../src/clirpc.c',
-             '../src/clinet.c',
-             '../src/state.c',
-             '../src/srvrpc.c',
-             '../src/pump.c',
-             '../src/rpc.c',
-             '../src/io.c',
-             '../src/include_server_if.c',
-             '../src/trace.c',
-             '../src/util.c',
-             '../src/tempfile.c',
-             '../src/filename.c',
-             '../src/bulk.c',
-             '../src/sendfile.c',
-             '../src/compress.c',
-             '../src/argutil.c',
-             '../src/cleanup.c',
-             '../src/emaillog.c',
-             '../src/timeval.c',
-             '../src/netutil.c',
-             '../lzo/minilzo.c',
-             'c_extensions/distcc_pump_c_extensions_module.c',
-             ],
-    include_dirs = ["../src", 
-                    "../lzo",
-                    os.path.join(os.getenv("BUILDDIR") or "", 
-                                 "src"),
-                    os.path.join(os.getenv("BUILDDIR") or "", 
-                                 "../src"),
-                    os.path.join(os.getenv("BUILDDIR") or "", 
-                                 "../../src"),
-                   ],
-    define_macros = [('_GNU_SOURCE', 1)],
-    library_dirs = [],
-    libraries = [],
-    runtime_library_dirs = [],
-    extra_objects = [],
+import shlex
+import sys
+
+OPTIONS_NOT_ALLOWED = ['-Iquote', '-Isystem', '-I-']
+
+# We include a partial command line parser instead of using the more the more
+# complicated one in parse_command.py.  This cuts down on dependencies in the
+# build system itself.
+
+
+def GetIncludes(flags):
+  """Parse a flags string for includes of the form -I<DIR>.
+
+  Args:
+    flags: a string in shell syntax denoting compiler options
+  Returns:
+    a list of <DIR>s of the includes
+  Raises:
+    ValueError:
+
+  In the doctests below, note that a single quoted backslash takes four
+  backslashes to represent if it is inside a single quoted string inside this
+  present triple-quoted string.
+    
+  >>> GetIncludes('-I x -X -I"y" -Y')
+  ['x', 'y']
+  >>> GetIncludes('-Ix -Dfoo -Iy')
+  ['x', 'y']
+  >>> GetIncludes(r'-Ix -I"y\\z" -I"y\\\\z" -Y')
+  ['x', 'y\\\\z', 'y\\\\z']
+  >>> GetIncludes('-DX -Iquote Y')
+  Traceback (most recent call last):
+        ...
+  ValueError: These options are not allowed: -Iquote, -Isystem, -I-.
+  >>> GetIncludes('-DX -I x -I')
+  Traceback (most recent call last):
+        ...
+  ValueError: Argument expected after '-I'.
+  """
+  flags = shlex.split(flags)
+  if set(OPTIONS_NOT_ALLOWED) & set(flags):
+    raise ValueError('These options are not allowed: %s.'
+                     % ', '.join(OPTIONS_NOT_ALLOWED))
+  # Fish out the directories of '-I' options.
+  i = 0
+  inc_dirs = []
+  while i < len(flags):
+    if flags[i].startswith('-I'):
+      inc_dir = flags[i][len('-I'):]
+      if inc_dir:
+        # "-Idir"
+        inc_dirs.append(inc_dir)
+        i += 1
+        continue
+      else:
+        # "-I dir"
+        if i == len(flags) - 1:
+          raise ValueError("Argument expected after '-I'.")
+        inc_dirs.append(flags[i+1])
+        i += 2
+    else:
+      i += 1
+  return inc_dirs
+
+cpp_flags_env = os.getenv('CPPFLAGS', '')
+if not cpp_flags_env:
+  # Don't quit; perhaps the user is asking for help using '--help'.
+  # CPPFLAGS checking.
+  print >> sys.stderr, 'setup.py: CPPFLAGS must be defined.'
+# CPPFLAGS is passed to us as it's used in the Makefile: a string that the shell
+# will interpret.  GetInclude uses shlex to do the same kind of interpretation
+# in order to identify the include directory options.
+cpp_flags_includes = GetIncludes(cpp_flags_env)
+
+# SRCDIR checking.
+if not os.getenv('SRCDIR'):
+  # Don't quit; perhaps the user is asking for help using '--help'.
+  print >> sys.stderr, 'setup.py: SRCDIR must be defined.'
+  srcdir = 'UNDEFINED'
+  srcdir_include_server = 'UNDEFINED'
+else:
+  # The distutils build system appends the source location to the build
+  # location, and so to avoid that relative source paths with '..' make built
+  # files end up outside the build location, the location is changed to an
+  # absolute path.
+  srcdir = os.path.abspath(os.getenv('SRCDIR'))
+  if not os.path.isdir(srcdir):
+    sys.exit("""Could not cd to SRCDIR '%s'.""" % srcdir)
+  srcdir_include_server = os.path.join(srcdir, 'include_server')
+
+# Specify extension.
+ext = distutils.extension.Extension(
+    name='include_server.distcc_pump_c_extensions',
+    sources=[os.path.join(srcdir, source)
+             for source in
+             ['src/clirpc.c',
+              'src/clinet.c',
+              'src/state.c',
+              'src/srvrpc.c',
+              'src/pump.c',
+              'src/rpc.c',
+              'src/io.c',
+              'src/include_server_if.c',
+              'src/trace.c',
+              'src/util.c',
+              'src/tempfile.c',
+              'src/filename.c',
+              'src/bulk.c',
+              'src/sendfile.c',
+              'src/compress.c',
+              'src/argutil.c',
+              'src/cleanup.c',
+              'src/emaillog.c',
+              'src/timeval.c',
+              'src/netutil.c',
+              'lzo/minilzo.c',
+              'include_server/c_extensions/distcc_pump_c_extensions_module.c',
+             ]],
+    include_dirs=cpp_flags_includes,
+    define_macros=[('_GNU_SOURCE', 1)],
+    library_dirs=[],
+    libraries=[],
+    runtime_library_dirs=[],
+    extra_objects=[],
     # This is the same list as is in configure.ac, except we leave out
     # -Wmissing-prototypes and -Wmissing-declarations, which don't apply
     # to python extensions (it exports global fns via a pointer),
     # and -Wwrite-strings, which just had too many false positives.
-    extra_compile_args = ("-W -Wall -Wimplicit -Wuninitialized "
-                          "-Wshadow -Wpointer-arith -Wcast-align "
-                          "-Waggregate-return -Wstrict-prototypes "
-                          "-Wnested-externs -Werror").split()
-)
+    extra_compile_args=('-W -Wall -Wimplicit -Wuninitialized '
+                        '-Wshadow -Wpointer-arith -Wcast-align '
+                        '-Waggregate-return -Wstrict-prototypes '
+                        '-Wnested-externs -Werror').split())
 
 args = {
-  'name': "include_server",
-  'package_dir': {'include_server':'.'},
-  'version': os.getenv("DISTCC_VERSION") or 'unknown',
-  'description': "Include server for distcc-pump",
-  'author': "Nils Klarlund",
-  'author_email': "opensource@google.com",
-  'url': 'http://code.google.com/p/distcc-pump',
-  'long_description': """The include server is part of distcc-pump.""",
-  'packages': ["include_server"],
-  'ext_modules': [ext],
+    'name': 'include_server',
+    # The 'include_server' package is in the srcdir_include_server.
+    'package_dir': {'include_server': srcdir_include_server},
+    'version': os.getenv('DISTCC_VERSION') or 'unknown',
+    'description': """Include server for distcc's pump-mode""",
+    'author': 'Nils Klarlund',
+    'author_email': 'opensource@google.com',
+    'url': 'http://code.google.com/p/distcc',
+    'long_description': 'The include server is part of distcc.',
+    'packages': ['include_server'],
+    'ext_modules': [ext],
 }
 
-setup(**args)
+# First, do a little self-testing.
+doctest.testmod()
+# Second, do the setup.
+distutils.core.setup(**args)
diff --git a/pump.in b/pump.in
index 79d763a..375e3aa 100755
--- a/pump.in
+++ b/pump.in
@@ -127,7 +127,7 @@ Initialize() {
   # set when the 'pump' script is made from 'pump.in'. Use this value in the
   # formation of the relative path that takes us from the 'bin' directory of an
   # installation to the 'include_server' directory.
-  include_server_relative="../lib/$PYTHON/site-packages/include_server"
+  include_server_relative="../lib/$(basename $PYTHON)/site-packages/include_server"
 
   # We use a little heuristic to determine whether this pump script is part of
   # an installation. Specifically, we check whether we're a bin directory, and