Daniel: I agree with Martin - perhaps breaking the patch up into more manageable pieces would make it easier to integrate - and we sorely need a build system specailist :-)
Thanks for your contributions. Cheers, Bernard -----Original Message----- From: [EMAIL PROTECTED] on behalf of Martin Knoblauch Sent: Tue 15/08/2006 09:22 To: Daniel Richard G.; ganglia-developers@lists.sourceforge.net Subject: Re: [Ganglia-developers] [PATCH] General fixes and cleanup Hi Daniel, seems we got ourselves as build-system specialist :-) I have taken over the bz entry and will work on your patch. It might be that I will ask you to split it up into more independent pieces though. Cheers Martin --- "Daniel Richard G." <[EMAIL PROTECTED]> wrote: > 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) > > ------------------------------------------------------------------------- > Using Tomcat but need to do more? Need to support web services, > security? > Get stuff done quickly with pre-integrated technology to make your > job easier > Download IBM WebSphere Application Server v.1.0.1 based on Apache > Geronimo > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 > _______________________________________________ > Ganglia-developers mailing list > Ganglia-developers@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/ganglia-developers > > ------------------------------------------------------ Martin Knoblauch email: k n o b i AT knobisoft DOT de www: http://www.knobisoft.de ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ Ganglia-developers mailing list Ganglia-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ganglia-developers
