Bug 429416
Summary: | Review Request: event-compat-sysv - sysVinit compatibility for upstart | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Casey Dahlin <cdahlin> |
Component: | Package Review | Assignee: | Bill Nottingham <notting> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | low | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | 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
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. 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. 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). New package: http://www4.ncsu.edu/~cjdahlin/event-compat-sysv-0.3.9-3.fc8.src.rpm http://www4.ncsu.edu/~cjdahlin/event-compat-sysv.spec (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? 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. Scott says the example scripts are under GPLv2 just like upstart itself. OK, then it would be nice to have a COPYING file in the tarball, at least. 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? Drop upstart's in as Source2: (or whatever), maybe with a copy of the note from Scott until there's a new upstream tarball. 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 Looks OK to me. Again, please build into dist-f9-upstart for now, and I'd like to comaintain. APPROVED. New Package CVS Request ======================= Package Name: event-compat-sysv Short Description: Upstart events to emulate SysVInit Owners: sadmac,notting Branches: InitialCC: Cvsextras Commits: yes cvs done. |