Bug 429028 - Review Request: upstart - an event-driven init daemon
Review Request: upstart - an event-driven init daemon
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Bill Nottingham
Fedora Extras Quality Assurance
:
Depends On: 429416 upstart
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-16 16:30 EST by Casey Dahlin
Modified: 2014-06-18 04:46 EDT (History)
14 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-07 00:27:51 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
notting: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
patch for gcc-4.3 (347 bytes, patch)
2008-02-01 13:36 EST, Bill Nottingham
no flags Details | Diff

  None (edit)
Description Casey Dahlin 2008-01-16 16:30:42 EST
Spec URL:
http://stevemilner.org/tmp/upstart.spec
http://stevemilner.org/tmp/event-compat-sysv.spec
SRPM URL:
http://stevemilner.org/tmp/upstart-0.3.9-1.fc8.src.rpm
http://stevemilner.org/tmp/event-compat-sysv-0.3.9-1.fc8.src.rpm

Description: Upstart is an event driven init system and service manager. It will allow us to move from preset runlevels starting a certain set of services to on-demand service activation for all services and unified monitoring and managing of both system and session level services.

Listed above are two packages. At the moment these two are essentially inseparable. The upstart package provides the new init system, and the event-compat-sysv package provides compatibility with the current SysV init scripts.
Comment 1 Rahul Sundaram 2008-01-18 07:56:44 EST
You have to submit event-compat-sysv as a separate review and mark it as a
dependency of this review so that they can be reviewed independently.
Comment 2 Casey Dahlin 2008-01-19 11:22:16 EST
(In reply to comment #1)
> You have to submit event-compat-sysv as a separate review and mark it as a
> dependency of this review so that they can be reviewed independently.

I have now done this.
Comment 3 Rudolf Kastl 2008-01-21 02:04:46 EST
package set conflicts with sysvinit. maybe this can be worked around by using
the one of the suffix configure flags and adjustments in the actual job files.

also id recommend to use %find_lang %{name} at the end of the %install section
and %files -f %{name}.lang instead of packaging the translation files manually
(you can remove them from the %files section then).
Comment 4 Casey Dahlin 2008-01-21 10:14:51 EST
This package will conflict with sysvinit, it will not be possible to install
both. I'd been waiting to determine whether a Conflicts or Obsoletes entry was
more appropriate. Also since our init-scripts package still requires several
tools from the sysvinit package, those will either have to be split off of the
sysvinit package or repackaged here (I like the former solution better, and am
still trying to work with Bill Nottingham on it).

Will fix the language issue tonight and re-post.
Comment 5 Patrice Dumas 2008-01-21 11:40:33 EST
It will be much better for testers and reviewers to be able to 
install both packages in parallel. initng can be installed in 
parallel, for example.

I am completly new to upstart, but it is a bit strange that
/sbin/init is in this package. Can't upstart be used by only tweaking 
inittab and using the previous /sbin/init?
Comment 6 Patrice Dumas 2008-01-21 11:56:43 EST
More on the packaging, unless I am missing something, it looks
like nih should be separated upstream, having that library 
bundled with upstart seems wrong. In fact libnih seems
to be a separate project but I haven't found any release.

If the .la aren't specifically useful they shouldn't be shipped.

Static libraries should go in -static subpackages, or not be 
shipped at all.

.so files should be in the devel packages.

You have to use %find_lang for the locales packaging (it is covered
in the guidelines), as also said in an above comment.

Also I suggest looking at the rpmlint output.
Comment 7 Casey Dahlin 2008-01-21 12:00:36 EST
(In reply to comment #5)
> It will be much better for testers and reviewers to be able to 
> install both packages in parallel. initng can be installed in 
> parallel, for example.
> 

Initng is packaged as something for more advanced users to experiment with. Its
packaging style is not well suited to being the default out of the box init
system (which this package hopes to be, FESCo will be talking that over on
Thursday).

> I am completly new to upstart, but it is a bit strange that
> /sbin/init is in this package. Can't upstart be used by only tweaking 
> inittab and using the previous /sbin/init?

In a word: No. Upstart's init is a full event-driven service manager that not
only starts services but can monitor them, restart them, and notify the system
of their state.

(In reply to comment #6)
> More on the packaging, unless I am missing something, it looks
> like nih should be separated upstream, having that library 
> bundled with upstart seems wrong. In fact libnih seems
> to be a separate project but I haven't found any release.
> 
> If the .la aren't specifically useful they shouldn't be shipped.
> 
> Static libraries should go in -static subpackages, or not be 
> shipped at all.
> 
> .so files should be in the devel packages.
> 

After further talks with upstream it turns out that neither of the lib packages
need be shipped with upstart. I will probably strike them from the package for now.

> You have to use %find_lang for the locales packaging (it is covered
> in the guidelines), as also said in an above comment.
> 

Working on that....

> Also I suggest looking at the rpmlint output.

Comment 8 Bill Nottingham 2008-01-21 13:11:52 EST
One issue is that in parallel with this, unless something has changed with
upstart, we need to figure out how to get SELinux policy loaded. The simple way
is in initramfs (just a simple matter of code), but it needs done.
Comment 9 Casey Dahlin 2008-01-21 13:24:37 EST
(In reply to comment #8)
> One issue is that in parallel with this, unless something has changed with
> upstart, we need to figure out how to get SELinux policy loaded. The simple way
> is in initramfs (just a simple matter of code), but it needs done.

I'll poke upstream about this. It should be easy to plug something in. Upstart
has proven highly flexible so far (for example, early login is now a one-liner
change).
Comment 10 Bill Nottingham 2008-01-21 13:27:12 EST
Doing it in initramfs means upstart doesn't even need to worry about it; if
upstart's init itself did it, it would need to reexec itself under the policy (ick).
Comment 11 Casey Dahlin 2008-01-21 13:36:56 EST
ahh. probably best to do it the other way then :)
Comment 12 Rudolf Kastl 2008-01-21 13:58:13 EST
well actually:

it is technically possible to package it in the proper way to make it parallel
installable. at the very least by using the alternatives system.

if it is packaged this way (and sysv too) it would be rather easy to also
actually package initng with using alternatives and properly switch between
those in an easy and somewhat clean manner.

actually id see a parallel packaged approach better for something that crucial
as an init system until a tester can be sure it does the job properly and
doesent resolve in an unbootable system because of any potential reason. field
testing will show that actually. just my personal opinion.

Comment 13 Bill Nottingham 2008-01-21 14:24:35 EST
Well, except I doubt we'd ever ship the system init daemon in a final release as
something alternatives-able. So, it's a lot of initial work for little long-term
gain.
Comment 14 Patrice Dumas 2008-01-21 14:30:11 EST
(In reply to comment #7)
> (In reply to comment #5)
> > It will be much better for testers and reviewers to be able to 
> > install both packages in parallel. initng can be installed in 
> > parallel, for example.
> > 
> 
> Initng is packaged as something for more advanced users to experiment with. Its
> packaging style is not well suited to being the default out of the box init
> system 

This seems to be untrue to me. Having initng as the default out
the box init system only requires changing grub command line. Anyway
it isn't the subject of this submission.

Using alternatives seems right to me since it would allow to keep
the old init system around after upstart has become the default system.

(In reply to comment #13)
> Well, except I doubt we'd ever ship the system init daemon in a final release as
> something alternatives-able. So, it's a lot of initial work for little long-term
> gain.

Why not? This would allow users to chose the old init system if they
happen to need it?
Comment 15 Bill Nottingham 2008-01-21 14:34:42 EST
If we need more than one init system, we've failed.

(See also: "not about choice", (c) 2008 ajax.)
Comment 16 Patrice Dumas 2008-01-21 14:46:27 EST
(In reply to comment #15)
> If we need more than one init system, we've failed.

Fedora as such shouldn't need it but we cannot discard the
possibility that somebody steps up to keep on maintaining the 
old system. In that case the alternative would still be useful.
It could also be used by initng, by the way, or the next init
system, with early planning and testing.

> (See also: "not about choice", (c) 2008 ajax.)

I disagreed in the thread, I won't agree now...
Comment 17 Casey Dahlin 2008-01-21 15:12:33 EST
(In reply to comment #16)
> (In reply to comment #15)
> > If we need more than one init system, we've failed.
> 
> Fedora as such shouldn't need it but we cannot discard the
> possibility that somebody steps up to keep on maintaining the 
> old system. In that case the alternative would still be useful.
> It could also be used by initng, by the way, or the next init
> system, with early planning and testing.
> 

This defeats the whole purpose of the package.

While this release is intended to simply swap in upstart in an unobtrusive
manner, taking full advantage of Upstart's power is going to involve many
changes to our current system. While packages wishing to install LSB/SysV
scripts will still work, there will eventually come a day when none of our
scripts use these mechanisms (i.e. when a default fedora install has an empty
/etc/init.d). This package provides compatibility with sysvinit, but an
alternative it is not. Upstart is going to allow us to build a very powerful and
robust init system, but it will be one which init-ng and sysv will likely not be
capable of driving.

By all means judge this package with the weight it demands. This is a very real
and very permanent change to a very central part of Fedora.

However, all of this gets off topic. The purpose of this bugzilla is to review
the practice of packaging upstart, not the principle. That we can save for Thursday.

> > (See also: "not about choice", (c) 2008 ajax.)
> 
> I disagreed in the thread, I won't agree now...

Comment 18 Rudolf Kastl 2008-01-21 15:51:04 EST
(In reply to comment #17)
> (In reply to comment #16)
> > (In reply to comment #15)
> > > If we need more than one init system, we've failed.

is the same true for mailservers? :=)

> > 
> > Fedora as such shouldn't need it but we cannot discard the
> > possibility that somebody steps up to keep on maintaining the 
> > old system. In that case the alternative would still be useful.
> > It could also be used by initng, by the way, or the next init
> > system, with early planning and testing.
> > 
> 
> This defeats the whole purpose of the package.

no it doesent because technically the issue is similar to postfix and sendmail.
postfix was also designed as a drop in replacement and properly packaged it
still doesent conflict.

> 
> While this release is intended to simply swap in upstart in an unobtrusive
> manner, taking full advantage of Upstart's power is going to involve many
> changes to our current system. While packages wishing to install LSB/SysV
> scripts will still work, there will eventually come a day when none of our
> scripts use these mechanisms (i.e. when a default fedora install has an empty
> /etc/init.d). This package provides compatibility with sysvinit, but an
> alternative it is not. Upstart is going to allow us to build a very powerful and
> robust init system, but it will be one which init-ng and sysv will likely not be
> capable of driving.

vice versa upstart cant use initng scripts so i am not sure if this is a real
technical argument.

> 
> By all means judge this package with the weight it demands. This is a very real
> and very permanent change to a very central part of Fedora.

I was trying to do that above because replaceing a mailserver couldnt make a
system potentially break it to an extent that it doesent boot anymore. 

> 
> However, all of this gets off topic. The purpose of this bugzilla is to review
> the practice of packaging upstart, not the principle. That we can save for
Thursday.

Agreed. Personally i consider a package that conflicts as a broken package by
default unless it really  Obsoletes: the older version for good ;). I am not
sure if completly obsoleting is the way to go.

> 
> > > (See also: "not about choice", (c) 2008 ajax.)
> > 
> > I disagreed in the thread, I won't agree now...
> 
> 

Comment 19 Michel Alexandre Salim 2008-01-24 12:09:41 EST
(In reply to comment #16)
> (In reply to comment #15)
> > If we need more than one init system, we've failed.
> 
> Fedora as such shouldn't need it but we cannot discard the
> possibility that somebody steps up to keep on maintaining the 
> old system. In that case the alternative would still be useful.
> It could also be used by initng, by the way, or the next init
> system, with early planning and testing.
> 
Um. Seeing as compatibility with SysInitV will get worse over time (as services'
init scripts get modified to integrate more tightly with Upstart), I'd say at
some point going back will not be possible anyway.
Comment 20 Casey Dahlin 2008-01-29 11:22:08 EST
New spec file here:

http://www4.ncsu.edu/~cjdahlin/upstart.spec

New srpm here:

http://www4.ncsu.edu/~cjdahlin/upstart-0.3.9-2.fc8.src.rpm
Comment 21 Bill Nottingham 2008-01-31 15:08:44 EST
MUST:
 - Package meets naming guidelines - OK
 - Spec file matches base package name. - OK
 - Spec has consistant macro usage. - ***
 - Meets Packaging Guidelines. - OK
 - License - GPLv2+ - OK
 - License field in spec matches - OK
 - License file included in package - ***
 - Spec in American English - OK
 - Spec is legible. - OK
 - Sources match upstream md5sum:
794208083d405ece123ad59a02f3e233 - OK
 - Package needs ExcludeArch - N/A
 - BuildRequires correct - OK
 - Spec handles locales/find_lang - OK
 - Package has %defattr and permissions on files is good. - OK
 - Package has a correct %clean section. - OK
 - Package has correct buildroot - OK
 - Package is code or permissible content. - OK
 - Doc subpackage needed/used. - N/A
 - Packages %doc files don't affect runtime. - OK, once they're added

 - Headers/static libs in -devel subpackage. - N/A, nuked
 - Spec has needed ldconfig in post and postun - OK
 - .pc files in -devel subpackage/requires pkgconfig - N/A
 - .la files are removed. - OK

 - Package compiles and builds on at least one arch. - once issue #1 addressed,
yes (tested x86_64)
 - Package has no duplicate files in %files. - OK
 - Package doesn't own any directories other packages own. - OK
 - Package owns all the directories it creates. - OK
 - No rpmlint output.
upstart.x86_64: W: conffile-without-noreplace-flag /etc/event.d/logd

This may be OK, if this isn't meant to be user modified.

 - final provides and requires are sane: ***

SHOULD Items:

 - Should build in mock. - OK (tested x86_64)
 - Should build on all supported archs - didn't test ppc
 - Should function as described. - ***
 - Should have sane scriptlets. - N/A
 - Should have dist tag - OK
 - Should package latest version - OK

Issues:
1) %configure --enable-compat=sysv --sbindir=/sbin --libdir=/lib

Needs to be --libdir=/%{_lib}, otherwise build will fail on x86_64, ppc64.

2) No docs, including licenses

Probably should include ChangeLog, AUTHORS, COPYING, NEWS, README, TODO, HACKING

3) Should this package require event-compat-sysv? Should it obsolete/provide
sysvinit?

4) Re: functionality. I think that will go in a separate bug.
Comment 22 Bill Nottingham 2008-01-31 15:13:14 EST
5) The runlevel man page ends up screwed up somehow.
Comment 23 Casey Dahlin 2008-01-31 15:49:50 EST
1) Will fix soon.

2) Ditto.

3) event-compat-sysv provides sysvinit. It requires upstart.

4) Broken in part due to the lack of a sysvinit-utils package. Still working
that out.

5) I'll look into this.
Comment 24 Bill Nottingham 2008-01-31 15:58:03 EST
The problem with #3 is that something still needs to obsolete sysvinit to a)
force the upgrade b) get sysvinit off the system.
Comment 25 Casey Dahlin 2008-01-31 16:01:56 EST
That would be event-compat-sysv :)

