[Linux-ha-dev] [PATCH] improvements to heartbeat.spec

Thu Aug 17 13:24:46 MDT 2006

On 8/17/06, Lars Marowsky-Bree <lmb at suse.de> wrote:
> I whole heartedly agree, and this is the reason why I have to build SUSE
> packages with a different specfile - because our RPM build process
> expects to start with a specfile and go from there, and this is exactly
> what I do there... (Among some other things.)

Ah! I'd love to look at that... I may have mentioned it earlier, but I
made a brief attempt at installing openSUSE so I could find the
differences - but I think looking at your file would be much  easier.
:)

> But, while we're at the patch itself, I personally like the general
> notion quite a bit:
>
> > --- heartbeat-2.0.7/heartbeat.spec    2006-08-14 00:38:40.000000000 -0400
> > +++ heartbeat.spec    2006-08-16 12:43:20.000000000 -0400
> > @@ -1,9 +1,38 @@
> > -#    $Id: heartbeat.spec.in,v 1.196 2006/08/14 04:22:04 alan Exp $
> > +#    $Id: heartbeat.spec.in,v 1.188 2006/07/13 16:32:18 alan Exp $
>
> ... it might be worth-while to diff against the latest source, though
> ;-)

Ok. Fair enough. :)

> >  %define _unpackaged_files_terminate_build 1
> >  %define _missing_doc_files_terminate_build 1
> > -%define ENABLE_MGMT 1
> > -%define ENABLE_SNMP_SUBAGENT 1
> > +
> > +# Setup defaults
> > +%{!?_with_mgmt: %{!?_without_mgmt: %define _without_mgmt --disable-mgmt}}
> > +%{!?_with_snmp: %{!?_without_snmp: %define _with_snmp --enable-snmp-subagent}}
> > +
> > +%{?_without_snmp: %define _without_snmp --disable-snmp-subagent}
> > +%{?_with_mgmt: %define _with_mgmt --enable-mgmt}
> > +
> > +# use "rpmbuild --with mgmt" or "rpm --define '_with_mgmt 1'" (for RPM 3.x)
> > +# to enable GUI Management Client (disabled by default)
> > +%{?_with_mgmt:%define ENABLE_MGMT 1}
> > +%{!?_with_mgmt:%define ENABLE_MGMT 0}
> > +
> > +# use "rpmbuild --without snmp" or "rpm --define '_without_snmp 1'" (for RPM 3.x)
> > +# to enable SNMP Subagent (enabled by default)
> > +%{?_with_snmp:%define ENABLE_SNMP_SUBAGENT 1}
> > +%{!?_with_snmp:%define ENABLE_SNMP_SUBAGENT 0}
> > +
> > +%if %{ENABLE_SNMP_SUBAGENT}
> > +%define snmp_prereq net-snmp-devel, bzip2-devel, lm_sensors-devel, libselinux-devel
> > +%define snmp_req net-snmp, bzip2, lm_sensors, libselinux
> > +%else
> > +%define snmp_req ,
> > +%define snmp_prereq ,
> > +%endif
> > +
> > +%if %{ENABLE_MGMT}
> > +%define mgmt_req libglade2, python-gtk
> > +%else
> > +%define mgmt_req ,
> > +%endif
>
> My goodness, this is certainly advanced RPM specfile magick! I'm
> impressed. Now, if you can use something similar to hide the (build)
> requirement differences across distributions & versions, you'll be my
> personal hero.

I'll do my best... I like being people's personal heroes.

My final goal is that my instructions for building rpms don't involve
editing spec files.

> I think that list is probably quite long for the snmp-subagent though,
> at least on openSUSE build-requires etc is expanded recursively?

> (BTW, I'll only comment on the lines I'm not sure about, the rest looks
> fine.)
>
> > -BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-build
> > -BuildPrereq: glib-devel, , perl, iputils, /usr/bin/ssh, openssl-devel, libnet
> > +Source: http://linux-ha.org/download/heartbeat-%{version}.tar.gz
> > +BuildRoot: %{_tmppath}/%{name}-%{version}-build
>
> Why did you strip the release number from the build root?

Wow. Because I messed up. Fixed in my source now.

> > @@ -149,7 +179,6 @@
> >        themselves and don't appear to be harmful.  They typically
> >        include a message something like this:
> >        WARN: Ignoring HA message (op=vote) from XXX: not in our membership list
> > -
> >  * Thu Jul 13 2006  Alan Robertson <alanr at unix.sh> (see doc/AUTHORS file)
> >  + Version 2.0.6 - bug fix and a few enhancements release
> >    + Added the ability to start/stop groups from the GUI
>
> BTW, we could take the opportunity to migrate the changes section out to
> a different file using -f.

