Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=904843 --- Comment #21 from Al Stone <ahs3@xxxxxxxxxx> --- (In reply to comment #18) > Non-sponsor review: > > Package Review > ============== > > Key: > [x] = Pass > [!] = Fail > [-] = Not applicable > [?] = Not evaluated > [ ] = Manual review needed > > > > ===== MUST items ===== > > C/C++: > [x]: Package does not contain kernel modules. > [x]: Package contains no static executables. > [x]: Package does not contain any libtool archives (.la) > [x]: Rpath absent or only used for internal libs. > > Generic: > [x]: Package is licensed with an open-source compatible license and meets > other legal requirements as defined in the legal section of Packaging > Guidelines. > [!]: %build honors applicable compiler flags or justifies otherwise. > > Note: Package comments explain why optimizations are disabled, > but the other compiler option flags should be honoured > (via %{optflags}) such as -g Use of %{optflags} now incorporated. It appears to have no affect on the test results and upstream does not appear to be as adamant as they used to be about not using -O with this latest version. > [x]: Package contains no bundled libraries without FPC exception. > [x]: Changelog in prescribed format. > [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the > beginning of %install. > > Note: rm -rf %{buildroot} present but not required Removed. > [x]: Sources contain only permissible code or content. > [x]: Each %files section contains %defattr if rpm < 4.4 > > Note: %defattr present but not needed Removed. No sense in carrying around something not needed. > [-]: Package contains desktop file if it is a GUI application. > [!]: Development files must be in a -devel package > > Note: should be automatically produced if -g option is used > during make Is this applicable? -debuginfo packages are automatically produced, but this package contains no libraries that would need header files and the like. > [x]: Package requires other packages for directories it uses. > [x]: Package uses nothing in %doc for runtime. > [!]: Package is not known to require ExcludeArch. > > Note: ExcludeArch should not be included (these tools can > be used to cross-prepare ACPI tables even on systems that > cannot use them) Removed. PPC and s390 do not build from source; as secondary architectures, this is not critical, but I would like to resolve it soon. > [x]: Package complies to the Packaging Guidelines > [-]: 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]: License field in the package spec file matches the actual license. > [x]: Package consistently uses macros (instead of hard-coded directory > names). > [x]: Package is named according to the Package Naming Guidelines. > [!]: Package does not generate any conflict. > > Note: Package should obsolete pmtools - alternatives mechanism will > only work if the other package is alternatives-aware, and it appears > to contain an older version of the same tools. Coordinate the > retirement of the obsoleted packages with the other package > maintainers. Obsoleting pmtools would remove the acpidump tool from Fedora; I actually use it, and perhaps others do, too. What I've done is provide a patch to the pmtools package and filed a bug asking for pmtools to use the alternatives mechanism for acpixtract (the only command in common between the two packages); please see BZ#924442 for details. I've also made that a blocker for this bug. > [x]: Package obeys FHS, except libexecdir and /usr/target. > [!]: If the package is a rename of another package, proper Obsoletes and > Provides are present. > > See note above. See note above :). > [x]: Package must own all directories that it creates. > [!]: Package does not own files or directories owned by other packages. > > See note on pmtools conflict above. See note above :). > [x]: Requires correct, justified where necessary. > [!]: Only use %_sourcedir in very specific situations. > > Note: %_sourcedir is used. Use %{SOURCE1} to refer to the Source1 > file instead. Fixed. > [x]: Spec file is legible and written in American English. > [x]: Package contains systemd file(s) if in need. > [!]: Useful -debuginfo package or justification otherwise. > > See notes on optflags above. With use of %optflags, this should now be corrected. > [-]: Large documentation must go in a -doc subpackage. > [x]: All build dependencies are listed in BuildRequires, except for any that > are listed in the exceptions section of Packaging Guidelines. > [x]: Macros in Summary, %description expandable at SRPM build time. > [x]: Package does not contain duplicates in %files. > [x]: Permissions on files are set properly. > [x]: Fully versioned dependency in subpackages, if present. > [x]: Spec file lacks Packager, Vendor, PreReq tags. > [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't > work. > [x]: Package is named using only allowed ASCII characters. > [x]: Package do not use a name that already exist > [x]: Package is not relocatable. > [x]: Sources used to build the package match the upstream source, as provided > in the spec URL. > [x]: Spec file name must match the spec package %{name}, in the format > %{name}.spec. > [x]: File names are valid UTF-8. > [x]: Packages must not store files under /srv, /opt or /usr/local > [x]: Package successfully compiles and builds into binary rpms on at least > one > supported primary architecture. > [x]: Package installs properly. > [x]: Rpmlint is run on all rpms the build produces. > Note: There are rpmlint messages (see attachment). > > ===== SHOULD items ===== > > Generic: > [!]: Uses parallel make. > > Note: Add %{?_smp_mflags} to the make to enable this. Added. > > [!]: Package has no %clean section with rm -rf %{buildroot} (or > $RPM_BUILD_ROOT) > > Note: %clean present but not required Removed %clean section completely. > [!]: If the source package does not include license text(s) as a separate > file > from upstream, the packager SHOULD query upstream to include it. The source is being licensed under the GPLv2 (and upstream is pretty religious about including the right text in all of the source files). Is it necessary to include a copy of GPLv2? I can, but it seems redundant. > [x]: Final provides and requires are sane (see attachments). > [x]: Package functions as described. > [x]: Latest version is packaged. > [x]: Package does not include license text files separate from upstream. > [!]: Patches link to upstream bugs/comments/lists or are otherwise justified. > > Note: Making "alike as possible" to Debian doesn't really explain > why the patches are justified. Ah. True. The patches are from the original iasl package and still solve some existing problems for big-endian builds and for systems that do not like unaligned accesses. I mistakenly removed the commentary from the front of the patches; that has been restored plus some of the additional commentary from the iasl changelog that explained which BZ#s were being fixed. NB: in the process of checking out other arch builds for this reply, I had to create another patch (name-miscompare.patch) to correct an upstream bug I uncovered by running the misc tests (incorrect byte order on big-endian machines). > [-]: Description and summary sections in the package spec file contains > translations for supported Non-English languages, if available. > [-]: Package should compile and build into binary rpms on all supported > architectures. > [x]: %check is present and all tests pass. > [x]: Packages should try to preserve timestamps of original installed files. > [x]: Sources can be downloaded from URI in Source: tag > [x]: Reviewer should test that the package builds in mock. > [x]: Buildroot is not present > [x]: Dist tag is present. > [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. > [x]: SourceX tarball generation or download is documented. > [x]: SourceX is a working URL. > [x]: Spec use %global instead of %define. > > ===== EXTRA items ===== > > Generic: > [x]: Large data in /usr/share should live in a noarch subpackage if package > is > arched. > [x]: Rpmlint is run on all installed packages. > Note: There are rpmlint messages (see attachment). > [x]: Spec file according to URL is the same as in SRPM. > > > Rpmlint > ------- > Checking: acpica-tools-20130214-1.fc17.x86_64.rpm > acpica-tools.x86_64: W: spelling-error %description -l en_US acpidump -> > dumpiness > acpica-tools.x86_64: W: spelling-error %description -l en_US pmtools -> pm > tools, pm-tools, tools > 1 packages and 0 specfiles checked; 0 errors, 2 warnings. > > > > > Rpmlint (installed packages) > ---------------------------- > # rpmlint acpica-tools > acpica-tools.x86_64: W: spelling-error %description -l en_US acpidump -> > dumpiness > acpica-tools.x86_64: W: spelling-error %description -l en_US pmtools -> pm > tools, pm-tools, tools > 1 packages and 0 specfiles checked; 0 errors, 2 warnings. > > > ===== Additional comments: > > - Recommend the use of globs in %files section to (a) reduce spec file size > and (b) reduce the number of edits required as upstream changes (e.g., adds > or deletes binaries): > > %{_bindir}/* > %{_mandir}/*/* Done. > The install section could also use globs to reduce editing for future > releases: > > install -pD generate/unix/bin/*/* %{buildroot}%{_bindir}/ > install -pDm 0644 *.1 %{buildroot}%{_mandir}/man1/ Done. > - Avoid macro references in changelog (%check) Whups. Fixed. > - README.Fedora notes on git clone steps are apparently obsolete Removed. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=28HslIKem8&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review