Hi,

Took the chance to review Lukasz's new RPMs.  Here are my items of concern:
- libXrdPss.so is in the server, but depends on libXrdFfs.so.  Hence, xrootd-server is dependent on xrootd-fuse is dependent on fuse.
  - In general, we probably want to remove the dependency on libfuse from libXrdPss.
- We probably want to make sub-RPMs for the different security plugins (ssl, gsi, krb5, etc)
- It's bad form to have multiple sources if the items in the .spec file if they're included in the tarball.  I.e., instead of:
  install -m 755 %{SOURCE2} $RPM_BUILD_ROOT%{_initrddir}/cmsd
You could have done:
  install -m 755 packaging/rhel/cmsd.init $RPM_BUILD_ROOT%{_initrddir}/cmsd
This cuts down on the number of sources and hence the ability for folks to introduce errors.
- It is kind and traditional to include in comments where the sources can be taken from.  I.e., from my Xrootd RPM:
# git clone http://xrootd.org/repo/xrootd.git xrootd
# cd xrootd
# git-archive master | gzip -9 > ~/rpmbuild/SOURCES/xrootd.tgz
Source0:        xrootd.tgz
- I strongly suggest a sample configuration file so the user can do a "yum install xrootd; service xrootd start" and have something working immediately.  I have samples I give folks, but assuredly Andy won't like them because they include GSI security by default :).  Willing to contribute if no one else pipes up.
- I notice you have three xrootd config files ({xrootd,cmsd,frmd}-main.cf) - I thought it was the general style of the project to have one config file?
- Couldn't get the makesrpm.sh script to work.  Failed with:
[brian@brian-test packaging]$ ./makesrpm.sh 
[i] Working on: ../
[i] Stoing the output to: .
[i] Working with version: unknown
[i] RPM compliant version: unknown
[i] Woring in: /tmp/tmp.pEERkq7868
fatal: unrecognized argument: --format=%H
[!] Unable to figure out the git commit hash
Poking around a little, it seems to be due to me not being on an exact tag.  I think instead of:
git log -1 --format='%ai %h'
you want:
git log -1 --pretty=format:'%ai %H'
Fixing this, the version number appears something like this:
20110312.73b5e2f
Could we instead use something like "git describe"'s output?
v3.0.2-90-g73b5e2f
This is something we can more easily mangle into the RPM.
- makesrpm.sh doesn't work because I have local macros set.  I removed your "dirty hack" and, with the patch below, it works.
- I strongly recommend adding an xrootd user rather than having it run as 'daemon'.
- What about the "cnsd" daemon?  No started up script is given for that.
- I'm not sure it's really plausible to have multiple xrootds run in this setup, but I think that's a larger issue 
- You probably want to add at least "Epoch: 1" here in order to match the EPEL packaging.

I have tried to include as many of these comments in a patch below; some require input or decision making from your side.

Overall, a great improvement!  Thanks Lukasz!  With a bit more finessing, I can put my separate RPMs to rest.

Brian


diff --git a/packaging/makesrpm.sh b/packaging/makesrpm.sh
index f62d693..45d4045 100755
--- a/packaging/makesrpm.sh
+++ b/packaging/makesrpm.sh
@@ -128,11 +128,12 @@ echo "[i] RPM compliant version: $VERSION"
 # exit on any error
 set -e
 
-TEMPDIR=`mktemp -d`
+TEMPDIR=`mktemp -d -t tmp.xrootd.srpm.XXXXXXX`
 RPMSOURCES=$TEMPDIR/rpmbuild/SOURCES
 mkdir -p $RPMSOURCES
+mkdir -p $TEMPDIR/rpmbuild/SRPMS
 
