Bug 429416

Summary: Review Request: event-compat-sysv - sysVinit compatibility for upstart
Product: [Fedora] Fedora Reporter: Casey Dahlin <cdahlin>
Component: Package ReviewAssignee: Bill Nottingham <notting>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: low Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, gnomeuser, ma, notting, rvokal, vanhoof
Target Milestone: ---Flags: notting: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-02-07 22:38:48 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 429028    

Description Casey Dahlin 2008-01-19 16:21:11 UTC
Spec URL: http://stevemilner.org/tmp/event-compat-sysv.spec
SRPM URL: http://stevemilner.org/tmp/event-compat-sysv-0.3.9-1.fc8.src.rpm

event-compat-sysv is a package that provides sysvinit compatibility for the
upstart service manager in Fedora. It will be an important component as we
transition from a standard set of sysv init scripts to an event driven init system.

Comment 1 Bill Nottingham 2008-01-31 20:42:00 UTC
MUST items:
 - Package meets naming guidelines - OK
 - Spec file matches base package name. - ***
 - Spec has consistant macro usage.  - OK
 - Meets Packaging Guidelines. - ***
 - License - ***
 - License field in spec matches - ***
 - License file included in package - ***
 - Spec in American English - OK
 - Spec is legible. - OK
 - Sources match upstream md5sum:
22d66ef8bc9d167eb822bbfecb584107 - OK
 - Package needs ExcludeArch - see note about buildarch below
 - BuildRequires correct - OK
 - Package has %defattr and permissions on files is good. - OK
 - Package has a correct %clean section. - OK
 - Package has correct buildroot - OK
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 - Package is code or permissible content. - OK
 - Doc subpackage needed/used. - N/A
 - Package compiles and builds on at least one arch. - OK
 - 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. ***
 - final provides and requires are sane: ***

SHOULD Items:
 - Should build in mock. - OK (tested on x86_64)
 - Should build on all supported archs - it's noarch
 - Should function as described. ****
 - Should have dist tag - OK
 - Should package latest version - OK

Issues:

1) Doesn't match upstream tarball; then again, 'example-jobs' is a pretty
awful name, so this should probably be ignored. Then again, this maybe
should move to initscripts at some point?

2) Package should have 'BuildArchitecture: noarch' and be built as a noarch
package - there's nothing arch specific about it, AFAICT

3) There's no license information in the source at all. Probably should be
cleaned up somewhere.

4) Explicit Provides for SysVinit seems wrong, especially w/o an obsolete. Need
to figure out whether the obsolete goes here or in upstart.

5) More rpmlint:
event-compat-sysv.noarch: W: non-conffile-in-etc /etc/event.d/rc2
event-compat-sysv.noarch: W: non-conffile-in-etc /etc/event.d/rc3
event-compat-sysv.noarch: W: non-conffile-in-etc /etc/event.d/rc0
...

Possibly should be marked %config(noreplace), as the admin may want to customize
these. Note: %config(noreplace) makes upgrading *very* hard in the case of event
file format changes.

6) See bug 431120.

Comment 2 Bill Nottingham 2008-01-31 20:42:40 UTC
More rpmlint:
event-compat-sysv.noarch: W: incoherent-version-in-changelog 1 0.3.9-1.fc9

Remove the spaces around the '-' in the version-release should fix this.

Comment 3 Casey Dahlin 2008-01-31 20:59:49 UTC
1) I don't think it belongs in initscripts. If anything it could merge into
upstart but I chose not to do this since technically a sysadmin could opt to
write his own events from scratch and not install any event scripts at all
(you'd have to assume he enjoyed pain, but its possible).

2) Whoops! I'll fix that.

3) Given the haphazard packaging, I wouldn't be surprised if it was public
domain, and the contents of these is so light I'm not certain its even
copyrightable material (IANAL), but I'll ask upstream.

4) It goes here, IMHO. The logic behind this is that it is in fact these scripts
which create the sysv-compatible environment, not upstart itself.

5) Event files are kind of a special case. I followed the guidelines for init
scripts, but we might need to add something to packaging procedure about how
these really should be done.

6) Already have a fix. Coming soon (see bug).

Comment 5 Bill Nottingham 2008-02-01 18:33:35 UTC
(In reply to comment #3)
> 1) I don't think it belongs in initscripts. If anything it could merge into
> upstart but I chose not to do this since technically a sysadmin could opt to
> write his own events from scratch and not install any event scripts at all
> (you'd have to assume he enjoyed pain, but its possible).

Well, you could do the same for the base scripts for sysvinit as well (inittab,
/etc/rc.sysinit) yet, we shipped those in initscripts.

As to the new version:

- #2 fixed
- Obsoletes need versioned. Otherwise, you're obsoleteing yourself due to the
later Provides:

- #6 is still an issue with the new packages. However, that's more a bug than a
packaging issue.

Licensing is still a review blocker; any response from upstream?


Comment 6 Casey Dahlin 2008-02-01 19:08:04 UTC
I have an email out to Scott, and I'm watching IRC to see if he shows up there
sooner.

The new package should start all ttys now. They still are set to start early though.

Comment 7 Casey Dahlin 2008-02-03 21:36:45 UTC
Scott says the example scripts are under GPLv2 just like upstart itself.

Comment 8 Bill Nottingham 2008-02-04 17:29:19 UTC
OK, then it would be nice to have a COPYING file in the tarball, at least.
 

Comment 9 Casey Dahlin 2008-02-04 17:56:44 UTC
I can drop a copy of upstart's COPYING in the package for now, or do we have to
wait for upstream to add one?

Comment 10 Bill Nottingham 2008-02-04 18:07:40 UTC
Drop upstart's in as Source2: (or whatever), maybe with a copy of the note from
Scott until there's a new upstream tarball.

Comment 11 Casey Dahlin 2008-02-07 06:10:53 UTC
new packages, should fix tty issue as well as add license:

Spec: http://www4.ncsu.edu/~cjdahlin/event-compat-sysv.spec
SRPM: http://www4.ncsu.edu/~cjdahlin/event-compat-sysv-0.3.9-4.fc8.src.rpm

Comment 12 Bill Nottingham 2008-02-07 16:49:25 UTC
Looks OK to me. Again, please build into dist-f9-upstart for now, and I'd like
to comaintain.

APPROVED.

Comment 13 Casey Dahlin 2008-02-07 17:02:37 UTC
New Package CVS Request
=======================
Package Name: event-compat-sysv
Short Description: Upstart events to emulate SysVInit
Owners: sadmac,notting
Branches: 
InitialCC: 
Cvsextras Commits: yes

Comment 14 Kevin Fenzi 2008-02-07 17:14:33 UTC
cvs done.