https://bugzilla.redhat.com/show_bug.cgi?id=1012391
Bug ID: 1012391 Summary: Review Request: makepasswd - Generates (pseudo-)random passwords of a desired length Product: Fedora Version: rawhide Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: kupo@kupo.se QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org
Spec URL: http://kupo.se/pub/makepasswd/makepasswd.spec SRPM URL: http://kupo.se/pub/makepasswd/makepasswd-0.5.1-1.fc18.src.rpm Description: Makepasswd generates (pseudo-)random passwords of a desired length. It is available under the GPL version 3.
Fedora Account System Username: opuk Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5986644
This is my first package submitted for Fedora and I'm in need of a sponsor.
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
Johan Swensson kupo@kupo.se changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cickumqt@gmail.com
--- Comment #1 from Christopher Meng cickumqt@gmail.com --- Informal review:
1. Source0: http://people.defora.org/~khorben/projects/makepasswd/makepasswd-0.5.1.tar.g...
You should use:
Source0: http://people.defora.org/~khorben/projects/makepasswd/%%7Bname%7D-%%7Bversio...
So when you update it to newer version you won't be tired of handling URL again.
2. Use macro:
/usr/bin --> %{_bindir}
/usr/share/man/man1 --> %{_mandir}/man1
3. install -m 755 src/makepasswd %{buildroot}/usr/bin install -m 644 doc/makepasswd.1 %{buildroot}/%{_mandir}/man1
You forgot to preserve the timestamp, guidelines tell us that it's a MUST.
4. %defattr(-, root, root -) is not needed now.
5. Don't mark manpages as %doc.
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
--- Comment #2 from Johan Swensson kupo@kupo.se --- Thank you Christopher. I have made changes accordingly.
SPEC: http://kupo.se/pub/makepasswd/makepasswd-2.spec SRPM: http://kupo.se/pub/makepasswd/makepasswd-0.5.1-2.fc18.src.rpm Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5986644
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
Terje Røsten terjeros@phys.ntnu.no changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|package-review@lists.fedora | |project.org | CC| |terjeros@phys.ntnu.no
--- Comment #3 from Terje Røsten terjeros@phys.ntnu.no ---
Don't version the file name, use makepasswd.spec always.
Source0: http://people.defora.org/~khorben/projects/makepasswd/%%7Bname%7D-%%7Bversio...
You can drop %{name} macro, the important thing is %{version}
Patch0: db2man.sh.diff
Indent correctly
BuildRequires: docbook-style-xsl libxslt
Use only one buildreq per line.
%description Makepasswd generates (pseudo-)random passwords of a desired length. It is available under the GPL version 3.
License info not needed in %description
cd doc
I prefer pushd doc
# make install DESTDIR=$RPM_BUILD_ROOT/usr/bin
Remove.
%{_mandir}/man1/makepasswd.1.gz
Drop .gz, just use %{_mandir}/man1/makepasswd.1* (conpression from rpmbuild might change).
Any license files in %files? Use %doc macro.
%changelog * Mon Sep 26 2013 Johan Swensson kupo@kupo.se 0.5.1-2 - package fixes with input from informal review
Insert empty line here.
* Mon Sep 26 2013 Johan Swensson kupo@kupo.se 0.5.1-1 - initial build
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5986644 From build log:
cc -W -Wall -g -O2 -pedantic -c makepasswd.c cc -W -Wall -g -O2 -pedantic -c md5c.c cc -o makepasswd makepasswd.o md5c.o -lcrypt
Correct build flags is not used, please fix.
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
--- Comment #27 from Johan Swensson kupo@kupo.se --- It now uses openssl's implementation of md5 instead of a bundled one.
SPEC: http://kupo.se/pub/makepasswd/makepasswd.spec SRPM: http://kupo.se/pub/makepasswd/makepasswd-0.5.3-1.fc19.src.rpm Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=6320380
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
Michael Schwendt bugs.michael@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #28 from Michael Schwendt bugs.michael@gmx.net --- * Indeed, the 25K diff between 0.5.2 and 0.5.3 drops the bundled MD5.
* fedora-review complains about missing parallel make macro, but that's only because of the extra "doc" make target. That's acceptable. Even if the smp flags were dropped from the first make invocation in %build, that would be acceptable IMO, because there's not much to build.
* Related to the "doc" target, a closer look reveals that the top Makefile builds the "doc" subdir target via target "all" already, so the explicit
pushd doc make
in %build is superfluous. rpmdiff suggests that you've readded that step after the -6 spec release.
* Options -e blowfish and -e sha1 are rejected. The man page comments on that, so I only point it out.
* No blockers that would justify requesting another update for review. You may fix the minor "make" issue in dist git.
APPROVED
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
Johan Swensson kupo@kupo.se changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #29 from Johan Swensson kupo@kupo.se --- New Package SCM Request ======================= Package Name: makepasswd Short Description: Generates (pseudo-)random passwords of a desired length Owners: opuk Branches: f19 f20
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
--- Comment #30 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=1012391
Michael Schwendt bugs.michael@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |RAWHIDE Last Closed| |2014-01-03 16:04:26
package-review@lists.fedoraproject.org