Spec URL: http://people.redhat.com/avagarwa/files/openpts/openpts.spec SRPM URL: http://people.redhat.com/avagarwa/files/openpts/openpts-0.2.3-1.fc16.src.rpm Description: Open Platform Trust Services is a proof-of-concept (PoC) and reference implementation of Platform Trust Services (PTS) which is defined by the Trusted Computing Group (TCG).
blockers: * Package doesn't build in mock/koji: > + autoreconf -fv --install > autoreconf: Entering directory `.' > autoreconf: running: autopoint --force > Can't exec "autopoint": Permission denied at /usr/share/autoconf/Autom4te/FileUtils.pm line 345. > autoreconf: failed to run autopoint: Permission denied > autoreconf: autopoint is needed because this package uses Gettext This might only be a missing BuildRequires on gettext*, see below... * > openpts.x86_64: W: file-not-in-%lang /usr/share/locale/ja/LC_MESSAGES/openpts.mo Use the %find_lang macro for translations, see https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files * Source0: points to a HTML page. If it is possible to use something similar to https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net to get a direct URL to the tarball, this should be done. This problem is not a blocker if no such option exists, obviously. * Is there a reason for the explicit "Requires: trousers openssl"? rpm seems to be able to correctly add automatic dependencies. If these requirements are necessary, please add a comment (per https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires ) * Requires(preun, post, postun) for scriptlets are missing, see https://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscripts_in_spec_file_scriptlets * It seems the default configuration uses /var/lib/openpts. If so, shouldn't the directory be owned by the package? * /usr/share/openpts and /usr/share/openpts/models should be owned by the package. non-blockers: > openpts.x86_64: W: no-manual-page-for-binary openpts > openpts.x86_64: W: no-manual-page-for-binary tpm_createkey > openpts.x86_64: W: no-manual-page-for-binary ptscd > openpts.x86_64: W: no-manual-page-for-binary rm2dot > openpts.x86_64: W: no-manual-page-for-binary uml2dot > openpts.x86_64: W: no-manual-page-for-binary ir2text > openpts.x86_64: W: no-manual-page-for-binary iml2aide > openpts.x86_64: W: no-manual-page-for-binary iml2text It would be nice to have man pages, but writing them is primarily upstream's responsibility, having man pages is not a requirement. * Including the documenation from doc/ would probably be useful to users. Please also consider including ChangeLog in %doc. > openpts.x86_64: W: incoherent-init-script-name ptscd ('openpts', 'openptsd') Something to consider... but not a hard requirement IMHO. * The parenthesized abbreviations in %description look a little strange to me: They are not used anywhere else, so they are rather superfluous - especially the PoC abbreviation. This purely a matter of taste, of course. * The correct macro for /etc/init.d is _initddir, not _initrddir. * For consistency, consider using _/sbin/_chkconfig in %post * The initscript should probably exit with 2, not 3, on invalid command name (per the example in https://fedoraproject.org/wiki/Packaging:SysVInitScript ) * Consider using (cp -p) and (make install DESTDIR=... INSTALL='install -p') in %install to preserve timestamps * https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
Thanks for your review. (In reply to comment #1) > blockers: > > * Package doesn't build in mock/koji: > > + autoreconf -fv --install > > autoreconf: Entering directory `.' > > autoreconf: running: autopoint --force > > Can't exec "autopoint": Permission denied at /usr/share/autoconf/Autom4te/FileUtils.pm line 345. > > autoreconf: failed to run autopoint: Permission denied > > autoreconf: autopoint is needed because this package uses Gettext > This might only be a missing BuildRequires on gettext*, see below... > Fixed. > * > openpts.x86_64: W: file-not-in-%lang > /usr/share/locale/ja/LC_MESSAGES/openpts.mo > Use the %find_lang macro for translations, see > https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files > Fixed. > * Source0: points to a HTML page. > If it is possible to use something similar to > https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net to get a > direct URL to the tarball, this should be done. This problem is not a blocker > if no such option exists, obviously. > Can you please check again, because for me, it points to the file and lets me download the package? > * Is there a reason for the explicit "Requires: trousers openssl"? > rpm seems to be able to correctly add automatic dependencies. If these > requirements are necessary, please add a comment (per > https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires ) > Fixed. > * Requires(preun, post, postun) for scriptlets are missing, see > > https://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscripts_in_spec_file_scriptlets > Fixed. > * It seems the default configuration uses /var/lib/openpts. If so, shouldn't > the directory be owned by the package? > Fixed. > * /usr/share/openpts and /usr/share/openpts/models should be owned > by the package. > Fixed. > non-blockers: > > openpts.x86_64: W: no-manual-page-for-binary openpts > > openpts.x86_64: W: no-manual-page-for-binary tpm_createkey > > openpts.x86_64: W: no-manual-page-for-binary ptscd > > openpts.x86_64: W: no-manual-page-for-binary rm2dot > > openpts.x86_64: W: no-manual-page-for-binary uml2dot > > openpts.x86_64: W: no-manual-page-for-binary ir2text > > openpts.x86_64: W: no-manual-page-for-binary iml2aide > > openpts.x86_64: W: no-manual-page-for-binary iml2text > It would be nice to have man pages, but writing them is primarily upstream's > responsibility, having man pages is not a requirement. > Not fixed for the obvious reason what you stated above. > * Including the documenation from doc/ would probably be useful to users. > Please also consider including ChangeLog in %doc. > Fixed. > > openpts.x86_64: W: incoherent-init-script-name ptscd ('openpts', 'openptsd') > Something to consider... but not a hard requirement IMHO. > Not fixed. They have this perhaps because there is already a binary named openpts in the package, so to avoid confusion another name for init script. That said, surely can talk with upstream about it. > * The parenthesized abbreviations in %description look a little strange to me: > They are not used anywhere else, so they are rather superfluous - especially > the PoC abbreviation. This purely a matter of taste, of course. > Partially fixed. PTS and TCG are standard abbreviations used by TCG. Removed PoC though. > * The correct macro for /etc/init.d is _initddir, not _initrddir. > Fixed. > * For consistency, consider using _/sbin/_chkconfig in %post > Fixed. > * The initscript should probably exit with 2, not 3, on invalid command name > (per the example in https://fedoraproject.org/wiki/Packaging:SysVInitScript ) > Fixed. > * Consider using (cp -p) and (make install DESTDIR=... INSTALL='install -p') > in %install to preserve timestamps > Fixed. > * > https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment As such there is no formal bug reporting system with upstream. I will send patches soon to upstream though. The updated spec/srpm is as at http://people.redhat.com/avagarwa/files/openpts/openpts.spec http://people.redhat.com/avagarwa/files/openpts/openpts-0.2.3-2.fc16.src.rpm
Thanks for the update. blocker: (In reply to comment #2) > (In reply to comment #1) > > * Source0: points to a HTML page. > > If it is possible to use something similar to > > https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net to get a > > direct URL to the tarball, this should be done. This problem is not a blocker > > if no such option exists, obviously. > > Can you please check again, because for me, it points to the file and lets me > download the package? This is what I get: > $ wget 'http://sourceforge.jp/projects/openpts/downloads/51233/openpts-0.2.3.tgz' ... > --2011-04-14 18:35:32-- http://sourceforge.jp/projects/openpts/downloads/51233/openpts-0.2.3.tgz/ > Length: unspecified [text/html] > Saving to: „index.html“ ... and the file indeed contains HTML. Are you seeing something different? Is it possible that the behavior depends on the location of the client? non-blocker: > > * Including the documenation from doc/ would probably be useful to users. > > Please also consider including ChangeLog in %doc. > > Fixed. I was thinking more of the manuals; the .eps files are included in the .tex files and probably not intended to be shipped stand-alone. It seems that the manuals are not built by default.
(In reply to comment #3) > Thanks for the update. > > blocker: > (In reply to comment #2) > > (In reply to comment #1) > > > * Source0: points to a HTML page. > > > If it is possible to use something similar to > > > https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net to get a > > > direct URL to the tarball, this should be done. This problem is not a blocker > > > if no such option exists, obviously. > > > > Can you please check again, because for me, it points to the file and lets me > > download the package? > > This is what I get: > > $ wget 'http://sourceforge.jp/projects/openpts/downloads/51233/openpts-0.2.3.tgz' > ... > > --2011-04-14 18:35:32-- http://sourceforge.jp/projects/openpts/downloads/51233/openpts-0.2.3.tgz/ > > Length: unspecified [text/html] > > Saving to: „index.html“ > ... and the file indeed contains HTML. > > Are you seeing something different? Is it possible that the behavior depends > on the location of the client? > > I am seeing same behaviour with wget. However, if you try to download by clicking on the link that takes a bit of time before it asks you to save the file. Not sure if this is the reason that wget can not download and outputs this error "Length: unspecified [text/html]". Anyway that is the only upstream link I have right now. Please let me know if this is OK to go ahead. > non-blocker: > > > * Including the documenation from doc/ would probably be useful to users. > > > Please also consider including ChangeLog in %doc. > > > > Fixed. > I was thinking more of the manuals; the .eps files are included in the .tex > files and probably not intended to be shipped stand-alone. It seems that the > manuals are not built by default. You are right and since *.tex are not compiled to get pdf or eps, I did not want to include them. If you want, I can remove *.eps. Although I though that something is better than nothing ;-) . Again, please let me know as the above things are not blocker, then is it OK to get ahead with the review?
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (In reply to comment #1) > > > > * Source0: points to a HTML page. > > > > If it is possible to use something similar to > > > > https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net to get a > > > > direct URL to the tarball, this should be done. This problem is not a blocker > > > > if no such option exists, obviously. > I am seeing same behaviour with wget. However, if you try to download by > clicking on the link that takes a bit of time before it asks you to save the > file. Not sure if this is the reason that wget can not download and outputs > this error "Length: unspecified [text/html]". Right, this is done inside the html file. So using this URL won't work with automated Source: checkers and the like. > Anyway that is the only upstream link I have right now. Please let me know if > this is OK to go ahead. We don't have anything better, so this will have to do. > > non-blocker: > > > > * Including the documenation from doc/ would probably be useful to users. > > > > Please also consider including ChangeLog in %doc. > > > > > > Fixed. > > I was thinking more of the manuals; the .eps files are included in the .tex > > files and probably not intended to be shipped stand-alone. It seems that the > > manuals are not built by default. > > You are right and since *.tex are not compiled to get pdf or eps, I did not > want to include them. If you want, I can remove *.eps. Although I though that > something is better than nothing ;-) . > > Again, please let me know as the above things are not blocker, then is it OK to > get ahead with the review? Sure. ACCEPting openpts-0.2.3-2.fc16.src.rpm .
New Package SCM Request ======================= Package Name: openpts Short Description: TCG Platform Trust Service (PTS) for embedded devices Owners: avesh Branches: f14 f15 f16 InitialCC:
It is far too early to request f16 branches; f15 hasn't even been released yet.
f14 and f15 branches are also fine. Here is new request: New Package SCM Request ======================= Package Name: openpts Short Description: TCG Platform Trust Service (PTS) for embedded devices Owners: avesh Branches: f14 f15 InitialCC:
Git done (by process-git-requests).
Built and shipped. Closing.