It seems strange, but in effect, it is that collection of event files which
enables sysv compatibility, not upstart.
Comment 27 Bill Nottingham 2008-02-01 13:29:43 EST
For #4, there is a patch in bug 431110 for sysvinit, if you want it for testing.
I suppose at some point those should move to util-linux or similar, but that's
beyond the scope of this. See also the 'upstart' meta-bug tracker.

Changes added address issues #1, #2, above. #5 is a run-of-the-mill bug, not
relevant to packaging.

Do you need a sponsor?
Comment 28 Bill Nottingham 2008-02-01 13:36:32 EST
Created attachment 293754 [details]
patch for gcc-4.3

This is needed to build with gcc-4.3 - a missing #include.
Comment 29 Casey Dahlin 2008-02-01 13:56:48 EST
Yes, I do need a sponsor.

Also, 5 Works For Me, so not sure what's going wrong.
Comment 30 Jakub 'Livio' Rusinek 2008-02-06 11:11:25 EST
Shouldn't every non-versioned .so library land in -devel?

Ie. %{_libdir}/libupstart.so

And, other, %find_lang should be probably used.
Comment 31 Casey Dahlin 2008-02-06 11:27:32 EST
(In reply to comment #30)
> Shouldn't every non-versioned .so library land in -devel?
> 
> Ie. %{_libdir}/libupstart.so
> 
> And, other, %find_lang should be probably used.

Are you looking at the latest package? Your concerns seem out of date.
Comment 32 Tom "spot" Callaway 2008-02-06 11:39:48 EST
I'm not sure if this is approved or not. Whoever did the review should mark the
flag appropriately. If you still need a sponsor after that (you shouldn't, as
both notting and tibbs can sponsor you), I'd be willing to.
Comment 33 Bill Nottingham 2008-02-06 15:26:19 EST
OK, approving and sponsoring; please enter CVS request. I'd like to co-maintain
it for the moment, if you don't mind.

For builds, please build into dist-f9-upstart until we get some of the bugs on
the tracker worked out.
Comment 34 Casey Dahlin 2008-02-06 20:16:48 EST
New Package CVS Request
=======================
Package Name: upstart
Short Description: An event-driven init system
Owners: sadmac,notting
Branches: 
InitialCC: 
Cvsextras Commits: yes
Comment 35 Kevin Fenzi 2008-02-06 21:32:12 EST
cvs done.

Note You need to log in before you can comment on or make changes to this bug.