Re: RFC: Build system changes
On Fri, Dec 14, 2012 at 01:25:05AM -0500, Greg Stein wrote: Consider this my typical potty-mouth response. I'll skip the actuals, and move along in peace... Save it for later. I'll ask for more next time we have a drink together :)
RFC: Build system changes
The attached patch makes several changes to how we discover compilers
and set flags on *nix:
* Search for clang as well as the default gcc/cc, and prefer clang(++)
over gcc/g++.
* Set standards-compliance mode (C90/C++11) even without maintainer-mode.
* Add -pipe to C(XX)FLAGS if the compiler supports it. This speeds up
compilation a bit in my tests.
I tested this on Mac OS (with clang) and Linux (without clang) but don't
know how it affects bindings.
Please review.
-- Brane
{{{
* build/ac-macros/compiler.m4: New file.
(SVN_PROG_CC, SVN_CFLAGS_ADD_IFELSE,
SVN_PROG_CXX, SVN_CXXFLAGS_ADD_IFELSE): New.
* aclocal.m4: Include build/ac-macros/compiler.m4.
* configure.ac:
- Use SVN_PROG_CC instead of AC_PROG_CC
- Use SVN_PROG_CXX instead of AC_PROG_CXX
- Use SVN_CFLAGS_ADD_IFELSE and SVN_CXXFLAGS_ADD_IFELSE
to add mainter-mode flags
- Do not remove -ansi for clang, since it's not set any more.
}}}
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Index: aclocal.m4
===
--- aclocal.m4 (revision 1421399)
+++ aclocal.m4 (working copy)
@@ -36,6 +36,7 @@
sinclude(build/ac-macros/aprutil.m4)
sinclude(build/ac-macros/apr_memcache.m4)
sinclude(build/ac-macros/berkeley-db.m4)
+sinclude(build/ac-macros/compiler.m4)
sinclude(build/ac-macros/ctypesgen.m4)
sinclude(build/ac-macros/gssapi.m4)
sinclude(build/ac-macros/java.m4)
Index: configure.ac
===
--- configure.ac(revision 1421399)
+++ configure.ac(working copy)
@@ -48,12 +48,10 @@
# Check for programs
-# Look for a C compiler (before anything can set CFLAGS)
-AC_PROG_CC
+# Look for a C and C++ compiler (before anything can set CFLAGS or CXXFLAGS)
+SVN_PROG_CC
+SVN_PROG_CXX
-# Look for a C++ compiler
-AC_PROG_CXX
-
# Look for a C pre-processor
AC_PROG_CPP
@@ -990,62 +988,31 @@
AC_MSG_NOTICE([maintainer-mode: adding GCC warning flags])
dnl Enable some extra warnings. Put these before the user's flags
dnl so the user can specify flags that override these.
-CFLAGS=-Wpointer-arith -Wwrite-strings -Wshadow -ansi -Wall
-Wformat=2 -Wunused -Waggregate-return -Wstrict-prototypes -Wmissing-prototypes
-Wmissing-declarations -Wno-multichar -Wredundant-decls -Wnested-externs
-Wunreachable-code -Winline -Wno-long-long $CFLAGS
-CXXFLAGS=-Wpointer-arith -Wwrite-strings -Wshadow -ansi -Wall
$CXXFLAGS
+dnl Add each of the following flags only if the C compiler accepts it.
+SVN_CFLAGS_ADD_IFELSE([-Werror=declaration-after-statement])
+SVN_CFLAGS_ADD_IFELSE([-Wextra-tokens])
+SVN_CFLAGS_ADD_IFELSE([-Wnewline-eof])
+SVN_CFLAGS_ADD_IFELSE([-Wshorten-64-to-32])
+SVN_CFLAGS_ADD_IFELSE([-Wold-style-definition])
+SVN_CFLAGS_ADD_IFELSE([-Wno-format-nonliteral])
+SVN_CFLAGS_ADD_IFELSE([-Wno-system-headers])
+
+dnl Add each of the following flags only if the C++ compiler accepts
it.
+SVN_CXXFLAGS_ADD_IFELSE([-Wextra-tokens])
+SVN_CXXFLAGS_ADD_IFELSE([-Wshorten-64-to-32])
+SVN_CXXFLAGS_ADD_IFELSE([-Wno-system-headers])
+
+dnl Finally, prepend a standard set of extra warnings that
+dnl even very old compilers understand
+CFLAGS=-Wpointer-arith -Wwrite-strings -Wshadow -Wall -Wformat=2
-Wunused -Waggregate-return -Wstrict-prototypes -Wmissing-prototypes
-Wmissing-declarations -Wno-multichar -Wredundant-decls -Wnested-externs
-Wunreachable-code -Winline -Wno-long-long $CFLAGS
+CXXFLAGS=-Wpointer-arith -Wwrite-strings -Wshadow -Wall $CXXFLAGS
+
dnl some additional flags that can be handy for an occasional review,
dnl but throw too many warnings in svn code, of too little importance,
dnl to keep these enabled. Remove the dnl to do a run with these
dnl switches enabled.
dnl CFLAGS=-Wswitch-enum -Wswitch-default $CFLAGS
-
-dnl Add each of the following flags only if the C compiler accepts it.
-
-CFLAGS_KEEP=$CFLAGS
-AC_LANG_PUSH([C])
-
-CFLAGS=-Werror=declaration-after-statement $CFLAGS_KEEP
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[]])], [CFLAGS_KEEP=$CFLAGS])
-
-CFLAGS=-Wextra-tokens $CFLAGS_KEEP
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[]])], [CFLAGS_KEEP=$CFLAGS])
-
-CFLAGS=-Wnewline-eof $CFLAGS_KEEP
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[]])], [CFLAGS_KEEP=$CFLAGS])
-
-CFLAGS=-Wshorten-64-to-32 $CFLAGS_KEEP
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[]])], [CFLAGS_KEEP=$CFLAGS])
-
-CFLAGS=-Wold-style-definition $CFLAGS_KEEP
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[]])], [CFLAGS_KEEP=$CFLAGS])
-
-CFLAGS=-Wno-system-headers $CFLAGS_KEEP
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[]])], [CFLAGS_KEEP=$CFLAGS])
-
-
Re: RFC: Build system changes
Branko Čibej br...@wandisco.com writes:
{{{
* build/ac-macros/compiler.m4: New file.
(SVN_PROG_CC, SVN_CFLAGS_ADD_IFELSE,
SVN_PROG_CXX, SVN_CXXFLAGS_ADD_IFELSE): New.
* aclocal.m4: Include build/ac-macros/compiler.m4.
* configure.ac:
- Use SVN_PROG_CC instead of AC_PROG_CC
- Use SVN_PROG_CXX instead of AC_PROG_CXX
- Use SVN_CFLAGS_ADD_IFELSE and SVN_CXXFLAGS_ADD_IFELSE
to add mainter-mode flags
- Do not remove -ansi for clang, since it's not set any more.
}}}
When running autogen.sh I get:
configure.ac:52: warning: AC_REQUIRE: `AC_PROG_CC' was expanded before it was
required
configure.ac:52:
http://www.gnu.org/software/autoconf/manual/autoconf.html#Expanded-Before-Required
../../lib/autoconf/c.m4:429: AC_LANG_COMPILER(C) is expanded from...
../../lib/autoconf/lang.m4:329: AC_LANG_COMPILER_REQUIRE is expanded from...
../../lib/autoconf/general.m4:2606: AC_COMPILE_IFELSE is expanded from...
build/ac-macros/compiler.m4:33: _SVN_XXFLAGS_ADD_IFELSE is expanded from...
build/ac-macros/compiler.m4:50: SVN_CFLAGS_ADD_IFELSE is expanded from...
build/ac-macros/compiler.m4:57: SVN_PROG_CC is expanded from...
configure.ac:52: the top level
and similar for AC_PROG_CXX, both repeated with different line numbers.
Linking fails:
cd subversion/libsvn_subr /bin/bash /home/pm/sw/subversion/obj/libtool
--tag=CC --silent --mode=link gcc -shared -std=c90 -pipe -pthread
-Werror=implicit-function-declaration -DSVN_DEBUG -DAP_DEBUG -rpath
/usr/local/subversionx/lib -version-info 0 -o libsvn_subr-1.la adler32.lo
atomic.lo auth.lo base64.lo cache-inprocess.lo cache-membuffer.lo
cache-memcache.lo cache.lo cache_config.lo checksum.lo cmdline.lo compat.lo
config.lo config_auth.lo config_file.lo config_win.lo crypto.lo ctype.lo
date.lo debug.lo deprecated.lo dirent_uri.lo dso.lo eol.lo error.lo
gpg_agent.lo hash.lo io.lo iter.lo lock.lo log.lo macos_keychain.lo magic.lo
md5.lo mergeinfo.lo mutex.lo named_atomic.lo nls.lo opt.lo path.lo pool.lo
prompt.lo properties.lo pseudo_md5.lo quoprint.lo sha1.lo simple_providers.lo
skel.lo sorts.lo spillbuf.lo sqlite.lo sqlite3wrapper.lo
ssl_client_cert_providers.lo ssl_client_cert_pw_providers.lo
ssl_server_trust_providers.lo stream.lo string.lo subst.lo sysinfo.lo target.lo
temp_serializer.lo time.lo token.lo types.lo user.lo username_providers.lo
utf.lo utf_validate.lo utf_width.lo validate.lo version.lo win32_crashrpt.lo
win32_crypto.lo win32_xlate.lo xml.lo -laprutil-1 -lapr-1 -lexpat -lz
-lsqlite3 -lmagic
gcc: error: unrecognized command line option '-soname'
gcc: error: libsvn_subr-1.so.0: No such file or directory
make: *** [subversion/libsvn_subr/libsvn_subr-1.la] Error 1
--
Certified Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: RFC: Build system changes
* Search for clang as well as the default gcc/cc, and prefer clang(++) over gcc/g++. Is clang considered superior, then? Fair enough, I haven't really kept up. * Add -pipe to C(XX)FLAGS if the compiler supports it. This speeds up compilation a bit in my tests. Hmm. It seems very strange to me that the compiler driver cannot already auto-select the most efficient means: pipes, shared memory, temp files, or (in gcc's case) just integrating the preprocessor into the compiler binary directly. And indeed, I thought gcc -pipe had been basically obsolete for many years (still supported, but no faster than the default mode). Is it really faster for you? With what compiler and platform? Perhaps I should test it here. + dnl Try to turn on C++11 mode so that we can use + dnl std::shared_ptr and similar goodies + dnl g++ and clang++ + SVN_CXXFLAGS_ADD_IFELSE([-std=c++11],[],[ +SVN_CXXFLAGS_ADD_IFELSE([-std=c++0x]) + ]) This bit confuses me as well. Does our C++ code now require this mode? If not, what's the use of the flag? Peter
Re: RFC: Build system changes
On Thu, Dec 13, 2012 at 1:13 PM, Branko Čibej br...@wandisco.com wrote: The attached patch makes several changes to how we discover compilers and set flags on *nix: * Search for clang as well as the default gcc/cc, and prefer clang(++) over gcc/g++. * Set standards-compliance mode (C90/C++11) even without maintainer-mode. It seems a bit odd to allow use of C++11 features and yet still use C90 for the rest of the codebase. I realize the C++ code is largely limited to interfaces with lower-level libraries and bindings, but I would lean toward C++98, at least initially. (That is, unless you've got a compelling reason for rvalue references. :P ) -Hyrum
Re: RFC: Build system changes
On 14.12.2012 02:32, Hyrum K Wright wrote: On Thu, Dec 13, 2012 at 1:13 PM, Branko Čibej br...@wandisco.com wrote: The attached patch makes several changes to how we discover compilers and set flags on *nix: * Search for clang as well as the default gcc/cc, and prefer clang(++) over gcc/g++. * Set standards-compliance mode (C90/C++11) even without maintainer-mode. It seems a bit odd to allow use of C++11 features and yet still use C90 for the rest of the codebase. I realize the C++ code is largely limited to interfaces with lower-level libraries and bindings, but I would lean toward C++98, at least initially. (That is, unless you've got a compelling reason for rvalue references. :P ) The only reason for trying for C++11 is, as far as I'm concerned, getting std::shared_ptr memory. The C++ bindings I'm slowly wrapping my head around will need it, and I don't want to even consider using standard containers without it. The only alternative to C++11 is using Boost, and that's a last resort grabbing for straws, as far as I'm concerned. -- Brane P.S.: Nothing wrong with Boost as such, of course; but including boost/shared_ptr.hpp tends to pull in some 90% of Boost's headers, and I consider that overkill. -- Branko Čibej Director of Subversion | WANdisco | www.wandisco.com
Re: RFC: Build system changes
On Thu, Dec 13, 2012 at 10:21 PM, Branko Čibej br...@wandisco.com wrote: ... P.S.: Nothing wrong with Boost as such, of course; but including boost/shared_ptr.hpp tends to pull in some 90% of Boost's headers, and I consider that overkill. I'll go on record with Boost is the Automake of C++. Tons of useless crap that you don't actually need. I needed a couple boost bits for Apache Thrift, iirc, and to get them... ended up with something like 100k new files in /usr/local. Consider this my typical potty-mouth response. I'll skip the actuals, and move along in peace... -g
Re: RFC: Build system changes
On 14.12.2012 07:38, Miha Vitorovic wrote: On 14.12.2012 4:21, Branko Čibej wrote: The only reason for trying for C++11 is, as far as I'm concerned, getting std::shared_ptr memory. The C++ bindings I'm slowly wrapping my head around will need it, and I don't want to even consider using standard containers without it. The only alternative to C++11 is using Boost, and that's a last resort grabbing for straws, as far as I'm concerned. -- Brane P.S.: Nothing wrong with Boost as such, of course; but including boost/shared_ptr.hpp tends to pull in some 90% of Boost's headers, and I consider that overkill. If your only need is shared_ptr why not use std::tr1? A quick research shows that it's bundled with all compilers. Yes, I realised that and already changed the code that way. It's not exactly bundled with /all/ compilers, but that's a detail I wont' worry about right now. -- Brane -- Branko Čibej Director of Subversion | WANdisco | www.wandisco.com
