Bug 239096
Summary: | Review Request: perl-libwhisker2 - Perl module geared specificly for HTTP testing | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Sindre Pedersen Bjørdal <sindrepb> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | rayvd |
Target Milestone: | --- | Keywords: | Reopened |
Target Release: | --- | Flags: | j:
fedora-review+
petersen: fedora-cvs+ |
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-05-27 04:41:40 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: | 239097 |
Description
Sindre Pedersen Bjørdal
2007-05-04 20:47:20 UTC
I'll take a look. Looks clean, but there are a couple of problems: W: perl-libwhisker2 unversioned-explicit-obsoletes perl-libwhisker W: perl-libwhisker2 unversioned-explicit-provides perl-libwhisker When you obsolete a package in this manner, you need to use versioned provides and obsoletes. The simplest thing is to obsolete <= the current perl-libwhisker version, and provide current version+epsilon. libwhisker2-2.4-lw1bridge.patch basically adds a GPL license statement onto a file. While I agree that it's best if each source file contains the (abbreviated) license statement, this is generally something that upstream should be doing instead of package maintainers. There's a test suite, and it actually works fine, but because it uses fixed ports it's not really suitable for running within the buildsys. Stick this in a %check section to run it: cd t perl ./test.pl Still, it would need to be commented out in the spec or made conditional on a flag so the buildsys wouldn't break. There's a typo in the Source0: line; "wiretripnet" should be "wiretrip.net". I'm not sure why you have those BuildRequires:; the package seems identical without them. What you may need is some Requires: instead, because the built package doesn't end up with any. Oddly, though, the test suite still passes. * source files match upstream: 5d136f6de318e140a03f76a50392628ed7d00fbee297579ae29b81466f8c8e00 libwhisker2-2.4.tar.gz * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * dist tag is present. * build root is OK. * license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. ? BuildRequires are proper. * %clean is present. * package builds in mock (development, x86_64). * package installs properly X rpmlint has complaints about unversioned provides and obsoletes. ? the dependencies seem a bit light: perl(LW) perl(LW2) perl-libwhisker perl-libwhisker2 = 2.4-1.fc7 = perl(:MODULE_COMPAT_5.8.8) perl(LW2) perl(strict) perl(vars) * %check is not present. There is a test suite, but it's not appropriate for running inside the buildsys. All tests pass when run manually. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no scriptlets present. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. Update: - Fix typo in Source0 url - Update lw1bridge patch to not include License info - Add explicit version to Provides and Obsoletes - Added tests, commented out - Cleaned up BuildRequires and Requires The Requires on openssl-perl and perl-Net-SSLeay I know are valid. The MD5 dependency I'm not sure about, but I think the LW2.pm actually does use it. Can you verify? Spec URL: http://folk.ntnu.no/sindrb/packages/green_nyc/perl-libwhisker2.spec SRPM URL: http://folk.ntnu.no/sindrb/packages/green_nyc/perl-libwhisker2-2.4-2.fc7.src.rpm OK, builds fine and rpmlint is quiet. You don't need "BuildRequires: perl" but it doesn't hurt anything to have it. Note for others who may check for BuildRequires: needed for the perl-devel split: this module doesn't build like a regular module and has a custom Makefile.pl which doesn't make use of the regular Perl build infrastructure. Yes, this package does at least try to use MD5 at some point although it will use an internal implementation if the module isn't available. At least according to my reading of src/mdx.pl. The only issue at this point that concerns me is the name of the package. Our naming guidelines indicate that packages should be named according to how they're carried in CPAN, but of course this module isn't in CPAN. According to CPAN rules it wold be called perl-LW2, but that seems somewhat suboptimal as well. Frankly I favor the current naming, but before I approve I'll gather a couple of other opinions. OK, I've thought about it some more and I'm happy with the current naming. libwhisker2 is the name that upstream users and that people would expect to find this package under, we can't guess at what CPAN would name the module since it accepts module names that are not identical to the name of the topmost defined package, and the expected Perl library provides perl(LW2) are still there for automatic dependencies. APPROVED New Package CVS Request ======================= Package Name: perl-libwhisker2 Short Description: Perl module geared specificly for HTTP testing Owners: foolish Branches: FC-5 FC-6 EL-4 EL-5 InitialCC: jaboutbo Found this as a result of a review on nikto (#239097). They LW1 compatability bridge does not appear to be being installed correctly: # rpm -ql perl-libwhisker2.noarch /usr/lib/perl5/vendor_perl/5.8.8/LW2.pm /usr/share/doc/perl-libwhisker2-2.4 /usr/share/doc/perl-libwhisker2-2.4/CHANGES /usr/share/doc/perl-libwhisker2-2.4/LICENSE /usr/share/doc/perl-libwhisker2-2.4/README /usr/share/man/man3/LW2.3pm.gz (Note: this is from FC6, but I think the same would occur in FC7) Building from the .spec file I see the following: Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.98077 + umask 022 + cd /net/leoray/leoray1/users/ray5147/rpmbuild/BUILD + cd libwhisker2-2.4 + LANG=C + export LANG + unset DISPLAY + rm -rf /var/tmp/perl-libwhisker2-2.4-2-root-ray5147 + mkdir -p /var/tmp/perl-libwhisker2-2.4-2-root-ray5147/usr/lib/perl5/vendor_perl/5.8.8 + mkdir -p /var/tmp/perl-libwhisker2-2.4-2-root-ray5147/usr/share/man/man3 + make install DESTDIR=/var/tmp/perl-libwhisker2-2.4-2-root-ray5147 export DESTDIR perl Makefile.pl install LW2.pm installed to /var/tmp/perl-libwhisker2-2.4-2-root-ray5147/usr/lib/perl5/vendor_perl/5.8.8 LW2.3pm installed to /var/tmp/perl-libwhisker2-2.4-2-root-ray5147/usr/share/man/man3 perl Makefile.pl install_compat Error: bad build command 'install_compat' The solution is to use this libwhisker2-2.4-lw1bridge.patch file instead of the one in the SRPM: http://www.bludgeon.org/~rayvd/rpms/perl-libwhisker2/libwhisker2-2.4-lw1bridge.patch The only difference being that we use "perl Makefile.pl install_lw1" as per the documentation instead of "perl Makefile.pl install_compat" Once I make this change I now see the LW v1 compatability bridge installed correctly and can use the nikto package. patch applied and package rebuilt in koji for devel, which is F8. The F7 branch is missing from all my packages, I'm looking into this. Seems the F7 branch wasn't created during merge, please add it. New Package CVS Request ======================= Package Name: halberd Short Description: Tool to discover HTTP load balancers Owners: foolish Branches: FC-6 EL-4 EL-5 F-7 You mean: Package Change Request ====================== Package Name: perl-libwhisker2 New Branches: F-7 Again, please do not reopen Package Review's for CVS Requests, since they are handled by the fedora-cvs flag not bug state. F-7 branch edded. |