Bug 673585 - Review Request: perl-Apache-Htgroup - Manage Apache htgroup files
Review Request: perl-Apache-Htgroup - Manage Apache htgroup files
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2011-01-28 15:18 EST by Steven Hadfield
Modified: 2015-10-02 11:52 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
panemade: fedora‑review?

Attachments (Terms of Use)

  None (edit)
Description Steven Hadfield 2011-01-28 15:18:00 EST
Spec URL: http://www.letu.edu/people/stevenhadfield/rpm/SPECS/perl-Apache-Htgroup.spec
SRPM URL: http://www.letu.edu/people/stevenhadfield/rpm/SRPMS/perl-Apache-Htgroup-1.22-1.fc15.src.rpm
Description: Perl CPAN module for managing Apache htgroup files
Comment 1 Steven Hadfield 2011-01-28 15:21:40 EST
First posted SPEC file and I need a sponsor. I plan to submit other SPEC files that I've generated for Perl as well as potentially ones for PHP and Python
Comment 2 Ralf Corsepius 2011-02-04 01:59:39 EST
Not a formal review, just some remarks:

* Invalid license field:
License:        Distributable, see LICENSE

According to http://cpansearch.perl.org/src/RBOW/Apache-Htgroup-1.23/LICENSE
and http://cpansearch.perl.org/src/RBOW/Apache-Htgroup-1.23/lib/Apache/Htgroup.pm
this package is licensed "under the same terms as Perl itself"

=> Set the license field to GPLv2+ or Artistic

* Apache-Htgroup-1.22 is outdated (from 2002)
The current version in CPAN is 1.23 (from 2010)

Please upgrade your submission to 1.23
Comment 3 Michael Schwendt 2011-02-05 08:30:17 EST
Please enter your real name in your bugzilla account preferences. That will also improve the current http://fedoraproject.org/PackageReviewStatus/NEEDSPONSOR.html list.
Comment 4 Steven Hadfield 2011-02-11 10:02:56 EST
Thank you for your assistance. I have made the mentioned changes.

Here's the updated URL to the srpm:
Comment 5 PRABIN KUMAR DATTA 2011-04-20 21:49:06 EDT
* MUST: rpmlint must be run on every package                         [Fix  ]
--/Rpmlint output/--
$ rpmlint -i RPMS/noarch/perl-Apache-Htgroup-1.23-1.fc14.noarch.rpm 
perl-Apache-Htgroup.noarch: W: invalid-license Artistic
The value of the License tag was not recognized.  Known values are: "AAL",
"Adobe", "ADSL", "AFL", "AGPLv1", "AGPLv3", "AGPLv3 with exceptions",
"AMDPLPA", "AML", "AMPAS BSD", "APSL 2.0", "APSL 2.0+", "ARL", "Artistic 2.0",
"Artistic clarified", "ASL 1.0", "ASL 1.0+", "ASL 1.1", "ASL 1.1+", "ASL 2.0",
"ASL 2.0+", "Beerware", "BeOpen", "BitTorrent", "Boost", "BSD", "BSD
Protection", "BSD with advertising", "CATOSL", "CC0", "CeCILL", "CeCILL-B",
"CeCILL-C", "CDDL", "CNRI", "Condor", "Copyright only", "CPAL", "CPL",
"Crystal Stacker", "DOC", "dvipdfm", "ECL 1.0", "ECL 2.0", "eCos", "EFL 2.0",
"EFL 2.0+", "Entessa", "EPL", "ERPL", "EU Datagrid", "EUPL 1.1", "Eurosym",
"Fair", "FTL", "Giftware", "GL2PS", "Glide", "gnuplot", "GPL+", "GPL+ or
Artistic", "GPL+ with exceptions", "GPLv1", "GPLv2 or Artistic", "GPLv2+ or
Artistic", "GPLv2", "GPLv2 with exceptions", "GPLv2+", "GPLv2+ with
exceptions", "GPLv3", "GPLv3 with exceptions", "GPLv3+", "GPLv3+ with
exceptions", "IBM", "IJG", "ImageMagick", "iMatix", "Imlib2", "Intel ACPI",
"Interbase", "ISC", "Jabber", "JasPer", "JPython", "Knuth", "LBNL BSD",
"LGPLv2", "LGPLv2 with exceptions", "LGPLv2+", "LGPLv2+ or Artistic", "LGPLv2+
with exceptions", "LGPLv3", "LGPLv3 with exceptions", "LGPLv3+", "LGPLv3+ with
exceptions", "libtiff", "LLGPL", "Logica", "LPL", "LPPL", "mecab-ipadic",
"MirOS", "MIT", "MIT with advertising", "mod_macro", "Motosoto", "MPLv1.0",
"MPLv1.0+", "MPLv1.1", "MPLv1.1+", "MS-PL", "MS-RL", "Naumen", "NCSA",
"NetCDF", "Netscape", "Newmat", "NGPL", "Nokia", "NOSL", "Noweb", "OML",
"OpenLDAP", "OpenPBS", "OpenSSL", "OReilly", "OSL 1.0", "OSL 1.0+", "OSL 1.1",
"OSL 1.1+", "OSL 2.0", "OSL 2.0+", "OSL 2.1", "OSL 2.1+", "OSL 3.0", "OSL
3.0+", "Phorum", "PHP", "PlainTeX", "Plexus", "PostgreSQL", "psutils", "Public
Domain", "Python", "Qhull", "QPL", "Rdisc", "RiceBSD", "Romio", "RPSL",
"Ruby", "Saxpath", "SCEA", "SCRIP", "Sendmail", "Sleepycat", "SISSL", "SLIB",
"SNIA", "SPL", "TCL", "Teeworlds", "TMate", "TOSL", "TPL", "UCD", "Vim",
"VNLSL", "VOSTROM", "VSL", "W3C", "Webmin", "WTFPL", "wxWidgets", "Xerox",
"xinetd", "XSkat", "YPLv1.1", "Zend", "zlib", "zlib with acknowledgement",
"ZPLv1.0", "ZPLv1.0+", "ZPLv2.0", "ZPLv2.0+", "ZPLv2.1", "ZPLv2.1+", "CDL",
"FBSDDL", "GFDL", "IEEE", "LDPL", "OFSFDL", "Open Publication", "Public Use",
"CC-BY", "CC-BY-ND", "CC-BY-SA", "DMTF", "DSL", "EFML", "Free Art",
"GeoGratis", "Green OpenMusic", "OAL", "AMS", "Arphic", "Baekmuk", "Bitstream
Vera", "DoubleStroke", "Hershey", "IPA", "Liberation", "Lucida", "MgOpen",
"mplus", "OFL", "PTFL", "STIX", "Utopia", "Wadalab", "XANO", "Redistributable,
no modification permitted", "Freely redistributable without restriction".

