Bug 394941 - Review Request: tpm-tools - TPM management utilities
Review Request: tpm-tools - TPM management utilities
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On: 323441 426152
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-21 15:53 EST by Kent Yoder
Modified: 2008-05-06 12:17 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-02 11:25:56 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Kent Yoder 2007-11-21 15:53:09 EST
Spec URL: http://trousers.sourceforge.net/fedora_tools_submit1/tpm-tools.spec

SRPM URL: http://trousers.sourceforge.net/fedora_tools_submit1/tpm-tools-1.3.1-1.src.rpm

Description: tpm-tools provides a command-line interface to TPM administration, with utilities to take ownership, enable and disable and view properties of your TPM. tpm-tools requires the trousers package, submitted for review here: https://bugzilla.redhat.com/show_bug.cgi?id=323441.
Comment 1 Mamoru TASAKA 2007-12-13 02:03:00 EST
Would you update your spec file with referring to
http://fedoraproject.org/wiki/Packaging/Guidelines
and my comments in bug 323441?
Comment 2 Kent Yoder 2007-12-13 14:45:51 EST
Latest version up for review is available here:

http://trousers.sourceforge.net/fedora_tools_submit2/

Thanks,
Kent
Comment 3 Mamoru TASAKA 2007-12-14 10:23:01 EST
For 1.3.1-2:

* SourceURL
  - Perhaps the correct one is
    http://downloads.sourceforge.net/trousers/%{name}-%{version}.tar.gz

* Redundant Requires
  A. "Requires: trousers" is not needed
     - rpmbuild checks the dependency for libraries. For this package
       tpm-tools binary rpm requires libtspi.so.1 and this dependency
       should pull trousers automatically.
  B. Requires(post/postun):   /sbin/ldconfig
     For this package this is not needed because
-------------------------------------------------------------
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
-------------------------------------------------------------
     automatically adds these dependencies.

? configure option
  - (This is a question)
    Would you explain why you want "--disable-pkcs11-support"?

