Bug 557776
Summary: | Review Request: libyubikey - Library for parsing / decrypting Yubikey passwords | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Maxim Burgerhout <maxim> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | fedora-package-review, mads, mtasaka, notting, zbe64533 |
Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
tcallawa: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 1.5-4.fc12 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-01-27 20:03:57 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: | 557794 |
Description
Maxim Burgerhout
2010-01-22 15:09:08 UTC
Hello. Here are some note: (1) Documentation - "rpmlint" says "libyubikey-devel.i686: W: no-documentation". - Pay attention about which subpackage you include documentation in, for example API documentation belongs in the -devel subpackage, not the main one. * http://fedoraproject.org/wiki/Packaging/Guidelines#Documentation (2) Compiler flags - Compilers used to build packages should honor the applicable compiler flags set in the system rpm configuration. * http://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags - $RPM_OPT_FLAGS or %{optflags} for C, C++, and Fortran compilers. (3) Timestamps - Please keep timestamps on installed files as much as possible: * http://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps - When using "cp" or "install" commands add "-p" option - Also please consider to add 'INSTALLFLAGS="-c -p"' to "make install" (4) URL - The URL and the Source0 is old (redirected), please keep the latest URL. * "http://code.google.com/p/yubico-c/" (5) binary files - If the software being packaged contains files intended solely for development, those files should be put in a -devel subpackage. * http://fedoraproject.org/wiki/Packaging/Guidelines#Devel_Packages o %{_bindir}/modhex o %{_bindir}/ykdebug (6) pkgconfig file - This library does not have pkgconfig file(.pc), so "BuildRequires: pkgconfig" is needless. Thanks for looking at this, Akio. I went over your notes: 1. Correct, but as modhex and ykdebug went to -devel, so did README (which is about those two files for the largest part). So -devel now has a %doc section. 2. Fixed. 3. Looked into this. I redid the download so it's timestamp matches upstream. However, the Makefile doesn't seem to handle INSTALLFLAGS or pass it to install-sh, but the timestamps of installed files are intact, afaics. I don't think the install part needs fixing. Please correct me if I'm wrong; I really want to learn this. 4. Fixed. 5. Moved them to -devel. I agree that is a better place for those binaries. 6. Fixed. Missed that. New versions at: http://dl.dropbox.com/u/284093/update-1/libyubikey.spec http://dl.dropbox.com/u/284093/update-1/libyubikey-1.5-3.fc12.src.rpm Some notes for 1.5-3 * Compilar flags - "CFLAGS="$RPM_OPT_FLAGS"" in "make %{?_smp_mflags}" is not needed. configure generated by recent autotools acknowledges CFLAGS environment and %configure sets this. Please check what %configure actually does by $ rpm --eval %configure . * Timestamp - Please consider to use -------------------------------------------------------------------- %{__make} install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p" -------------------------------------------------------------------- to keep timestamps on installed files. This method usually works for Makefiles generated by recent autotools. * %defattr - Now we prefer to use %defattr(-,root,root,-) * File placement - As the Summary of the main package already says "low-level C software *development* kit", I don't see strict necessity for putting two binaries into -devel package. - Also "README" should usually be packaged in main package. * %changelog - It is recommended to put one line between each %changelog entry like below because it is useful on Fedora CVS --------------------------------------------------------------------- * Sun Jan 24 2010 - Maxim Burgerhout <maxim> - 1.5-3 - Took out the dep on libusb1-devel - Moved README doc to -devel: it's mostly about ykdebug and modhex * Sun Jan 24 2010 - Maxim Burgerhout <maxim> - 1.5-2 - Used macros in Source0 - URL no longer point to redirect - Removed INSTALL from documentation - Moved modhex and ykdebug to -devel - Inserted compilerflags - Inserted INSTALLFLAGS to keep timestamps - Some more macros for make and sed * Wed Jan 20 2010 - Maxim Burgerhout <maxim> - 1.5-1 - First packaged release --------------------------------------------------------------------- Mamoru, thank you for looking into this package. Timestamps: done. Compilerflags: reverted. Defattr: done. Changelog: done. File placement: reverted. Updated files: - http://dl.dropbox.com/u/284093/update-2/libyubikey.spec - http://dl.dropbox.com/u/284093/update-2/libyubikey-1.5-4.fc12.src.rpm Sorry. The above review comment was my FIRST one and it introduced the confusion.... Mamoru-san: I have one question about rpmlint. It still says "libyubikey-devel.i686: W: no-documentation". Could we simply ignore this message? (In reply to comment #5) > Mamoru-san: > I have one question about rpmlint. > It still says "libyubikey-devel.i686: W: no-documentation". > Could we simply ignore this message? If the needed documents are already installed in one of the rpms the rpm depens on, we ignore this warning. Now - This package can be approved. - You have another review requests, which is in good shape to some extent. ------------------------------------------------------ This package (libyubikey) is APPROVED by mtasaka ------------------------------------------------------ Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Get a Fedora Account". After you request for sponsorship a mail will be sent to sponsor members automatically (which is invisible for you) which notifies that you need a sponsor. After that, please also write on this bug for confirmation that you requested for sponsorship and your FAS (Fedora Account System) name. Then I will sponsor you. If you want to import this package into Fedora 11/12, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). If you have questions, please ask me. Mamoru, I have requested membership. My FAS name is wzzrd. Now I am sponsoring you. Please follow "Join" wiki again. Thanks! Here's the CVS request: New Package CVS Request ======================= Package Name: libyubikey Short Description: C library for decrypting and parsing Yubikey One-time passwords Owners: wzzrd Branches: F-12 InitialCC: CVS done (by process-cvs-requests.py). Please actually build this package on koji (not by scratch build) and for F-12 please submit push requests on bodhi. Will do. CVS import gave me a hard time this morning, so as soon as I have that working, I'm building. The scratch builds were just me checking out koji. libyubikey-1.5-4.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/libyubikey-1.5-4.fc12 Submitted to Bodhi. Thanks, Mamoru! Closing. libyubikey-1.5-4.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. Package Change Request ====================== Package Name: libyubikey New Branches: el5 el6 Owners: wzzrd Recent builds of fedora-packager require ykpers (and therefore libyubikey); fedora-packager availability in EPEL requires availability of ykpers and libyubikey in EPEL, too. Git done (by process-git-requests). |