-echo "[i] Woring in: $TEMPDIR" 1>&2
+echo "[i] Working in: $TEMPDIR" 1>&2
 
 if test -d rhel -a -r rhel; then
   for i in rhel/*; do
@@ -154,7 +155,7 @@ set +e
 
 CWD=$PWD
 cd $SOURCEPATH
-COMMIT=`git log --format="%H" -1`
+COMMIT=`git log --pretty=format:"%H" -1`
 
 if test $? -ne 0; then
   echo "[!] Unable to figure out the git commit hash" 1>&2
@@ -185,8 +186,8 @@ cat rhel/xrootd.spec.in | sed "s/__VERSION__/$VERSION/" > $TEMPDIR/xrootd.s
 echo "[i] Creating the source RPM..."
 
 # Dirty, dirty hack!
-export HOME=$TEMPDIR
-rpmbuild -bs $TEMPDIR/xrootd.spec > $TEMPDIR/log
+echo "%_topdir $TEMPDIR/rpmbuild" > $TEMPDIR/rpmmacros
+rpmbuild --define "_topdir $TEMPDIR/rpmbuild" -bs $TEMPDIR/xrootd.spec > $TEMPDIR/log
 if test $? -ne 0; then
   echo "[!] RPM creation failed" 1>&2
   exit 8


diff --git a/packaging/rhel/xrootd.spec.in b/packaging/rhel/xrootd.spec.in
index 7c8d2c6..89e24bc 100644
--- a/packaging/rhel/xrootd.spec.in
+++ b/packaging/rhel/xrootd.spec.in
@@ -5,16 +5,10 @@ Summary:   An eXtended Root Daemon (xrootd)
 Group:     System Environment/Daemons
 License:   Stanford (modified BSD with advert clause)
 URL:       http://xrootd.org/
+# git clone http://xrootd.org/repo/xrootd.git xrootd
+# cd xrootd
+# git-archive master | gzip -9 > ~/rpmbuild/SOURCES/xrootd.tgz
 Source0:   xrootd.tar.gz
-Source1:   xrootd.sysconfig
-Source2:   cmsd.init
-Source3:   frm_purged.init
-Source4:   frm_xfrd.init
-Source5:   xrootd.init
-Source6:   xrootd.functions
-Source7:   xrootd-main.cf
-Source8:   cmsd-main.cf
-Source9:   frmd-main.cf
 BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
 
 BuildRequires: autoconf automake libtool readline-devel openssl-devel fuse-devel libxml2-devel
@@ -89,19 +83,26 @@ mkdir -p $RPM_BUILD_ROOT%{_var}/run/%{name}
 # init stuff
 mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig
 mkdir -p $RPM_BUILD_ROOT%{_initrddir}
-install -m 644 %{SOURCE1} $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig/%{name}
-install -m 755 %{SOURCE2} $RPM_BUILD_ROOT%{_initrddir}/cmsd
-install -m 755 %{SOURCE3} $RPM_BUILD_ROOT%{_initrddir}/frm_purged
-install -m 755 %{SOURCE4} $RPM_BUILD_ROOT%{_initrddir}/frm_xfrd
-install -m 755 %{SOURCE5} $RPM_BUILD_ROOT%{_initrddir}/xrootd
-install -m 755 %{SOURCE6} $RPM_BUILD_ROOT%{_initrddir}/xrootd.functions
-install -m 644 %{SOURCE7} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/xrootd-main.cf
-install -m 644 %{SOURCE8} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/cmsd-main.cf
-install -m 644 %{SOURCE9} $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/frmd-main.cf
+install -m 644 packaging/rhel/xrootd.sysconfig $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig/%{name}
+install -m 755 packaging/rhel/cmsd.init $RPM_BUILD_ROOT%{_initrddir}/cmsd
+install -m 755 packaging/rhel/frm_purged.init $RPM_BUILD_ROOT%{_initrddir}/frm_purged
+install -m 755 packaging/rhel/frm_xfrd.init $RPM_BUILD_ROOT%{_initrddir}/frm_xfrd
+install -m 755 packaging/rhel/xrootd.init $RPM_BUILD_ROOT%{_initrddir}/xrootd
+install -m 755 packaging/rhel/xrootd.functions $RPM_BUILD_ROOT%{_initrddir}/xrootd.functions
+install -m 644 packaging/common/xrootd-main.cf $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/xrootd-ma
+install -m 644 packaging/common/cmsd-main.cf $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/cmsd-main.c
+install -m 644 packaging/common/frmd-main.cf $RPM_BUILD_ROOT%{_sysconfdir}/%{name}/frmd-main.c
 
 %clean
 rm -rf $RPM_BUILD_ROOT
 
+%pre
+getent group xrootd >/dev/null || groupadd -r xrootd
+getent passwd xrootd >/dev/null || \
+       useradd -r -g xrootd -c "Xrootd runtime user" \
+       -s /sbin/nologin -d /etc/xrootd xrootd
+exit 0
+
 %post
 /sbin/chkconfig --add xrootd
 /sbin/chkconfig --add cmsd