Bug 1046959 - Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys
Summary: Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to sec...
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Meng
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1045849
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-12-27 18:11 UTC by Michael S.
Modified: 2018-05-28 12:52 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
i: fedora-review?


Attachments (Terms of Use)

Description Michael S. 2013-12-27 18:11:15 UTC
Spec URL: http://www.zarb.org/~misc/tmp/simple-tpm-pk11.spec
SRPM URL: http://www.zarb.org/~misc/tmp/simple-tpm-pk11-0.01-1.fc20.src.rpm
Description: A simple tool for using the TPM chip to secure SSH keys.
Fedora Account System Username: misc


However, currently, the package do not build in mock due to bug 1045849

Comment 1 Christopher Meng 2013-12-27 18:23:37 UTC
Macros should be enclosed by braces.

Comment 2 Michael S. 2014-02-24 18:27:01 UTC
And so, besides the macros that should be enclosed, is there any problem to fix ?

Comment 3 Christopher Meng 2014-02-25 03:29:46 UTC
(In reply to Michael Scherer from comment #2)
> And so, besides the macros that should be enclosed, is there any problem to
> fix ?

I haven't run the fedora-review because the review doesn't pass first on SPEC itself. Also 0.02 is available, please update the package, then it's possible for me to review.

And, %configure will insert the cflags, why did you insert them again in

make CFLAGS="$RPM_OPT_FLAGS" %{?_smp_mflags}?

%check section not usable, because there is a bug in gtest package? If so, where is the bug report?

Comment 4 Michael S. 2014-02-26 21:08:31 UTC
1) I fail to understand what you mean by "doesn't pass first on SPEC itself", can you clarify ?

I will post a updated with 0.0.2 shortly, didn't see there was a new version since I posted it.

2) for %configure, likely a wrong cut and paste, will remove it in the nextiteration.


3) I didn't report a bug, because this is not a bug.
%check is not usable because it requires the source code of gtest directly, ie it likly use a non public API by using internals of gtest.cc. And so asking to gtest to distribute the code as part of the rpm would likely make others people use it ( so make it public while it likely shouldn't ), which will them be a hack and quite fragile. I will not ask the package to carry a non public API just for a test.

Comment 5 Christopher Meng 2014-02-27 02:35:27 UTC
(In reply to Michael Scherer from comment #4)
> 1) I fail to understand what you mean by "doesn't pass first on SPEC
> itself", can you clarify ?

I review packages first from SPEC, if spec is not fine, the review of course will generate many problems. SPEC should be bascially correct.

> I will post a updated with 0.0.2 shortly, didn't see there was a new version
> since I posted it. 

Fine.

> 2) for %configure, likely a wrong cut and paste, will remove it in the
> nextiteration.

Fine.

> 3) I didn't report a bug, because this is not a bug.
> %check is not usable because it requires the source code of gtest directly,
> ie it likly use a non public API by using internals of gtest.cc. And so
> asking to gtest to distribute the code as part of the rpm would likely make
> others people use it ( so make it public while it likely shouldn't ), which
> will them be a hack and quite fragile. I will not ask the package to carry a
> non public API just for a test.

Yes, but you should mention this in the initial comment so that we won't waste time here.

Nice to see you have time, so update the bug and I will review it formally.

Comment 6 Christopher Meng 2014-07-12 16:36:59 UTC
0.03 is out.

Comment 8 Christopher Meng 2014-07-14 01:02:30 UTC
changelog invalid. Please add the 0.03 changelog.

Please brackets: %_bindir -> %{_bindir}

Source0 suggestion:

https://github.com/ThomasHabets/simple-tpm-pk11/archive/%{version}.tar.gz#/%{name}-%{version}.tar.gz

Comment 9 Upstream Release Monitoring 2015-12-12 19:39:07 UTC
puiterwijk's scratch build of simple-tpm-pk11-0.04-1.fc22.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12165732

Comment 10 Patrick Uiterwijk 2015-12-12 19:42:54 UTC
I just found this review after packaging it myself as well.

Michael: if you wnat a new (fixed) spec and SRPM:
SPEC: https://puiterwijk.fedorapeople.org/simple-tpm-pk11.spec
SRPM: https://puiterwijk.fedorapeople.org/simple-tpm-pk11-0.04-1.fc22.src.rpm

Comment 11 Pierre-YvesChibon 2015-12-17 16:30:15 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/simple-tpm-pk11

Comment 12 Pierre-YvesChibon 2015-12-17 16:38:40 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/simple-tpm-pk11

Comment 13 Pierre-YvesChibon 2015-12-17 16:48:33 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/simple-tpm-pk11

Comment 14 Patrick Uiterwijk 2015-12-21 20:27:53 UTC
Note: those approval messages were because of a test with pkgdb. This package still needs review.

Comment 15 Nicolas Chauvet (kwizart) 2018-05-28 12:52:59 UTC
Hello, anyone still interested in this package ?

I've a tpm device I can test on and I would like to review this package.
Best would be to make a new review with an updated spec file if possible.
Then please make me assigned for the review.


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