perl-Apache-Htgroup.noarch: W: invalid-license GPL, see LICENSE
--skiped same as above--

1 packages and 0 specfiles checked; 0 errors, 2 warnings.

$ rpmlint -i SRPMS/perl-Apache-Htgroup-1.23-1.fc15.src.rpm 
--skiped same as rpmlint output for rpm file--

1 packages and 0 specfiles checked; 0 errors, 2 warnings.

=> it should have to be
| License:       GPLv2 or Artistic |

* MUST: The package must be named according to the Package Naming    [OK  ] 
Check rpmlint output for pynag-0.3-2.src.rpm

* MUST: The spec file for the package MUST be legible.               [OK  ] 

* Must: Spec file matches base package                               [OK   ] 

* Must: License must be licensed with a Fedora approved license and meet the
  Licensing Guidelines                                               [Fix  ]
=> Check rpmlint warnings

* Must: License in spec must match actual license                    [Fix  ] 
=> Check rpmlint warnings

* Must: License file included in %doc                                [OK   ]

* Must: Spec file written in American English                        [OK   ]

* Must: Tar ball matches upstream                                    [OK   ]

* Must: Package successfully builds binary RPMs                      [OK   ]
local build -(f14)
Koji build -(f15) http://koji.fedoraproject.org/koji/taskinfo?taskID=3014888

* Must: No duplicate files                                           [OK   ]

* Must: Macro use must be consistant                                 [OK   ]
"Apache-Htgroup" can be replaced by
1. define a macro at the top
|%define		realname  Apache-Htgroup|
2. use %{realname} in place of Apache-Htgroup

* Must: At the beginning of %install, each package MUST run rm -rf %{buildroot}
                                                                     [OK   ]

* Must: All filenames in rpm packages must be valid UTF-8            [OK   ]

* Other: %clean section and rm -fr $RPM_BUILD_ROOT are not required for F-13
and above. http://fedoraproject.org/wiki/Packaging/Guidelines#.25clean
Comment 6 Jason Tibbitts 2012-06-29 20:33:38 EDT
Not sure what's happening here.  The above comment has valid complaints but it's not exactly easy to find them.  It also has a bunch of bogus advice (not at all required for packages to rm the buildroot in %install, for example), so maybe it's better to just ignore it.

When I grab the above srpm and build it, I do get some rpmlint complaints about the license.  But the spec link above has a correct license tag.  It's important that you always provide a spec and the srpm that was built from it; if they don't match, it's just confusing and the last thing you want to do is confuse those who might review your package.

Can you provide a spec and srpm that reflect what you would like to have reviewed?
Comment 7 Steven Hadfield 2012-07-07 02:55:47 EDT
I apologize for the long delay. I have made the necessary changes to the RPM spec and have rebuilt the project on 16.

