[Bug 1046959] New: Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

Bug ID: 1046959
           Summary: Review Request: simple-tpm-pk11 - A simple tool for
                    using the TPM chip to secure SSH keys
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]

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

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

Michael Scherer  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends On|                            |1045849

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1045849
[Bug 1045849] do not create /run/lock when used with mock

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

Christopher Meng  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

--- Comment #1 from Christopher Meng  ---
Macros should be enclosed by braces.

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959
Bug 1046959 depends on bug 1045849, which changed state.

Bug 1045849 Summary: do not create /run/lock when used with mock
https://bugzilla.redhat.com/show_bug.cgi?id=1045849

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #2 from Michael Scherer  ---
And so, besides the macros that should be enclosed, is there any problem to fix
?

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #3 from Christopher Meng  ---
(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?

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #4 from Michael Scherer  ---
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.

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #5 from Christopher Meng  ---
(In reply to Michael Scherer from comment #4)
> 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.
> for %configure, likely a wrong cut and paste, will remove it in the
>
> nextiteration.

Fine.
> 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.

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #6 from Christopher Meng  ---
0.03 is out.

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #7 from Michael Scherer  ---
Spec URL: http://www.zarb.org/~misc/tmp/simple-tpm-pk11.spec
SRPM URL: http://www.zarb.org/~misc/tmp/simple-tpm-pk11-0.03-1.fc22.src.rpm

Koji scratch build :

http://koji.fedoraproject.org/koji/taskinfo?taskID=7135050

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #8 from Christopher Meng  ---
changelog invalid. Please add the 0.03 changelog.

Please brackets: %_bindir -> %{_bindir}

Source0 suggestion:

https://github.com/ThomasHabets/simple-tpm-pk11/archive/%%7Bversion%7D.tar.g...

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #9 from Upstream Release Monitoring  ---
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

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

Patrick Uiterwijk  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #10 from Patrick Uiterwijk  ---
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

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #11 from Pierre-YvesChibon  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/simple-tpm-pk11

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #12 from Pierre-YvesChibon  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/simple-tpm-pk11

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #13 from Pierre-YvesChibon  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/simple-tpm-pk11

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #14 from Patrick Uiterwijk  ---
Note: those approval messages were because of a test with pkgdb. This package
still needs review.

[Bug 1046959] Review Request: simple-tpm-pk11 - A simple tool for using the TPM chip to secure SSH keys

https://bugzilla.redhat.com/show_bug.cgi?id=1046959

--- Comment #15 from Nicolas Chauvet (kwizart)  ---
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.