Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 166713
Summary: | Review Request: perl-GnuPG-Interface | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Matt Domsch <matt_domsch> |
Component: | Package Review | Assignee: | Paul Howarth <paul> |
Status: | CLOSED NEXTRELEASE | QA Contact: | David Lawrence <dkl> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-extras-list, rc040203, tcallawa |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | http://domsch.com/linux/fedora/extras/perl-GnuPG-Interface | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2005-10-05 17:44:06 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779, 168070 |
Description
Matt Domsch
2005-08-24 20:30:12 UTC
Review: - rpmlint clean - package and spec naming OK - package meets guidelines - license is same as perl, matches spec - spec written in English and is legible - sources match upstream - builds OK on FC4 and in mock for rawhide (i386) - buildreqs OK - no locales, libraries, subpackages or pkgconfigs to worry about - not relocatable - no directory ownership or permissions issues - no duplicate files - %clean section present and correct - macro usage consistent - code, not content - no large docs - docs don't affect runtime - no scriptlets Nitpick: - %check output can be cleaned somewhat by adding "chmod 700 test" before "make test" - any idea what's happening here? + make test PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/clearsign................ok t/decrypt..................ok t/detach_sign..............ok t/encrypt..................ok 1/3unknown record type tru at blib/lib/GnuPG/Interface.pm (autosplit into blib/lib/auto/GnuPG/Interface/get_keys.al) line 548, <GEN27> line 1. t/encrypt..................ok t/encrypt_symmetrically....ok t/export_keys..............ok t/Fingerprint..............ok t/get_public_keys..........unknown record type tru at blib/lib/GnuPG/Interface.pm (autosplit into blib/lib/auto/GnuPG/Interface/get_keys.al) line 548, <GEN14> line 1. t/get_public_keys..........ok t/get_secret_keys..........ok t/import_keys..............ok t/Interface................ok t/list_public_keys.........ok t/list_secret_keys.........ok t/list_sigs................ok t/passphrase_handling......ok t/sign.....................ok t/sign_and_encrypt.........ok t/UserId...................ok t/verify...................ok t/wrap_call................ok - for an FC3 build you don't need the patch, so you could maintain a different spec in the FC3 branch, or conditionally apply the patch: %if 0%{?fedora} > 3 %patch0 -p1 %endif - license text not included in package; suggest adding to %setup: perldoc perlgpl > GPL perldoc perlartistic > Artistic and to %files: %doc GPL Artistic - although "BuildReq: gpg" works, might "gnupg" be better? Fixed everything noted by Paul. Found a patch from Mail::GPG to handle the 'tru' (trust?) field now reported by gnupg-1.4. Apply the test-fix patch only for gnupg > 1.2. Spec Name or Url: http://domsch.com/linux/fedora/extras/perl-GnuPG-Interface/perl-GnuPG-Interface.spec SRPM Name or Url: http://domsch.com/linux/fedora/extras/perl-GnuPG-Interface/perl-GnuPG-Interface-0.33-1.src.rpm Thanks for your insight, please review again. I dislike the magic being used to apply the patch conditionally. As I don't have FC3 anymore, I can't easily test, so let me ask: Does Patch1 cause any harm/failures/malfunctions on FC3? - If no, then I'd propose you to apply the patch unconditionally. - If yes, then I'd propose to replace the gpgversion detection magic by using 2 different versions of the spec file (one for FC3 and one for FC4). Missing: Requires: gpg (In reply to comment #3) > I dislike the magic being used to apply the patch conditionally. > As I don't have FC3 anymore, I can't easily test, so let me ask: Does Patch1 > cause any harm/failures/malfunctions on FC3? > > - If no, then I'd propose you to apply the patch unconditionally. > > - If yes, then I'd propose to replace the gpgversion detection magic by using 2 > different versions of the spec file (one for FC3 and one for FC4). "make test" fails on FC3 if the patch is applied because the output format from gpg is not recognised. Regarding version detection magic, I'm personally in favour of it (at least if it's fairly clear what's going on - you might want to add a comment about it in the spec) because I can maintain a single spec file for all branches (currently devel, FC-4, and Fc-3), which makes future changes easier to do. Others, like Ralf, clearly prefer to maintain separate spec files when necessary in each branch, which will be clearer to read and less error-prone if there's ever an issue with the "magic". But I think it's the packager's choice, not a policy issue. > Missing: > Requires: gpg Yes, this is needed (or Requires: gnupg). (In reply to comment #4) > (In reply to comment #3) > > I dislike the magic being used to apply the patch conditionally. > Regarding version detection magic, I'm personally in favour of it ( Others, like > Ralf, clearly prefer to maintain separate spec files when necessary in each > branch, which will be clearer to read and less error-prone if there's ever an > issue with the "magic". The point is: This magic is only necessary to cater obsolete versions of gpg and versions of Fedora to be EOL'ed soon. It thereby unnecessarily polutes upstream Fedora rpm.specs. I think such selective patch magic should be allowed at the packager's preference. However, given there would be a need for exactly 2 spec files at this point (FC4 and higher, and FC3 and below), I've removed the magic for the package to go into the devel trunk, and the patch can be removed in the FC3 branch if/when one is made. Also, tested with gnupg2, and it works (provided you either symlink gpg to gpg2, or set GnuPG::Interface::call => gpg2. As both gnupg and gnupg2 Provide: gpg, I've changed the BR and Requires lines back to requiring gpg. Spec Name or Url: http://domsch.com/linux/fedora/extras/perl-GnuPG-Interface/perl-GnuPG-Interface.spec SRPM Name or Url: http://domsch.com/linux/fedora/extras/perl-GnuPG-Interface/perl-GnuPG-Interface-0.33-2.src.rpm Please use "perldoc -t" rather than plain "perldoc" - I forgot that in Comment #1. You might also want to add "%{?_smp_mflags}" as an option to make in %build. You can address these in CVS. APPROVED. Ping? It's 2 weeks since this package had been approved. I was hoping to have legal approval to become a contributor by now. I'll ask spot to commit it to CVS on my behalf. spot, I fixed the last nits Paul noticed, and put the srpm here, if you would be so kind: http://domsch.com/linux/fedora/extras/perl-GnuPG-Interface/perl-GnuPG- Interface-0.33-3.fc4.src.rpm This should be built ok now. |