Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=904843
--- Comment #21 from Al Stone <ahs3(a)redhat.com> ---
(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