On the advice of Bernard Li, I have posted the patch (and a copy of its
associated walk-through) to the Ganglia Bugzilla:
http://bugzilla.ganglia.info/cgi-bin/bugzilla/show_bug.cgi?id=112
Below is a full copy of the original post, sans patch, for completeness.
(The original post has been cancelled out of the moderator queue.)
>From [EMAIL PROTECTED] Fri Aug 25 05:34:01 2006
Date: Sat, 12 Aug 2006 02:22:01 -0400
From: "Daniel Richard G." <[EMAIL PROTECTED]>
To: ganglia-developers@lists.sourceforge.net
Subject: [PATCH] General cleanup
Message-ID: <[EMAIL PROTECTED]>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Status: RO
Content-Length: 12329
Lines: 386
Hello all,
Attached is an omnibus patch (against SVN) that addresses numerous issues
in the source tree, most notably those of portability. I was attempting to
set up Ganglia on a compile farm of various Unices, and fixed each problem
as I encountered it. Problems with the use of Autoconf/Automake are also
addressed, along with some miscellany here and there.
Below follows a walkthrough of the patch...
monitor-core/configure.in:
* Enable --enable-maintainer-mode switch, so that we can use "if
MAINTAINER_MODE" in Makefile.am later on.
* Use --with-gmetad when running "make distcheck", for completeness.
(Ganglia is now fairly close to passing "make distcheck")
* Don't invoke configure scripts directly. The correct way to handle these
is with AC_CONFIG_SUBDIRS(). (I would hardcode the --disable-nls setting
for srclib/confuse)
* All -Dxxx directives belong in CPPFLAGS, not CFLAGS.
* Check for the proper form of the "const" and "inline" keywords. (E.g. the
latter could be "__inline" in some cases)
* Use the standard Autoconf-provided GCC variable to check for gcc.
* Removed the -Wall bit, as it's already added in the aforementioned GCC
conditional.
* Specify -D_POSIX_C_SOURCE=... and -D_BSD_SOURCE feature test macros to
allow Linux ANSI builds to work.
* Always use -D__EXTENSIONS__ on Solaris, otherwise you miss out on a lot
of useful bits. Don't define __STDC__; the proper way is to use
-Xc/-ansi, but strict ANSI on Solaris is a bit much. (I've confirmed that
ganglia builds fine with both cc and gcc, using this setting.)
* Removed what looks like a redundant bit in the FreeBSD case (-pthread and
-D_REENTRANT are used no matter what)
* Output web/Makefile, since we're going to need it.
monitor-core/scripts/svn2cl.sh:
* Use $SVN if set in the environment. Not a big deal, but my default
Subversion install lacked SSL support, so this was a nice way to handle
that.
monitor-core/gmetric/Makefile.am:
* All -Ixxx flags should be assigned to AM_CPPFLAGS, not AM_CFLAGS.
* Use $(top_srcdir) instead of "..", so that out-of-source-tree builds
work.
monitor-core/gmetad/gmetad.c:
* Was missing the all-important include<config.h>.
monitor-core/gmetad/gmetad.h,
monitor-core/gmetad/rrd_helpers.c,
monitor-core/gmetad/conf.c:
* Minor stuff.
monitor-core/gmetad/cleanup.c:
* No need for the lib/ prefix there, as you already have
-I$(top_srcdir)/lib in the makefile
monitor-core/gmetad/Makefile.am:
* -Ixxx directives in AM_CPPFLAGS, not AM_CFLAGS
* Use -I$(top_srcdir)/... instead of -I$(top_builddir)/..., because the
required files are not generated.
* Don't specify -O0 in this file. That doesn't belong here.
* Conditionalize the gmetad build using Automake conditionals, instead of
tinkering with SUBDIRS in the parent makefile.
monitor-core/gmond/gstat/Makefile.am:
* Switched the -Ixxx directives to use $(top_srcdir) instead of
$(top_builddir), so that out-of-source-tree builds work
monitor-core/gmond/gmond.c:
* Don't use C++ comments in C code.
* Can't declare a variable-size array, so use good ol' malloc() instead.
Swapped out "return" for "goto cleanup", so that the memory always gets
free()d at the end.
monitor-core/gmond/Makefile.am:
* Fixed up the -Ixxx directives for out-of-source-tree builds.
monitor-core/lib/protocol.x:
* Pull in config.h, as the source code made from this file is
platform-sensitive.
* Get rid of the register keyword, as it's pretty much useless nowadays
(and unavailable in ANSI mode)
monitor-core/lib/Makefile.am:
* Fixed up the include directives for out-of-source-tree builds.
* You can't use $< in a non-implicit rule. GNU Make allows it, but most
other make(1) programs don't.
* Specify libganglia's inter-library dependencies via libganglia_la_LIBADD.
This way, you can just link against -lganglia, and not have to worry
about the rest.
monitor-core/acinclude.m4:
* When you use AC_DEFUN(), you have to quote the name of the new macro or
else aclocal(1) complains.
monitor-core/Makefile.am:
* Do not conditionalize the value of SUBDIRS. This is messy and just asking
for trouble.
* Added the "web" subdirectory to SUBDIRS, as that is now handled in the
standard way.
* Specify EXTRA_DIST explicitly---don't say "mans", say "mans/gmetad.1 ..."
* Surrounded the Changelog double-colon rule with "if MAINTAINER_MODE", so
that non-GNU Makes are unlikely to see it.
* The dist-hook and distclean-local bits are no longer needed. Will get
back to this further down...
monitor-core/ganglia-config.in:
* Put in a dummy substitution for @datarootdir@, just to shut up
config.status when it runs.
monitor-core/web/Makefile.am:
* Distribute the files here using EXTRA_DIST. A bit more verbose, a lot
less trouble.
* Trimmed the width of the "install" target message a bit. (The ==== lines
were 81 characters long... not very neat-looking)
* Chucked the dist-hook rule. (Not even updated since ganglia lived in
CVS...)
monitor-core/bootstrap:
* Don't cd all over the place; just do "(cd subdir && whatever)". Much
easier to keep track of things that way.
* Don't put everything in a giant "&&" chain; just use "set -e".
monitor-core/srclib/expat/lib/Makefile.in:
* Don't pass -rpath to libtool when linking the library. This informs
Libtool that the shared library won't be installed, and so when
libganglia links to it, it'll actually pull in the component object files
of libexpat instead of making reference to the library itself. This is
necessary because libexpat is never actually installed.
monitor-core/srclib/expat/lib/expat.h:
*** This file should not be in SVN ***, as it is produced from expat.h.in
by config.status.
monitor-core/srclib/apr/configure.in:
* Don't pass -rpath to libtool.
monitor-core/srclib/confuse/m4/check.m4:
* More AC_DEFUN() argument quoting.
monitor-core/srclib/confuse/src/lexer.c:
* Not a good idea to have this file in SVN.
monitor-core/srclib/confuse/src/confuse.c:
* Need locale.h to build on... damn, forgot which Unix this was on now....
monitor-core/srclib/confuse/src/Makefile.am:
* Specify libconfuse.la as noinst. This is the clean way of preventing
-rpath from being passed to libtool.
* Variable/rule tweaks to work with Automake's built-in lex know-how.
monitor-core/srclib/confuse/src/lexer.l:
* YY_PROTO() is not defined, and since K&R compilers probably don't need to
be supported, good riddance.
monitor-core/srclib/confuse/configure.ac:
* Removed AC_DISABLE_SHARED, as this gets in the way of building a shared
libganglia. (Fails when libtool tries to pull in a static libconfuse.a
into a shared libganglia.so, although not on Linux/x86)
monitor-core/srclib/confuse/doc/listing5.c,
monitor-core/srclib/confuse/doc/listing8.c,
monitor-core/srclib/confuse/examples/ftpconf.c,
monitor-core/srclib/confuse/examples/reread.c,
monitor-core/srclib/confuse/examples/simple.c:
* I think it was Tru64 that didn't allow initializing an array of structs
unless the array was declared static. Sad, eh?
(I don't remember now which system needed the changes made to simple.c...
I'll need to double-check that.)
monitor-core/srclib/confuse/doc/html/*:
* These files really shouldn't be in SVN.
monitor-core/srclib/confuse/doc/Makefile.am:
* Don't just "cp -pr" these directories, because that potentially copies
over SVN version data and other random crud into the tarball.
monitor-core/srclib/confuse/Makefile.am:
* Is there any need for this dist-hook?
monitor-core/srclib/Makefile.am:
* Instead of using a dist-hook target to take care of putting apr and expat
into the tarball, just use a big EXTRA_DIST. It works fine. libmetrics
and confuse can be handled by the DIST_SUBDIRS mechanism.
* Defined some additional dummy targets that should not recurse. Prefixed
the echo(1) commands with @ for neatness.
monitor-core/srclib/libmetrics/libmetrics.h:
* Some general reorganization, so that things occur in the approximate
order that they usually do.
* The whole rpl_malloc hack is a bad idea. (Anything that re-#defines a
C library function is _evil_.) Instead, define lm_malloc() and
lm_realloc(), and have the metrics code use them exclusively. If we're
using glibc, then the two functions are straight aliases for malloc() and
realloc(). If not, then they go to wrapper functions that yield the
desired "GNU-compatible" behavior.
monitor-core/srclib/libmetrics/tests/test-metrics.c:
* You need _something_ for that third field....
monitor-core/srclib/libmetrics/tests/Makefile.am:
* Added -I$(top_srcdir) for libmetrics.h
* Removed the dependency on libganglia
monitor-core/srclib/libmetrics/freebsd/metrics.c:
* Tweaks to allow building on FreeBSD 4.0
* A slightly less awkward way of avoiding the conflicting "struct pmap"
definitions
* Use lm_malloc() instead of malloc()
monitor-core/srclib/libmetrics/solaris/metrics.c:
* No C++ comments in C code!
* Use lm_malloc() instead of malloc()
monitor-core/srclib/libmetrics/hpux/metrics.c:
* Give proper command line, with CPPFLAGS
* Fixed the signatures of cpu_aidle_func() and swap_free_func()
monitor-core/srclib/libmetrics/configure.in:
* Don't use AC_FUNC_MALLOC or AC_FUNC_REALLOC, as these do the evil
re-#define-itions.
* Use -D_BSD_SOURCE on Linux, as some bits need it when building in ANSI
mode.
* Use CPPFLAGS instead of CFLAGS for -Dxxx directives.
* Same deal with using one set of Solaris flags instead of two.
monitor-core/srclib/libmetrics/osf/metrics.c:
* No C++ comments in C code!
* Straightened out a string with an embedded newline. (Trimmed the trailing
whitespace too)
* My system (Digital UNIX V4.0G) doesn't have a "mhz" field in struct
tbl_processor. You'll need to write the test that sets HAVE_CPUINFO_MHZ
when appropriate.
monitor-core/srclib/libmetrics/stamp-h1:
*** This file should not be in SVN ***
monitor-core/srclib/libmetrics/linux/metrics.c:
* Use lm_malloc(), fix C++ comments
monitor-core/srclib/libmetrics/file.c,
monitor-core/srclib/libmetrics/file.h:
* Imported from libganglia, to break the circular dependency
monitor-core/srclib/libmetrics/cygwin/metrics.c:
* Use lm_malloc()
monitor-core/srclib/libmetrics/get_ifi_info.c:
* Not all systems have SIOCGIFMTU.
monitor-core/srclib/libmetrics/Makefile.am:
* Reformatted for clarity.
monitor-core/srclib/libmetrics/libmetrics.c:
* Define lm_malloc()/lm_realloc() if needed
Some observations/comments:
* libmetrics is a bit awkwardly placed inside the srclib directory. Why not
move it up to the top-level directory, as a peer to gmond/gmetad/etc.?
Seems a bit odd to treat it as a third-party library, when it clearly is
not.
* There are a _lot_ of generated files in SVN (Makefile.in, configure,
config.h.in, Doxygen output et al.)... this is really annoying to work
with. (Spurious M-flagged files, lots of crud to trim from patches, etc.)
* libmetrics/Makefile.am shouldn't be assigning @OS@ to SUBDIRS; Automake
conditionals better solve the problem of which platform-specific library
to build. Felt too lazy to do this myself :]
* The top-level configure script shoud enable the gmetad build with
--enable-gmetad, not --with-gmetad. The --with-* options are
conventionally used for specifying the use of external
libraries/programs, whereas --enable-* typically toggles a self-contained
feature.
* It would be very good to get Ganglia to the point where "make distcheck"
passes, because then it will be straightforward to check that all the
Automake rules are in order. Mainly, it was weirdness with the XML
documentation tools that tripped me up; I'm not familiar with those. At
the very least, "make dist" works better now, and out-of-source-tree
builds are now possible.
I'll be happy to answer any questions concerning these changes. Thanks to
everyone here for this most useful suite!
--Daniel
--
NAME = Daniel Richard G. ## Remember, skunks _\|/_ meef?
EMAIL1 = [EMAIL PROTECTED] ## don't smell bad--- (/o|o\) /
EMAIL2 = [EMAIL PROTECTED] ## it's the people who < (^),>
WWW = http://www.******.org/ ## annoy them that do! / \
--
(****** = site not yet online)