* Directory ownership issue
--------------------------------------------------------------
%files devel
%{_includedir}/tpm_tools/*.h
--------------------------------------------------------------
  - This causes
--------------------------------------------------------------
[tasaka1@localhost ~]$ LANG=C rpm -qf /usr/include/tpm_tools/
file /usr/include/tpm_tools is not owned by any package
--------------------------------------------------------------
Comment 4 Kent Yoder 2007-12-14 14:01:05 EST
(In reply to comment #3)
> For 1.3.1-2:
> 
> * SourceURL
>   - Perhaps the correct one is
>     http://downloads.sourceforge.net/trousers/%{name}-%{version}.tar.gz

  Yep, I'll fix.
 
> * Redundant Requires
>   A. "Requires: trousers" is not needed
>      - rpmbuild checks the dependency for libraries. For this package
>        tpm-tools binary rpm requires libtspi.so.1 and this dependency
>        should pull trousers automatically.

 ok

>   B. Requires(post/postun):   /sbin/ldconfig
>      For this package this is not needed because
> -------------------------------------------------------------
> %post -p /sbin/ldconfig
> %postun -p /sbin/ldconfig
> -------------------------------------------------------------
>      automatically adds these dependencies.

ok
 
> ? configure option
>   - (This is a question)
>     Would you explain why you want "--disable-pkcs11-support"?

  pkcs11 support requires the opencryptoki package, which hasn't been submitted.
If at some future time we submit that, we could enable this support.
 
> * Directory ownership issue
> --------------------------------------------------------------
> %files devel
> %{_includedir}/tpm_tools/*.h
> --------------------------------------------------------------
>   - This causes
> --------------------------------------------------------------
> [tasaka1@localhost ~]$ LANG=C rpm -qf /usr/include/tpm_tools/
> file /usr/include/tpm_tools is not owned by any package
> --------------------------------------------------------------

  Ok, I'll add /usr/include/tpm_tools as I did in the trousers spec file.

Kent
Comment 5 Kent Yoder 2007-12-14 14:13:54 EST
Latest re-spin is here:

http://trousers.sourceforge.net/fedora_tools_submit3/

Thanks,
Kent
Comment 6 Mamoru TASAKA 2007-12-18 11:46:46 EST
For 1.3.1-3:

* mixed-use-of-spaces-and-tabs rpmlint
------------------------------------------------------
$ rpmlint -I mixed-use-of-spaces-and-tabs
mixed-use-of-spaces-and-tabs :
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.
------------------------------------------------------

* Unneeded BuildRequires
  - "BuildRequires: autoconf automake libtool" are not needed
     as this spec file acutally does not use any autotools

* Macros in %changelog
  - When to write macros in %changelog, please use %% to stop
    macros from being expanded.
-------------------------------------------------------
[tasaka1@localhost tpm-tools]$ rpmlint *-3*src*rpm
tpm-tools.src:79: W: macro-in-%changelog build
tpm-tools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 12)
tpm-tools.src:79: W: macro-in-%changelog build
tpm-tools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 12)
-------------------------------------------------------

? library linking
  - tpm-tools contains the library libtpm_unseal.so.0, however
    no binaries in tpm-tools are using this library??
-------------------------------------------------------
$ rpm -ql tpm-tools | grep '/usr/s*bin' | xargs ldd -r | grep -v '^/usr' | sort
| uniq | grep tpm_unseal
(no returns)
-------------------------------------------------------
Comment 7 Kent Yoder 2007-12-18 12:29:05 EST
Thanks Mamoru, latest re-spin is here:

http://trousers.sourceforge.net/fedora_tools_submit4/

Kent
Comment 8 Kent Yoder 2007-12-18 12:32:27 EST
Also - no apps use the library because its provided for developers...

Kent
Comment 9 Mamoru TASAKA 2007-12-18 13:05:49 EST
Okay.

---------------------------------------------------------
   This package (tpm-tools) is APPROVED by me
---------------------------------------------------------

! Note
  To rebuild this package on koji, you have to rebuild trousers
  first. On dist-f9 koji tag, it takes ~2 hours to get the rebuilt
  rpms imported into new buildroot (what I mean is that
  you will probably have to wait about 2 hours after trousers is
  rebuilt to rebuild this package).

  Or you can use "chain build" method.
Comment 10 Kent Yoder 2007-12-18 15:00:27 EST
Hi Mamoru,

  It looks like I need to go open a review request for openCryptoki, then modify
the tpm-tools specfile to build the pkcs11 stuff.  RHIT#184786 is for submission
of openCryptoki, so it should be going through this process too.  I'll add a
dependency here when the new openCryptoki review request is opened.

Kent
Comment 11 Kent Yoder 2007-12-18 15:48:13 EST
Hmm, looks like entering a new dependency wipes out the old one. Is there a way
to make this bug depend on openCryptoki and trousers?

Thanks,
Kent
Comment 12 Jason Tibbitts 2007-12-18 15:58:15 EST
Just enter more than one bug number to depend on.  Comma separated works fine;
space separated probably works as well.
Comment 13 Mamoru TASAKA 2007-12-18 21:52:05 EST
Is supporting openCryptoki _definitely_ needed before importing
this package into Fedora? If not, please import this package
as it is and when openCryptoki review request (if you submit it) is passed
modify tpm-tools package.
Comment 14 Kent Yoder 2007-12-19 10:50:46 EST
Hi Mamoru,

  As long as openCryptoki gets in at some point, I'm ok.  I'll go ahead and
import tpm-tools while the openCryptoki request is in progress...

Kent
Comment 15 Kent Yoder 2007-12-19 10:54:23 EST
New Package CVS Request
=======================
Package Name: tpm-tools
Short Description: TPM management utilities
Owners: key
Branches: F-7 F-8 EL-5
InitialCC: 
Cvsextras Commits: yes
Comment 16 Tom "spot" Callaway 2007-12-19 15:42:18 EST
cvs done
Comment 17 Mamoru TASAKA 2007-12-21 02:05:54 EST
Please close this bug when rebuild and request on bodhi is done.
Comment 18 Mamoru TASAKA 2008-01-02 11:25:56 EST
Closing for now as this is already in devel.
Comment 19 Emily Ratliff 2008-05-05 13:23:10 EDT
Package Change Request
======================
Package Name: tpm-tools
Updated Fedora Owners: key,ejratl

Comment 20 Kent Yoder 2008-05-05 13:24:27 EDT
Ack'd co-maintainership by ejratl.
Comment 21 Kevin Fenzi 2008-05-06 12:17:53 EDT
cvs done.

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