SRPM: https://docs.google.com/open?id=0B-X5qiDWVFeUdzhKbDNEd21oOEk
SPEC: https://docs.google.com/open?id=0B-X5qiDWVFeUUmlQU2xKTi01UTg

Everything should be in order with the exception of the license, which rpmlint reports as having an old address for the GPLv2 license. Until the upstream author adjusts this, there doesn't seem to be any more for me to do.
Comment 8 Parag AN(पराग) 2015-08-16 03:33:35 EDT
Please respond submitter if you still want to continue with this package submission in a week time.

Initiating https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews#Submitter_not_responding

Can't find FAS associated with this package submitter email.
Comment 9 Mamoru TASAKA 2015-08-16 04:02:35 EDT
Parag, why are you setting needinfo from the reporter although the last comment on this bug is from the "reporter"? What we must do is to respond to the comment 7, not asking the response from the reporter.
Comment 10 Mamoru TASAKA 2015-08-16 04:03:50 EDT
Also, it is reasonable that FE-NEEDSPONSOR people has no FAS.
Comment 11 Parag AN(पराग) 2015-08-16 05:01:37 EDT
I just asked a simple question if submitter is around? If he is, I will sponsor him. 

(In reply to Mamoru TASAKA from comment #10)
> Also, it is reasonable that FE-NEEDSPONSOR people has no FAS.

Sorry, but I really think FAS is MUST when you want to contribute a package in Fedora which is also documented on package review process wiki page by following https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Create_a_Fedora_Account

Anyways as you suggested here is my review for srpm submitted in comment#7

Package Review

[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed

- Package uses either %{buildroot} or $RPM_BUILD_ROOT
  Note: Using both %{buildroot} and $RPM_BUILD_ROOT
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros

==> I think this is self-explanatory how to fix this in spec file.

- 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 %license.
  Note: License file LICENSE is marked as %doc instead of %license

=> For this please move LICENSE file to %license macro just below %doc as
%license LICENSE

- Ask upstream to update fsf address in LICENSE file.

- Group tag is optional in spec, you can remove it.

===== MUST items =====

[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated". 1 files have unknown license. Detailed
     output of licensecheck in /home/parag/Downloads/perl-Apache-
[-]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[x]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by:
     /usr/share/perl5/vendor_perl/Apache(perl-Apache-DBI, ocsinventory-
     server, perl-Apache-LogRegex, perl-XMLRPC-Lite, perl-Apache-Session-
     LDAP, perl-Apache-Session-NoSQL, perl-Apache-DBI-Cache, perl-Apache-
     Session-Browseable, perl-SOAP-Lite, perl-Apache-LogFormat-Compiler,
     perl-Apache-Htpasswd, perl-Apache-RPC, perl-Apache-Session, perl-
     Frontier-RPC, perl-Maypole)

==> This is okay based on https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership

[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

=> Please remove this removal of buildroot line from spec as its not needed nowadays as rpm will take care of it.

[x]: Sources contain only permissible code or content.
[!]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed

=> Please remove this defattr line from spec as its not needed nowadays as rpm will take care of it.

[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[-]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 30720 bytes in 2 files.
[x]: Package complies to the Packaging Guidelines

=> Except the optional things now in spec is not removed.

[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).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[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]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[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
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

[x]: Package contains the mandatory BuildRequires and Requires:.
[x]: CPAN urls should be non-versioned.

===== SHOULD items =====

[!]: Buildroot is not present
     Note: Buildroot: present but not needed

=> Please remove this defattr line from spec as its not needed nowadays as rpm will take care of it.

[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[-]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
[!]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define realname Apache-Htgroup

=> As given above please use %global. See more on this https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Package has no %clean section with rm -rf %{buildroot} (or
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX is a working URL.

===== EXTRA items =====

[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).

Checking: perl-Apache-Htgroup-1.23-2.fc24.noarch.rpm
perl-Apache-Htgroup.noarch: E: incorrect-fsf-address /usr/share/doc/perl-Apache-Htgroup/LICENSE
2 packages and 0 specfiles checked; 1 errors, 0 warnings.

Rpmlint (installed packages)
sh: /usr/bin/python: No such file or directory
perl-Apache-Htgroup.noarch: W: invalid-url URL: http://search.cpan.org/dist/Apache-Htgroup/ <urlopen error [Errno -5] No address associated with hostname>
perl-Apache-Htgroup.noarch: E: incorrect-fsf-address /usr/share/doc/perl-Apache-Htgroup/LICENSE
1 packages and 0 specfiles checked; 1 errors, 1 warnings.

perl-Apache-Htgroup (rpmlib, GLIBC filtered):


Source checksums
http://search.cpan.org/CPAN/authors/id/R/RB/RBOW/Apache-Htgroup-1.23.tar.gz :
  CHECKSUM(SHA256) this package     : 915ddf0d60c7889f417646a6f998c264626336306b906e3a7e21d25eaafb986c
  CHECKSUM(SHA256) upstream package : 915ddf0d60c7889f417646a6f998c264626336306b906e3a7e21d25eaafb986c

Please fix the issues reported above and submit new update by updating release tag and adding relevant changelog entry.

Comment 12 Steven Hadfield 2015-08-18 11:44:39 EDT
Thank you for the suggested fixes. I've updated the specs (and pushed to a Github repo). As I recall, I asked the author about updating the license some years ago and never received a response. I can try again, but I'm not sure I'll get a response.

Spec: https://raw.githubusercontent.com/steven-hadfield/rpm-specs/master/SPECS/perl-Apache-Htgroup.spec
Srpm: https://github.com/steven-hadfield/rpm-specs/blob/master/SRPMS/perl-Apache-Htgroup-1.23-3.fc22.src.rpm

I have also signed up for a FAS account. Was there anything else I missed?
Comment 13 Parag AN(पराग) 2015-08-18 12:01:18 EDT
Wow! I never thought you would be around. I was almost prepared to close this after a week but its Mamoru who guided me correctly here.

As promised above I will sponsor you but you need to follow following things please.

We have this process  http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group to get sponsored into the packager group. Can you either submit few more packages and/or some full detailed package reviews? This is needed to make sure package submitter understands the rpm packaging well and follows the fedora packaging guidelines.

Please go through the following links
1) http://fedoraproject.org/wiki/Package_Review_Process

2) https://fedoraproject.org/wiki/PackagingGuidelines

3) To find the packages already submitted for review, check http://fedoraproject.org/PackageReviewStatus/

4) http://fedoraproject.org/wiki/Packaging:ReviewGuidelines and http://fedoraproject.org/wiki/Package_Review_Process#Reviewer is useful while doing package reviews.

5) https://fedorahosted.org/FedoraReview/ this is fedora-review tool to help review packages in fedora. You need to use this and do un-official package reviews of packages submitted by other contributors. While doing so mention "This is un-official review of the package." at top of your review comment.

Good to review packages listed in http://fedoraproject.org/PackageReviewStatus/NEW.html

When you do full package review of some packages, provide that review comment link here so that I can look how you have reviewed those packages.

If you got any questions please ask :)
Comment 14 Parag AN(पराग) 2015-08-18 12:21:24 EDT
you can also use fedora-review on this bugzilla but it will not work as you gave srpm link which is not direct downloadable. Let me paste here your package links again.

Spec: https://raw.githubusercontent.com/steven-hadfield/rpm-specs/master/SPECS/perl-Apache-Htgroup.spec
Srpm: https://raw.githubusercontent.com/steven-hadfield/rpm-specs/master/SRPMS/perl-Apache-Htgroup-1.23-3.fc22.src.rpm

Here is how you can use it
fedora-review -b 673585 -m fedora-rawhide-x86_64
Comment 15 Parag AN(पराग) 2015-08-18 12:57:16 EDT
Here is my review for your -3 release srpm 

1)I see you have used Buildroot tag which is also become optional and not needed. Can you remove it and upload again spec and srpm? No need to bump the release tag. Also, fedora-review output review.txt showed mismatch of spec given by spec url and spec file inside srpm as

Diff spec file in url and in SRPM
--- /home/parag/673585-perl-Apache-Htgroup/srpm/perl-Apache-Htgroup.spec        2015-08-18 21:51:58.536356400 +0530
+++ /home/parag/673585-perl-Apache-Htgroup/srpm-unpacked/perl-Apache-Htgroup.spec       2015-08-18 20:38:27.000000000 +0530
@@ -35,6 +35,6 @@

-%doc README
 %license LICENSE
+%doc README
@@ -42,5 +42,5 @@
 * Tue Aug 18 2015 Steven Hadfield <hadfieldster@gmail.com> 1.23-3
-- RPM spec cleanup
+- RPM spec fixes

 * Fri Jul 6 2012 Steven Hadfield <StevenHadfield@letu.edu> 1.23-2

2) It is okay if there is no response from upstream on fixing fsf address issue.

3) Start reviewing other people's packages and once you did full package review like I did for you in comment#11 here, post that review link in this bugzilla to let me know that you are reviewing and your reviews are good where you have pointed some issues with how to fix them.
Comment 16 Parag AN(पराग) 2015-09-23 02:25:11 EDT
Any updates here?

Can you submit updated spec and srpm?

Can you provide links of reviews done by you for other people's packages?
Comment 17 Steven Hadfield 2015-10-02 11:52:37 EDT
I apologize for not updating sooner. I did update the SPEC and SRPM.

I commented on package request for 1174346 and started a review for 1196037

Note You need to log in before you can comment on or make changes to this bug.