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
