I need a sponsor. Spec URL: http://sven.lank.es/Fedora/SPECS/yacpi.spec SRPM URL: http://sven.lank.es/Fedora/SRPM/yacpi-3.0.1-5.fc9.src.rpm Description: Yacpi is an acpi monitoring program for Linux. It displays various acpi information like battery status and ac status on notebooks. yacpi depends on libacpi for which I am going to create a separate package review.
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.