rpmlint:
> tboot.src: W: spelling-error %description -l en_US pre -> per, ore, pee
False positive.

> tboot.src: E: description-line-too-long C Trusted Boot (tboot) is an open source, pre-kernel/VMM module that uses Intel(R) Trusted Execution Technology (Intel(R) TXT) to perform a measured and verified launch of an OS kernel/VMM.
Please fix.

> tboot.x86_64: W: incoherent-version-in-changelog 20100401-1 ['20100427-1.fc12', '20100427-1']
Please fix.

> tboot.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/tboot-20100427/README
Not that important IMHO, but if you can fix it...


Licensing incompatibility: AFAICS tboot/* includes code under GPL by various copyright holders (including the FSF and Linus Toarvalds), and common/sha1.c, which are incompatible; therefore the compiled binary can not be distributed.

(Stopping the review here, for now.)

A new SRPM that fixes the identified errors (and is built on Fedora 13) and does not have any licensing issues is available at: https://sourceforge.net/projects/tboot/files/tboot/tboot-20100826-1.fc13.src.rpm/download

rpmlint:
> tboot.x86_64: W: spelling-error %description -l en_US pre -> per, ore, pee
False positive.

> tboot.x86_64: W: no-manual-page-for-binary lcp_crtpol
> tboot.x86_64: W: no-manual-page-for-binary acminfo
> tboot.x86_64: W: no-manual-page-for-binary lcp_crtpol2
> tboot.x86_64: W: no-manual-page-for-binary lcp_crtpconf
> tboot.x86_64: W: no-manual-page-for-binary tpmnv_lock
> tboot.x86_64: W: no-manual-page-for-binary parse_err
> tboot.x86_64: W: no-manual-page-for-binary tpmnv_relindex
> tboot.x86_64: W: no-manual-page-for-binary lcp_readpol
> tboot.x86_64: W: no-manual-page-for-binary lcp_mlehash
> tboot.x86_64: W: no-manual-page-for-binary lcp_crtpollist
> tboot.x86_64: W: no-manual-page-for-binary lcp_crtpolelt
> tboot.x86_64: W: no-manual-page-for-binary lcp_writepol
> tboot.x86_64: W: no-manual-page-for-binary tpmnv_defindex
> tboot.x86_64: W: no-manual-page-for-binary tpmnv_getcap
> tboot.x86_64: W: no-manual-page-for-binary tb_polgen
> tboot.x86_64: W: no-manual-page-for-binary txt-stat
Nice to have, not required.  Please include the existing documentation, at
least.

Licensing: Pretty close to violating
https://fedoraproject.org/wiki/PackagingGuidelines#Packages_which_are_not_useful_without_external_bits , but fine IMO (the sinit modules are not used "in the runtime system environment").


Documentation: Should lctptools/Linux_LCP_Tools_User_Manual.pdf,
lcptools/lcptools2.txt be included in the binary packages?


All of the problems below must be fixed:

Licensing: printk.h is under GPLv2, contradicting the spec license

Per https://fedoraproject.org/wiki/PackagingGuidelines#Trademarks_in_Summary_or_Description , the (R) marks should "never" be present.

The ExclusiveArch needs to be more general (probably using %ix86) if you want
the package to be available on 32-bit x86.

There should be an useful debuginfo package (do not use -s in install(1)).  I'm not sure if/how to handle debuginfo for /boot/tboot.gz , perhaps check if/how the kernel package (or the old xen packages) does it.

Regarding sinit, vendors are putting it into the BIOS to make it available. For example: http://lists.fedoraproject.org/pipermail/devel/2010-March/133089.html. We should be OK on that account.

I have uploaded a new SRPM to http://sourceforge.net/projects/tboot/files/tboot/tboot-20100910-1.fc13.src.rpm/download

FYI, the tboot build process now puts its binaries into /usr/sbin instead of /usr/bin.

This fixes all of the above comments except the debuginfo package.  I changed %build and %install to call make with 'debug=y', which causes the makefiles to compile with '-g' and removes the '-s' from the install commands.  However, no debuginfo package is created and rpmlint warns about unstripped binaries.  Everything I've been able to find on debuginfo packages seems to indicate that as long as the binaries are compiled with '-g' and not stripped, that 'rpmbuild -ba' "should just work" to make a debuginfo package.  Your wisdom on this is greatly appreciated.  (In the case of tboot.gz, the makefile explicitly strips the symbols out itself and creates a tboot-syms file, which it always copies to /boot.)

(In reply to comment #5)
> This fixes all of the above comments except the debuginfo package.  I changed
> %build and %install to call make with 'debug=y', which causes the makefiles to
> compile with '-g' and removes the '-s' from the install commands.  However, no
> debuginfo package is created and rpmlint warns about unstripped binaries. 
> Everything I've been able to find on debuginfo packages seems to indicate that
> as long as the binaries are compiled with '-g' and not stripped, that 'rpmbuild
> -ba' "should just work" to make a debuginfo package.  Your wisdom on this is
> greatly appreciated.

"%global debug_package %{nil}" was left on the top of the spec file.  Removing it seems to produce reasonable results.

> (In the case of tboot.gz, the makefile explicitly strips
> the symbols out itself and creates a tboot-syms file, which it always copies to
> /boot.)
That should be good enough considering that this can't be debugged from within a running system anyway.


I'm sorry, another thing: https://fedoraproject.org/wiki/PackagingGuidelines#Compiler_flags  - Perhaps not for the kernel-mode part, but the user-space utilities should use these flags in CFLAGS.  This will probably require some changes to the makefile system, collecting user-space flags into a variable that can be overridden from the spec file..

Attached are a .spec and source tree (20101005) that look like they build correctly and use RPM_OPT_FLAGS.  If these changes are correct, then I will check in the tboot changes.  (I'm seeing a copy of the compiler flags being appended to CFLAGS, due to the export, but I'm not sure how to fix it and it doesn't cause any harm.)

Created attachment 451567 [details]
tboot spec file for 20101005 build

Created attachment 451568 [details]
tboot source tree for 201005 build

Thank you, that seems to work fine.

Package accepted.

Adding the Intel Confidential Group.

Miroslav,

oOes this mean that tboot is going into Fedora 14?

Please modify the Source0 URL to point to the tarball at sourceforge (of course you'll have to upload the tarball there). Here is the guideline for the SF source urls:
https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net

With that fix the package should comply with the Fedora guidelines. Please apply for the Fedora Packager CVS Commit Group in the Fedora Account system and I will sponsor you. Then you can ask for creating the branches in the Fedora git and import the package into it.

Created attachment 457211 [details]
tboot spec file for 20101005 build

Updated spec file with correct Source0

Package APPROVED from me as well.

rpmlint -v tboot-20101005-1.fc13.src.rpm tboot-20101005-1.fc13.x86_64.rpm tboot-debuginfo-20101005-1.fc13.x86_64.rpm 
tboot.src: I: checking
tboot.src: W: spelling-error %description -l en_US pre -> per, ore, pee
OK, no typo here
tboot.x86_64: I: checking
tboot.x86_64: W: spelling-error %description -l en_US pre -> per, ore, pee
OK, as above
tboot.x86_64: W: no-manual-page-for-binary lcp_crtpol
tboot.x86_64: W: no-manual-page-for-binary tpmnv_relindex
tboot.x86_64: W: no-manual-page-for-binary lcp_writepol
tboot.x86_64: W: no-manual-page-for-binary lcp_crtpol2
tboot.x86_64: W: no-manual-page-for-binary lcp_crtpconf
tboot.x86_64: W: no-manual-page-for-binary tpmnv_lock
tboot.x86_64: W: no-manual-page-for-binary parse_err
tboot.x86_64: W: no-manual-page-for-binary tpmnv_defindex
tboot.x86_64: W: no-manual-page-for-binary lcp_crtpolelt
tboot.x86_64: W: no-manual-page-for-binary lcp_mlehash
tboot.x86_64: W: no-manual-page-for-binary lcp_crtpollist
tboot.x86_64: W: no-manual-page-for-binary lcp_readpol
tboot.x86_64: W: no-manual-page-for-binary acminfo
tboot.x86_64: W: no-manual-page-for-binary tpmnv_getcap
tboot.x86_64: W: no-manual-page-for-binary tb_polgen
tboot.x86_64: W: no-manual-page-for-binary txt-stat
It would be nice to get the manual pages sooner or later but it does not block the package acceptance.
tboot-debuginfo.x86_64: I: checking
3 packages and 0 specfiles checked; 0 errors, 14 warnings.

New Package SCM Request
=======================
Package Name: tboot
Short Description: Trusted Boot (tboot) is an open source, pre-kernel/VMM module that uses Intel Trusted Execution Technology (Intel TXT) to perform a measured and verified launch of an OS kernel/VMM.
Owners: jcihula
Branches: f14
InitialCC: eparis mitr sgrubb tmraz

This is a longest short description I ever saw in Fedora Packages.

Erm, yeah, please resubmit with something under 80 characters.

New Package SCM Request
=======================
Package Name: tboot
Short Description: A pre-kernel module for enabling Intel TXT in the kernel
Owners: jcihula
Branches: f14
InitialCC: eparis mitr sgrubb tmraz

Git done (by process-git-requests).

is this progressing towards being in Fedora 14?

Joseph is the "owner" of the package in Fedora, the only person allowed to commit changes or build the package.

Next steps are:
- build the package in rawhide
- copy the rawhide files to the f14 branch
- build the package in f14
- create an update in bodhi.

This is described in https://fedoraproject.org/wiki/PackageMaintainers/Join#Check_out_the_module and the following steps (except that Joseph has already imported the package to rawhide).

If you need any help with the tools, feel free to ask on IRC on #fedora-devel , send me an e-mail, or ask in this bug (preferably in that order).

tboot-20101005-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/tboot-20101005-1.fc14

tboot-20101005-1.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update tboot'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/tboot-20101005-1.fc14

tboot-20101005-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.