That would making searching for things in the spec file easier. I'm
game for that.

> > @@ -1326,7 +1355,23 @@
> >  #
> >  #CFLAGS="${RPM_OPT_FLAGS}" \
> >  #  ./configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var
> > -./configure '--prefix=/usr' '--sysconfdir=/etc' '--localstatedir=/var' '--with-group-id=90' '--with-ccmuser-id=90' '--libexecdir=/usr/lib64' '--libdir=/usr/lib64' '--disable-ansi' '--enable-mgmt' '--enable-bundled_ltdl' '--enable-snmp-subagent' 'CFLAGS= -fno-unit-at-a-time' --enable-ltdl-convenience
> > +%if %{ENABLE_MGMT}
> > +config_flags="${config_flags} --enable-mgmt "
> > +%else
> > +config_flags="${config_flags} --disable-mgmt "
> > +%endif
> > +
> > +%if %{ENABLE_SNMP_SUBAGENT}
> > +config_flags="${config_flags} --enable-snmp-subagent "
> > +%else
> > +config_flags="${config_flags} --disable-snmp-subagent "
> > +%endif
>
> We should only do this for diverting from the default, and then we can
> use something like:
>
> %configure ... \
> %if %{ENABLE_MGMT}
>         --enable-mgmt \
> %endif
>         ...
>
> That's slightly less clutter.

I like that. That will probably be more readable than the solution to
that clutter that I was cooking up. :)

> Maybe some of the changes I made for the openSUSE package are
> interesting to you? http://repos.opensuse.org/server:/ha-clustering/

I'm going to look at that this evening. Maybe we can have one magic spec file!

> (Even if you went quite a bit beyond than I did in replacing the file
> section with proper rpm macros, because I obviously only had the Linux
> target in mind.)
>
> > +CFLAGS="${RPM_OPT_FLAGS} -fno-unit-at-a-time" \

> Why hard-code -fno-unit-at-atime here? I don't like that.

I really just moved it. It was there for some reason that I didn't
understand, so I just kept it.

> > +%configure '--with-group-id=90' '--with-ccmuser-id=90' '--disable-ansi' '--enable-bundled_ltdl' '--enable-ltdl-convenience' ${config_flags}
>
> Be careful. The group-id and ccmuser-id are also platform-specific.

Aha! I'll have to do some digging to solve that better.

> > +CFLAGS="${RPM_OPT_FLAGS} -fno-unit-at-a-time" \
> >  make
>
> You could simply export CFLAGS in the build section and be done with it.

But that would make too much sense! *sigh* you're right.

> > @@ -1490,11 +1535,11 @@
> >  else
> >    GROUPOPT="-g 90"
> >    if
> > -    /usr/sbin/groupadd $GROUPOPT haclient 2>/dev/null
> > +    %{_sbindir}/groupadd $GROUPOPT haclient 2>/dev/null
> >    then
> >      : OK we were able to add group haclient
> >    else
> > -    /usr/sbin/groupadd haclient
> > +    %{_sbindir}/groupadd haclient
> >    fi
> >  fi
> >
> > @@ -1503,13 +1548,13 @@
> >  then
> >    : OK user hacluster already present
> >  else
> > -  USEROPT="-g haclient -u 90 -d /var/lib/heartbeat/cores/hacluster"
> > +  USEROPT="-g haclient -u 90 -d %{_localstatedir}/lib/heartbeat/cores/hacluster"
> >    if
> > -    /usr/sbin/useradd $USEROPT hacluster 2>/dev/null
> > +    %{_sbindir}/useradd $USEROPT hacluster 2>/dev/null
> >    then
> >      : OK we were able to add user hacluster
> >    else
> > -    /usr/sbin/useradd hacluster
> > +    %{_sbindir}/useradd hacluster
> >    fi
> >  fi
> >
>
> This section needs to be changed to reflect the uid/gid variables.

Agreed. Are they documented anywhere you know of?

> Overall, I like the idea quite a bit. If we could get one cross-platform
> RPM spec file which worked for all platforms which use rpms, we might be
> able to get rid of the spec.in file.
>
> (I hope that now that this is suggested by someone else the idea will be
> attributed more merit ;-)

I second that. :) I'll work on your suggestions and get a new diff
against the current (CVS head?) source.

Monty