[Bug 225847] Merge Review: gnupg
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=225847
Nalin Dahyabhai na...@redhat.com changed:
What|Removed |Added
Flag|needinfo?(na...@redhat.com) |
--- Comment #3 from Nalin Dahyabhai na...@redhat.com 2009-02-19 15:32:26 EDT
---
(In reply to comment #1)
Hi Nalin. Here's a review for gnupg.
Thanks for that, sorry for the delay.
NEEDSWORK items
* rpmlint produces several warnings and errors on the srpm
Fixed up at various times by various people... looks pretty clean now.
The binary rpm produces one warning:
$ rpmlint gnupg-1.4.6-4.fc6-results/gnupg-1.4.6-4.fc6.i386.rpm
W: gnupg file-not-utf8 /usr/share/man/man1/gpg.ru.1.gz
I don't know Russian so I couldn't verify if iconv would properly converted
the
man page so I left it alone.
rpmlint says this needs to be UTF-8, but it's evidently KOI8-RU. Converting.
* Scriplets are sane
The scriptlets to install info pages could be simplified somewhat and made
more
consistent with the examples in Packaging/ScriptletSnippets
The additional checks fix #91641 (error when installed with --excludedocs).
I'd rather just leave it as-is and not have to worry about that.
Comments/Questions/Notes
There are a number of unneeded configure flags to enable zlib, bzip, readline,
and curl. These are all enabled by default in the current gnupg so they can
be
removed.
Dropped explicit requests for bzip, readline, and curl, but leaving the one for
zlib in so that configure won't fall back to the internal copy if something
looks off about the system one.
Why is %{_libdir} used for %{_libexecdir}? Packaging/Guidelines allow the use
of this dir and it is what upstream does by default. %{_libdir}/gnupg is used
for extensions, though none are currently shipped with this package (or by any
others in Fedora AFAIK).
We used to not allow %{_libexecdir}. Reverted.
Another very a minor point, the preferred value for the BuildRoot tag is
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
This is not a blocker.
Changed to match at some point. I think it looks pretty good now.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225847] Merge Review: gnupg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: gnupg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225847 [EMAIL PROTECTED] changed: What|Removed |Added Status|ASSIGNED|NEEDINFO Flag||needinfo?([EMAIL PROTECTED]) -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225847] Merge Review: gnupg
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: gnupg
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225847
[EMAIL PROTECTED] changed:
What|Removed |Added
Status|NEW |ASSIGNED
AssignedTo|[EMAIL PROTECTED]|[EMAIL PROTECTED]
CC||[EMAIL PROTECTED]
Flag||fedora-review?
--- Additional Comments From [EMAIL PROTECTED] 2007-02-03 10:32 EST ---
Hi Nalin. Here's a review for gnupg.
MUST items verified
* Adheres to naming guidelines
* Specfile name matches package name
* Meets packaging guidelines (see below for comment on %{_libexecdir} usage
* License meets open-source requirements
* License included in %doc
* License field matches the upstream license
* Specfile is in American English
* Specfile is legible
* Source matches upstream (sha1: 9cbbef5c94f793867ff3ae4941816962311a0563)
* Builds, installs, and works (tested on FC6, i386)
* Owns directories that it creates
* Does not own files or directories of other packages
* File list has no duplicates
* File perms are sane
* Specfile includes %clean section
* Macros used consistently
* Package contains code or permissible content
SHOULD items verified
* Builds in mock against fedora-{5,6}-i386-core targets
* Package functions correctly (tested on FC6, i386)
NEEDSWORK items
* rpmlint produces several warnings and errors on the srpm
$ rpmlint gnupg-1.4.6-3.src.rpm
W: gnupg summary-ended-with-dot A GNU utility for secure communication and data
storage.
E: gnupg tag-not-utf8 %changelog
E: gnupg non-utf8-spec-file gnupg.spec
W: gnupg buildprereq-use autoconf, automake, bzip2-devel, expect, ncurses-devel
W: gnupg buildprereq-use openldap-devel, readline-devel, zlib-devel,
gettext-devel
W: gnupg buildprereq-use curl-devel
W: gnupg buildprereq-use libusb-devel
W: gnupg unversioned-explicit-provides gpg
W: gnupg unversioned-explicit-provides openpgp
W: gnupg prereq-use /sbin/install-info
W: gnupg make-check-outside-check-section make check
E: gnupg use-of-RPM_SOURCE_DIR
W: gnupg mixed-use-of-spaces-and-tabs (spaces: line 73, tab: line 50)
All except the unversioned-explicit-provides on the virtual gpg and openpgp
packages should be corrected.
The binary rpm produces one warning:
$ rpmlint gnupg-1.4.6-4.fc6-results/gnupg-1.4.6-4.fc6.i386.rpm
W: gnupg file-not-utf8 /usr/share/man/man1/gpg.ru.1.gz
I don't know Russian so I couldn't verify if iconv would properly converted the
man page so I left it alone.
* Scriplets are sane
The scriptlets to install info pages could be simplified somewhat and made more
consistent with the examples in Packaging/ScriptletSnippets
Comments/Questions/Notes
There are a number of unneeded configure flags to enable zlib, bzip, readline,
and curl. These are all enabled by default in the current gnupg so they can be
removed.
Why is %{_libdir} used for %{_libexecdir}? Packaging/Guidelines allow the use
of this dir and it is what upstream does by default. %{_libdir}/gnupg is used
for extensions, though none are currently shipped with this package (or by any
others in Fedora AFAIK).
The CFLAGS are set explicitly to prevent the binaries from having text
relocations, as per BZ#145836 (in case anyone wonders about that).
Another very a minor point, the preferred value for the BuildRoot tag is
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
This is not a blocker.
I was unable to build this in mock for the development target due to expect
having a broken dep on libtcl8.4.so at the moment.
NEEDSWORK
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225847] Merge Review: gnupg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: gnupg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225847 --- Additional Comments From [EMAIL PROTECTED] 2007-02-03 10:37 EST --- Created an attachment (id=147274) -- (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=147274action=view) patch correcting small issues mentioned above Here's a patch to correct the (relatively minor) issues mentioned above. Feel free to ditch my changelog entry if you use any parts of the patch. I'll take the blame for things I break but I don't care about getting credit. :) -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225847] Merge Review: gnupg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: gnupg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225847 [EMAIL PROTECTED] changed: What|Removed |Added AssignedTo|[EMAIL PROTECTED]|[EMAIL PROTECTED] -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
