Spec URL: http://sundaram.fedorapeople.org/packages/systemd.spec SRPM URL: http://sundaram.fedorapeople.org/packages/systemd-0-0.0.20100602git.src.rpm Description: Systemd is a system and session manager compatible with SysV init and LSB init script headers. Systemd has aggressive parallelization capabilities, uses D-Bus activation for starting services and keeps track of processes using cgroups. -- This will only build on rawhide due to the build requirements, in particular udev needs to be a higher version than what is in F-13. I am filing this review request on behalf of Lennart since he isn't keen on packaging. I have confirmed that it is ok by him. I will add him as the primary maintainer once the review process is over.
(In reply to comment #0) > Spec URL: http://sundaram.fedorapeople.org/packages/systemd.spec > SRPM URL: > http://sundaram.fedorapeople.org/packages/systemd-0-0.0.20100602git.src.rpm > > Description: > Systemd is a system and session manager compatible with SysV init and LSB init > script headers. Systemd has aggressive parallelization capabilities, uses D-Bus > activation for starting services and keeps track of processes using cgroups. > > -- > > This will only build on rawhide due to the build requirements, in particular > udev needs to be a higher version than what is in F-13. I am filing this review > request on behalf of Lennart since he isn't keen on packaging. I have > confirmed that it is ok by him. I will add him as the primary maintainer once > the review process is over. OK here are some initial comments: 1) Does not build in rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=2221974 2) No %clean 3) Missing instructions on how the tarball was generated 4) Please add an abbreviated git commit id to the release (date is not unique) 5) "%{_mandir}/man?/*.[0-9]* " no need for using fancy regex here 6) rpmlint output: ------ systemd.src: W: spelling-error %description -l en_US init -> unit, int, nit systemd.src: W: spelling-error %description -l en_US parallelization -> parallelism, parallelogram, channelization systemd.src: W: spelling-error %description -l en_US cgroups -> groups, c groups, Citigroup systemd.src:16: W: macro-in-comment %{name} systemd.src:16: W: macro-in-comment %{version} systemd.src:49: E: hardcoded-library-path in /lib/systemd/ systemd.src: W: no-cleaning-of-buildroot %install systemd.src: W: no-cleaning-of-buildroot %clean systemd.src: W: no-buildroot-tag systemd.src: W: no-%clean-section systemd.src: W: invalid-url Source0: systemd-2010-06-02.tar.xz ------- Can be mostly ignored. No full review possible due to build failure. I will do a proper review once you fix the noted issues and the build.
Is there any reason to use --libexecdir=/lib/systemd instead of {_prefix}/libexecdir in this package? libexecdir=/lib/%{name} looks like the default libexec path in debian/ubuntu. http://fedoraproject.org/wiki/PackagingGuidelines#Libexecdir
(In reply to comment #1) > 2) No %clean Writing an explicit %clean section is not necessary anymore since RPM 4.8 because a sane default one is now added (http://rpm.org/ticket/81).
(In reply to comment #3) > (In reply to comment #1) > > 2) No %clean > > Writing an explicit %clean section is not necessary anymore since RPM 4.8 > because a sane default one is now added (http://rpm.org/ticket/81). Ah, OK and as it is a rawhide only package, it should be fine ... so scratch that from the list.
The man pages are being built from docbook xml files on the fly. I tried adding libxslt as a BR but it still fails trying to grab xsl file from the net. Seems I am still missing some other dependency but not sure which one. Suggestions?
You need a dep on "docbook-style-xsl" for the docbook stuff. I have now added --with-rootdir= to deal with splitting up the appropriate stuff between / and /usr. Just pass it to configure with an empty argument to get things right. Might be good to use the "official" package description we now have upstream: http://lists.freedesktop.org/archives/systemd-devel/2010-June/000055.html
Hmm, what's the plan for /cgroup/systemd? This should probably be in the package...
git master now mounts a tmpfs to /cgroup. That means that the /cgroup/systemd dir can be dynamically created at runtime by systemd. However, the /cgroup dir must exist on boot, since / tends to be ro during early boot. That means something or somebody must create /cgroup on package installation. I'd probably just do an mkdir in %post, if that's acceptable.
"mkdir -p -m 0755 /cgroup" in %post seems sufficient to me.
(In reply to comment #9) > "mkdir -p -m 0755 /cgroup" in %post seems sufficient to me. Why can't you do this in your install target? (i.e make install)
(In reply to comment #10) > (In reply to comment #9) > > "mkdir -p -m 0755 /cgroup" in %post seems sufficient to me. > > Why can't you do this in your install target? (i.e make install) Or atleast in %install ... don't see a reason why this has to be done in %post unless you don't systemd to own it. In that case adding it to the filesystem package would be sufficient.
/cgroup should definitely not be owned by systemd. It has uses outside of systemd too.
(In reply to comment #12) > /cgroup should definitely not be owned by systemd. It has uses outside of > systemd too. OK, that makes sense, filed a bug against filesystem to get it added there: https://bugzilla.redhat.com/show_bug.cgi?id=599664
http://sundaram.fedorapeople.org/packages/systemd.spec http://sundaram.fedorapeople.org/packages/systemd-0-0.1.20090609git2f198e.fc13.src.rpm scratch build http://koji.fedoraproject.org/koji/taskinfo?taskID=2241423
Rahul, by putting "--with-rootdir=/usr" the systemd binary gets put in /usr/bin rather than /bin -- I'm pretty sure something as low level as an init system deserves to be in /bin. :-)
I encountered a few errors while trying to build this: 1) It's not 2009 any more (git_date) 2) The x86_64 lib workaround didn't seem to fly, I workaround it by (re)defining _libdir to /lib unconditionally 3) /etc/init.d didn't exist so it bombed in %install 4) Redefining _bindir to /bin and clearing the --with-rootdir= part managed to get the binaries in the right place I'll attach a diff of the spec that I got working
Created attachment 422929 [details] Patch to get the package to build on a standard F13 x86_64 system
I just tried the scratch build mentioned in comment 14 by installing the systemd RPM on my rawhide box and then naïvely rebooting and adding "init=/usr/bin/systemd" to the kernel command line. Right after mounting the cgroup file systems it looked like systemd segfaulted and then I got a kernel panic. Is there some other magic that I need to do to try out systemd or am I encountering an actual bug. What's the best way to debug this problem other than posting screenshots of the kernel panic?
Thanks David Hollis http://sundaram.fedorapeople.org/packages/systemd.spec http://sundaram.fedorapeople.org/packages/systemd-0-0.1.20100610git2f198e.fc13.src.rpm
(In reply to comment #18) > I just tried the scratch build mentioned in comment 14 by installing the > systemd RPM on my rawhide box and then naïvely rebooting and adding > "init=/usr/bin/systemd" to the kernel command line. Right after mounting the > cgroup file systems it looked like systemd segfaulted and then I got a kernel > panic. Is there some other magic that I need to do to try out systemd or am I > encountering an actual bug. What's the best way to debug this problem other > than posting screenshots of the kernel panic? According to Lennart a recent commit "broke everything" ... so it is a known issue that will hopefully resolved soon. (In reply to comment #19) > Thanks David Hollis > > http://sundaram.fedorapeople.org/packages/systemd.spec > http://sundaram.fedorapeople.org/packages/systemd-0-0.1.20100610git2f198e.fc13.src.rpm ---- %post mkdir -p -m 0755 /cgroup ---- This isn't needed as the directory is created and owned by libcgroup. ----- %global git_date 20100610 %global git_version 2f198e %define _bindir /bin %define _libdir /lib ----- Use %global for the later two too. rpmlint: ------ systemd.src:21: W: macro-in-comment %{name} systemd.src:21: W: macro-in-comment %{git_version} systemd.src:21: W: macro-in-comment %{version} systemd.src:21: W: macro-in-comment %{git_date} systemd.src:21: W: macro-in-comment %{git_version} systemd.src:24: W: macro-in-comment %{name} systemd.src:24: W: macro-in-comment %{version} ------- Even though none of those will cause any issues; avoid any macros in comments (multiline macros can cause all sort of weirdness). --- systemd.src:6: W: mixed-use-of-spaces-and-tabs (spaces: line 6, tab: line 3) --- Either stick to spaces or tabs but don't use both. Other than that it looks good; it builds fine there is still still some other noise from rpmlint but it can be ignored. Please fix the issues noted above; once you done that there shouldn't be much blocking approval.
I haven't removed the macros from comments since I don't want to be hardcoding the values and adjusting them every bump. Just silenced it. Should be a temporary thing till the point we switch over to releases. http://sundaram.fedorapeople.org/packages/systemd.spec http://sundaram.fedorapeople.org/packages/systemd-0-0.3.20100610git2f198e.fc13.src.rpm
(In reply to comment #21) > I haven't removed the macros from comments since I don't want to be hardcoding > the values and adjusting them every bump. Just silenced it. Should be a > temporary thing till the point we switch over to releases. Yeah got that; another option would be to just escape them (%% instead of %). > http://sundaram.fedorapeople.org/packages/systemd.spec > http://sundaram.fedorapeople.org/packages/systemd-0-0.3.20100610git2f198e.fc13.src.rpm Looks good, I have not actually tried to run it as it is known broken but the spec is clean now and it builds fine. => Approved.
(In reply to comment #20) > (In reply to comment #18) > > Right after mounting the > > cgroup file systems it looked like systemd segfaulted and then I got a kernel > > panic. > > According to Lennart a recent commit "broke everything" ... so it is a known > issue that will hopefully resolved soon. I looked on the systemd-devel list and didn't see any mention... Is there a specific commit that can be reverted easily or is there a deeper problem?
(In reply to comment #23) > (In reply to comment #20) > > (In reply to comment #18) > > > Right after mounting the > > > cgroup file systems it looked like systemd segfaulted and then I got a kernel > > > panic. > > > > According to Lennart a recent commit "broke everything" ... so it is a known > > issue that will hopefully resolved soon. > > I looked on the systemd-devel list and didn't see any mention... Is there a > specific commit that can be reverted easily or is there a deeper problem? He said than on IRC ... no idea have not actually looked at it. I'd expect it to be fixed soon though.
Thanks everyone. Appreciate the help New Package CVS Request ======================= Package Name: systemd Short Description: A System and Session Manager Owners: lennart sundaram Branches: InitialCC:
CVS done (by process-cvs-requests.py).
Some trival issues that can be fixed after the feature is approved(thus upstart should be obsoleted) 1.This package install 64bit elf files to /lib. /lib/systemd/systemd-cgroups-agent /lib/systemd/systemd-initctl /lib/systemd/systemd-logger 2.I suggest not to override system rpm macros in spec, using /bin,/lib or /%{_lib} in %file directly to keep consistency with others core packages(e.g. glibc iptables) will be better. 3.%configure --sbindir=/sbin --libexecdir=%{_prefix}/libexec --with-rootdir= --with-distro=fedora CFLAGS="%{optflags}" --libexecdir=%{_prefix}/libexec and CFLAGS="%{optflags}" seems not needed. See rpm --eval %configure
(In reply to comment #27) > Some trival issues that can be fixed after the feature is approved(thus upstart > should be obsoleted) > > 1.This package install 64bit elf files to /lib. > /lib/systemd/systemd-cgroups-agent > /lib/systemd/systemd-initctl > /lib/systemd/systemd-logger This is intended that way. It's like udev's utility binaries in /lib/udev/ Rahul, thanks a lot for doing this work. Very much appreciated!
For those following along, a feature proposal at https://fedoraproject.org/wiki/Features/systemd