Bug 445152

It is recommended to use macros instead of using fixed paths. Therefore
"/usr/share/doc/" should be replaced by %{_defaultdocdir} and "/usr/share/man/"
by %{_mandir}

Based on the header of get_cpu.* files, I think that the correct license tag is
GPLv2+.

The presence of the explicit CFLAGS line in the Makefile prohibits the build
process to use the mandatory Fedora compile flags.

> It is recommended to use macros instead of using fixed paths.

Fixed. I'm also now using %{_defaultdocdir} while converting the changelog to
UTF8 - is that correct there?

> Based on the header of get_cpu.* files, I think that the correct 
> license tag is GPLv2+.

You are right. I don't know why I entered GPLv2

> The presence of the explicit CFLAGS line in the Makefile prohibits the build
> process to use the mandatory Fedora compile flags.

Removed.

New SRPM: http://sven.lank.es/Fedora/SRPM/yacpi-3.0.1-6.fc9.src.rpm
SPEC: http://sven.lank.es/Fedora/SPECS/yacpi.spec

The patch is dangerous. If it were a larger program, there would
be the risk that DESTDIR finds its way into compiled files.
prefix should become /usr, and only the install paths (in the
install Makefile target) should start with DESTDIR.


> Requires: ncurses

Drop this. There's an automatic dependency on the ncurses
library SONAME added by rpmbuild. Query the binary rpm to see.


%dir %{_defaultdocdir}/yacpi  is missing in %files list.


manual pages are marked as %doc automatically.


CFLAGS still don't include $RPM_OPT_FLAGS.

Should be all fixed now. Thanks.

New Version:

New SRPM: http://sven.lank.es/Fedora/SRPM/yacpi-3.0.1-7.fc9.src.rpm
SPEC: http://sven.lank.es/Fedora/SPECS/yacpi.spec

The package installed into /usr/share/doc/yacpi instead of yacpi-version until now.

The following update fixes this:

New SRPM: http://sven.lank.es/Fedora/SRPM/yacpi-3.0.1-8.fc9.src.rpm
SPEC: http://sven.lank.es/Fedora/SPECS/yacpi.spec

I am not sponsored yet, so cannot review. Just have some helpful remarks 

Could not build:
No package libacpi-devel

Without building I have a small suggestions:

1. You should consistently use %{name} e.g Source0 link or in %files (man page)
2. For single file encoding change this one looks lot simple and clean
 iconv -f iso-8859-1 -t utf-8 -o CHANGELOG{.utf8,}
 mv CHANGELOG{.utf-8}

Yours is also okay, it is your choice.

3. description needs a bit correction:
    This part "battery status and ac status" is mentioned twice.

4. Preferred Buildroot tag is %(mktemp -ud %{_tmppath}/%{name}-%{version}-
%{release}-XXXXXX)

due to missing build requirements (libacpi-devel) that is not available in 
Fedora.  If you've or are planning to submitted it for review, then it should 
be a blocker on this bug.

Hello Rakesh, 

thanks for your remarks. I have updated the specfile:

SPEC: http://sven.lank.es/Fedora/SPECS/yacpi.spec
New SRPM: http://sven.lank.es/Fedora/SRPM/yacpi-3.0.1-9.fc9.src.rpm

0. no package libacpi-devel: libacpi is a separate review request - #445153 -
445153 blocks this bug so it looks correct in my eyes?
1. fixed
2. changed, I agree - two lines less is better
3. fixed
4. changed

Due to the stricter checking implemented now in rpm, you'll have to replace
"%patch -p1" with "%patch0 -p1", otherwise the patch is not applied.


Two more serious issues
- Neither rpm_opt_flags nor parallel build flags are taken into consideration
while actually compiling
- the created debuginfo rpm is empty

> Due to the stricter checking implemented now in rpm, you'll have to replace
> "%patch -p1" with "%patch0 -p1", otherwise the patch is not applied.

fixed

- Neither rpm_opt_flags 

Wow - good catch. Fixed. I've also notified upstream.

- nor parallel build flags are taken into consideration

Fixed

- the created debuginfo rpm is empty

... because the makefile was stripping the binary. Fixed.

forgot the new URLs:

Spec URL: http://sven.lank.es/Fedora/SPECS/yacpi.spec
SRPM URL: http://sven.lank.es/Fedora/SRPM/yacpi-3.0.1-10.fc10.src.rpm

https://bugzilla.redhat.com/show_bug.cgi?id=445153#c17 applies here too, for the moment. will come back once I have news.

Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format
%{name}.spec.
 [x] Package meets the Packaging Guidelines.
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested on: devel/x86_64, devel/PPC
 [x] Rpmlint output:
source RPM: empty
binary RPM:empty
 [x] Package is not relocatable.
 [x] Buildroot is correct ( %(mktemp -ud
%{_tmppath}/%{name}-%{version}-%{release}-XXXXXX))
 [x] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type:MIT
 [x] If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided
in the spec URL.
     SHA1SUM of package: 4f2911cb0c737335003c2c13edef76821dee2cd1 
/tmp/libacpi-0.2.tar.gz
 [!] Package is known to not work on ppc/ppc64
See issue 1
 [x] All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [x] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [x] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [x] Development .so files in -devel subpackage, if present.
 [x] Fully versioned dependency in subpackages, if present.
 [x] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI
application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: devel/x86_64, devel/ppc
 [-] Package should compile and build into binary rpms on all supported
architectures.
     Tested on:
 [!] Package functions as described.
See isssue 1
 [x] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.


=== Issues ===
1. See https://bugzilla.redhat.com/show_bug.cgi?id=445153#c19
Short version: the code is dependent on x86 specific architecture and cannot be
used on ppc
Therefore the ppc architecture must be excluded


Sven, please add the required excludearch line in the spec and I'll approve the
package.
Wishful thinking: it would be nice of you could persuade the upstream author to
rewrite the code in a more portable way. Or ar least tell him about the
problems

ExcludeArch added. 

libacpi/yacpi can hopefully be replaced by something based on DeviceKitPower in the future but until that happens the tools are still very useful.

Spec URL: http://sven.lank.es/Fedora/SPECS/yacpi.spec
SRPM URL: http://sven.lank.es/Fedora/SRPM/yacpi-3.0.1-11.fc10.src.rpm

package APPROVED

New Package CVS Request
=======================
Package Name: yacpi
Short Description: Yet Another Configuration and Power Interface 
Owners: slankes
Branches: F-9
InitialCC:
Cvsextras Commits: yes

cvs done.

Sven, I see that the package was built. Any reason to not close the bug ?

yacpi-3.0.1-11.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/yacpi-3.0.1-11.fc9

yacpi-3.0.1-11.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update yacpi'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-8337

yacpi-3.0.1-11.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.