[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #18 from Fedora Update System 2009-08-01 19:58:17 EDT --- olpc-powerd-9-1.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 Fedora Update System changed: What|Removed |Added Status|ASSIGNED|CLOSED Fixed In Version||9-1.fc11 Resolution||ERRATA -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #17 from Fedora Update System 2009-07-31 10:26:23 EDT --- olpc-powerd-9-1.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/olpc-powerd-9-1.fc11 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 Jason Tibbitts changed: What|Removed |Added Flag|fedora-cvs? |fedora-cvs+ --- Comment #16 from Jason Tibbitts 2009-07-24 15:23:13 EDT --- CVS done. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 Paul Fox changed: What|Removed |Added Flag||fedora-cvs? --- Comment #15 from Paul Fox 2009-07-24 11:07:04 EDT --- New Package CVS Request === Package Name: olpc-powerd Short Description: power management for the XO laptop Owners: pgf Branches: F-11 InitialCC: cwickert -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #14 from Christoph Wickert 2009-07-23 13:09:53 EDT --- Practically not, theoretically yes. Do as you like. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #13 from Paul Fox 2009-07-23 10:49:26 EDT --- actually, i'm a little bit confused by the need to add the upstart dependency. powerd itself requires the presence of upstart in order to run. why must the dependency be called out separately for the scriptlets? is it even possible to configure a modern fedora without upstart? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #12 from Paul Fox 2009-07-23 10:47:07 EDT --- i will add the dependency on upstart. many thanks, again. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #11 from Christoph Wickert 2009-07-22 21:44:54 EDT --- Uhh, I have somehow overseen the "exit 0". Need to get some sleep. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #10 from Paul Fox 2009-07-22 21:07:05 EDT --- i need to set up mock on f11 in order to recreate the rpmlint warnings. as for the silent scripts, the guidelines implied (and my shell experience says) that "exit 0" would work for this. i guess you're saying it does not? (again, apologies -- i have not had a chance to do an actual test with the new packages yet.) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #9 from Christoph Wickert 2009-07-22 20:54:03 EDT --- (In reply to comment #7) > rpmlint now gives 0 warnings/0 errors -- however, my development machine is > currently fedora 9. i did nothing for the "false positive" error that > cwickert reported. I still get warnings for the binary package: $ rpmlint olpc-powerd-8-1.20090720git702fd8e.fc9.i386.rpm olpc-powerd.i386: W: incoherent-version-in-changelog 8-1 ['8-1.20090720git702fd8e.fc9', '8-1.20090720git702fd8e'] olpc-powerd.i386: W: devel-file-in-non-devel-package /usr/bin/powerd-config The latter is bogus, the first is rpmlint not accepting the your changelog style. However according to https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs it is valid. I have overlooked something, an issue from comment # 3 is still open: - scriptlets should run silent. Add ||: at the end of the last command, so the scripts will always return 0. If they return 1, rpm will abort the transaction. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 Christoph Wickert changed: What|Removed |Added Flag|fedora-review? |fedora-review+ --- Comment #8 from Christoph Wickert 2009-07-22 20:37:59 EDT --- One last thing I have spotted: We also need to require upstart for initctl: # these are all for the manipulation of powerd Requires(post): upstart Requires(postun): upstart Requires(preun): upstart I have tested the package on my XO 1 powerd works fine, keys work etc. test case for scriptlets: $ /etc/init.d/ohmd status $ rpm -Ivvh olpc-powerd-8-1.20090720git702fd8e.fc9.i386.rpm $ /etc/init.d/ohmd status $ ps -A | grep powerd Upgrade: $ rpm -Uvvh olpc-powerd-8-1.20090720git702fd8e.fc9.i386.rpm --force $ ps -A | grep powerd (other process id) $ /etc/init.d/ohmd status Removal: $ rpm -evv olpc-powerd $ ps -A | grep powerd (other process id) $ /etc/init.d/ohmd status -> works as it should. The package is APPROVED ... but please don't forget to add the upstart requires after import. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #7 from Paul Fox 2009-07-20 10:32:48 EDT --- i believe i've addressed all of the FIX issues listed above. rpmlint now gives 0 warnings/0 errors -- however, my development machine is currently fedora 9. i did nothing for the "false positive" error that cwickert reported. regarding comment #5, for the "unsafe" rm -- i changed powerd to clean up after itself, so that the rm in the uninstall is no longer needed. i had the best intentions of testing the install/remove/upgrade paths for the scriptlets this weekend, but was not able to. i agree, given my reading of the spec, with cwickert's proposed implementation in comment #4, and have applied it verbatim. so: here are the new pre-release srpm and spec: http://dev.laptop.org/~pgf/rpms/srpms/olpc-powerd-8-1.20090720git702fd8e.src.rpm http://dev.laptop.org/~pgf/rpms/srpms/olpc-powerd.spec-8-1.20090720git702fd8e and an f-9 rpm: http://dev.laptop.org/~pgf/rpms/olpc-powerd-8-1.20090720git702fd8e.fc9.i386.rpm i will test at earliest opportunity, but if someone else tests sooner, please let me know. many thanks. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #6 from Christoph Wickert 2009-07-16 21:08:20 EDT --- BTW: he scripts from comment #4 result in a couple of additional deps: Requires(post): chkconfig Requires(preun): chkconfig # This is for /sbin/service Requires(preun): initscripts See: https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscript_packaging -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=507089
--- Comment #5 from Christoph Wickert 2009-07-16
19:27:11 EDT ---
One more thing: The rm -rf in %preun is not nice and causes an rpmlint warning.
We should let the package own the file, then rpm will remove it:
%install
...
mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/run
...
touch $RPM_BUILD_ROOT%{_localstatedir}/run/powerevents
%files
...
%ghost(missingok) %{_localstatedir}/run/powerevents
IMO this is the best solution, but we don't have a guideline for files in
/var/run. On my machine half of the files is owned by a package, the others are
not. I think I should ask packaging committee about this and/or write a
proposal.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #4 from Christoph Wickert 2009-07-16 19:21:40 EDT --- On the scriptlets again... Your scripts only handle the case where one replaces ohm with powerd, but not when upgrading powerd. The problem is the order: Upon an upgrade, %preun of the old package runs after %post of the new package, see https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Scriptlet_Ordering So what we get is: initctl start powerd initctl start olpc-switchd ... initctl stop olpc-switchd initctl stop powerd This cannot work! How about this: # Only on install %post if [ $1 = 1 ] ; then if test -e /etc/init.d/ohmd ; then service ohmd stop >/dev/null 2>&1 chkconfig ohmd off fi initctl -q start powerd initctl -q start olpc-switchd fi # Only on uninstall %preun if [ $1 = 0 ] ; then initctl stop -q olpc-switchd initctl stop -q powerd if test -e /etc/init.d/ohmd then /sbin/service ohmd start >/dev/null 2>&1 /sbin/chkconfig ohmd on fi fi # Restart after upgrade %postun if [ "$1" -ge "1" ] ; then initctl stop -q olpc-switchd initctl stop -q powerd initctl start -q powerd initctl start -q olpc-switchd fi -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=507089
--- Comment #3 from Christoph Wickert 2009-07-16
19:16:42 EDT ---
Sorry it took so long, but the scriptlets really caused me some headache...
(In reply to comment #1)
> i'm aware that the post-install hook that disables ohmd is almost certainly
> not
> acceptable. ohmd and olpc-powerd cannot run at the same time, though they can
> co-exist otherwise.
It's definitely not according to the guidelines, see
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Why_don.27t_we
But as ohmd is olpc specific and not in Fedora IMO we can make an exception
here.
REVIEW FOR 55f351e35ee8dbb622c4565a8a7b1412 olpc-powerd-7-1.src.rpm
FIX - MUST: rpmlint must be run on every package. The output should be posted
in the review:
$ rpmlint Desktop/olpc-powerd-*
olpc-powerd.i586: W: incoherent-version-in-changelog 6-2 ['7-1.fc11', '7-1']
=> See https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs
olpc-powerd.i586: W: devel-file-in-non-devel-package /usr/bin/powerd-config
=> false positive, see https://bugzilla.redhat.com/show_bug.cgi?id=468987#c1
olpc-powerd.i586: W: dangerous-command-in-%preun rm
=> that's /var/run/powerevents, save to igonore.
OK - MUST: The package is named according to the Package Naming Guidelines.
OK - MUST: The spec file name matches the base package %{name}, in the format
%{name}.spec.
OK - MUST: The package meets the Packaging Guidelines.
OK - MUST: The package is licensed with a Fedora approved license and meets the
Licensing Guidelines: GPLv2+
OK - MUST: The License field in the package spec file matches the actual
license.
FIX - MUST: The license file from the source package is included in %doc, but
it's LGPLv2.1
OK - MUST: The spec file is in American English.
OK - MUST: The spec file for the package is legible.
N/A - MUST: The sources used to build the package match the upstream source by
MD5 (Git checkout)
OK - MUST: The package successfully compiles and builds into binary rpms on
%{ix86}
N/A - MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch.
OK - MUST: All build dependencies are listed in BuildRequires.
N/A - MUST: The spec file handles locales properly with the %find_lang macro.
N/A - MUST: Every binary RPM package (or subpackage) which stores shared
library files (not just symlinks) in any of the dynamic linker's default paths,
must call ldconfig in %post and %postun.
N/A - MUST: If the package is designed to be relocatable, the packager must
state this fact in the request for review, along with the rationalization for
relocation of that specific package.
OK - MUST: The package owns all directories that it creates.
FIX - MUST: The package contains duplicate files in the %files listing:
warning: File listed twice: /etc/event.d/olpc-switchd
warning: File listed twice: /etc/event.d/powerd
warning: File listed twice: /etc/powerd/pleaseconfirm.pgm
warning: File listed twice: /etc/powerd/powerd.conf
warning: File listed twice: /etc/powerd/shuttingdown.pgm
OK - MUST: Permissions on files are set properly. Every %files section includes
a %defattr(...) line.
OK - MUST: The package has a %clean section, which contains rm -rf
$RPM_BUILD_ROOT.
OK - MUST: The package consistently uses macros, as described in the macros
section of Packaging Guidelines.
OK - MUST: The package contains code, or permissable content.
N/A - MUST: Large documentation files should go in a -doc subpackage.
OK - MUST: Files included as %doc do not affect the runtime of the application.
N/A - MUST: Header files must be in a -devel package.
N/A - MUST: Static libraries must be in a -static package.
N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires:
pkgconfig'.
N/A - MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1), then library files that end in .so (without suffix) must go in
a -devel package.
N/A - MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency: Requires: %{name} =
%{version}-%{release}
OK - MUST: The package does not contain any .la libtool archives.
N/A - MUST: Packages containing GUI applications must include a %{name}.desktop
file.
OK - MUST: The packages does not own files or directories already owned by
other packages.
OK - MUST: At the beginning of %install, the package runs rm -rf
$RPM_BUILD_ROOT.
OK - MUST: All filenames in rpm packages are valid UTF-8.
SHOULD Items:
N/A - SHOULD: If the source package does not include license text(s) as a
separate file from upstream, the packager SHOULD query upstream to include it.
N/A - SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if available.
OK - SHOULD: The the pa
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 Peter Robinson changed: What|Removed |Added Blocks||462625(FedoraOLPCDelta) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 Christoph Wickert changed: What|Removed |Added Status|NEW |ASSIGNED CC||fed...@christoph-wickert.de AssignedTo|nob...@fedoraproject.org|fed...@christoph-wickert.de Flag||fedora-review? --- Comment #2 from Christoph Wickert 2009-06-20 15:35:29 EDT --- (In reply to comment #1) > i've attempted to apply all the lessons i learned during the olpc-kbdshim > review. hope i've succeeded. We'll see. ;) Stay tuned. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 507089] Review Request: olpc-powerd - power management for the XO laptop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=507089 --- Comment #1 from Paul Fox 2009-06-20 14:46:54 EDT --- i've attempted to apply all the lessons i learned during the olpc-kbdshim review. hope i've succeeded. i'm aware that the post-install hook that disables ohmd is almost certainly not acceptable. ohmd and olpc-powerd cannot run at the same time, though they can co-exist otherwise. i'm hoping for advice on this. i also just noticed that the Source0: line is incorrect -- my automated package build commmands aren't correct yet. my apologies. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
