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 185407
Summary: | Review Request: pwgen - Automatic password generation | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | James Bowes <jbowes> |
Component: | Package Review | Assignee: | Thorsten Leemhuis (ignored mailbox) <bugzilla-sink> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | bkearney, jose.p.oliveira.oss, pertusus |
Target Milestone: | --- | Flags: | kevin:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-04-03 16:21:11 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 |
Description
James Bowes
2006-03-14 15:06:35 UTC
I cannot approve, as you are seeking a sponsor, but here are my comments. The Source0 is wrong, as it should be un URL, like Source0: http://dl.sourceforge.net/sourceforge/pwgen/pwgen-%{version}.tar.gz rpmlint gives the following warnings, that should be easily sorted out: W: pwgen strange-permission pwgen-2.05.tar.gz 0755 W: pwgen strange-permission pwgen.spec 0755 Those 2 items are blockers. The remaining are comments. Have you verified that the pending patch ahs been merged? (at a quick glance it seems so) http://sourceforge.net/tracker/index.php?func=detail&aid=1108220&group_id=28391&atid=393206 I personnally prefer to use globs for man pages extensions such that rpmbuild picks the man page whatever compression scheme is used (even none). The corresponding entry in files becomes: %{_mandir}/man1/pwgen.1* Otherwise everything seems ok. Locations for updated files: Spec Url: http://flame.cs.dal.ca/~bowes/packages/pwgen/rev2/pwgen.spec SRPM Url: http://flame.cs.dal.ca/~bowes/packages/pwgen/rev2/pwgen-2.05-2.src.rpm (In reply to comment #1) > I cannot approve, as you are seeking a sponsor, but here are my comments. > > The Source0 is wrong, as it should be un URL, like > > Source0: http://dl.sourceforge.net/sourceforge/pwgen/pwgen-%{version}.tar.gz > Fixed. > rpmlint gives the following warnings, that should be easily sorted out: > > W: pwgen strange-permission pwgen-2.05.tar.gz 0755 > W: pwgen strange-permission pwgen.spec 0755 > > Those 2 items are blockers. > Fixed. I have no idea how that happened in the first place... > The remaining are comments. > > Have you verified that the pending patch ahs been merged? (at a quick glance it > seems so) > http://sourceforge.net/tracker/index.php?func=detail&aid=1108220&group_id=28391&atid=393206 > From what I can tell, it has been. > I personnally prefer to use globs for man pages extensions such that rpmbuild > picks the man page whatever compression scheme is used (even none). The > corresponding entry in files becomes: > > %{_mandir}/man1/pwgen.1* > Ah, good point. That is now fixed. > > Otherwise everything seems ok. Thanks for the feedback! James, A couple more notes: * the dist tag is missing; the release entry should be something like: Release: 2%{?dist} * [BLOCKER] don't strip the binary; rpmbuild will do that for you (when it tries to create the debuginfo RPM) just delete the line "strip $RPM_BUILD_ROOT/%{_bindir}/pwgen" jpo Locations for updated files: Spec Url: http://flame.cs.dal.ca/~bowes/packages/pwgen/rev3/pwgen.spec SRPM Url: http://flame.cs.dal.ca/~bowes/packages/pwgen/rev3/pwgen-2.05-3.src.rpm (In reply to comment #3) > James, > > A couple more notes: > > * the dist tag is missing; > the release entry should be something like: > > Release: 2%{?dist} > Fixed. > * [BLOCKER] don't strip the binary; > rpmbuild will do that for you (when it tries to create the debuginfo RPM) > > just delete the line "strip $RPM_BUILD_ROOT/%{_bindir}/pwgen" > Fixed. > jpo > Thanks for the feedback! PUBLISH++ MD5SUMS: 0b6a48c327fb103d634486c4a64052f0 pwgen-2.05-3.src.rpm b94546a346cb352054ea2d3d09f7f885 pwgen-2.05.tar.gz 4aaddffd833922b16b61c06baecbab1d pwgen.spec Good: * Source tarball MD5 digest verified * URL and Source URL are valid * License verified (only mentioned in http://sourceforge.net/projects/pwgen/) * RPM_OPT_FLAGS are present during the building step * File permissions are valid (rpm -qplv ...) * Builds without problems in FC-3, FC-4, and FC-5 Note: This is my vote for approval. The final word belongs to Patrice as he has done the first review. (In reply to comment #2) > > %{_mandir}/man1/pwgen.1* > > > > Ah, good point. That is now fixed. This still isn't a blocker, but the way you made the glob, the man page won't be taken if there is no compression. Indeed in that case, the file name will be %{_mandir}/man1/pwgen.1 which isn't taken by your glob %{_mandir}/man1/pwgen.1.* and that's why I advertise %{_mandir}/man1/pwgen.1* (In reply to comment #5) > This is my vote for approval. The final word belongs to Patrice as he has done > the first review. I interpret that you are sponsoring James in that case... APPROVED There is only a minor issue, I think the changelog is more readable if there is an empty line between release fields, like * Sat Mar 25 2006 James Bowes <jbowes> - 2.05-3 - Add dist tag to release. - Don't strip binary, since rpmbuild will do it. * Fri Mar 24 2006 James Bowes <jbowes> - 2.05-2 - Use url for Source0 in spec file. - Use glob for man page extension. - Increment release number. And also you can remove the line - Increment release number. as it is redundant with having a new changelog entry. No need to add a changelog entry for those changes, nor if you change the glob for man pages, and you can do that after importing in the cvs. Thanks for all the help! Moved blocker to FE-ACCEPT. This bug should be assigned to jpo, I believe. Package Change Request ====================== Package Name: pwgen New Branches: EL-4 EL-5 cvs done. |