Spec URL: http://jmrodri.fedorapeople.org/pkgs/tpp.spec SRPM URL: http://jmrodri.fedorapeople.org/pkgs/tpp-1.3.1-3.fc14.src.rpm Description: tpp stands for text presentation program and is an ncurses-based presentation tool. The presentation can be written with your favorite editor in a simple description format and then shown on any text terminal that is supported by ncurses - ranging from an old VT100 to the Linux framebuffer to an xterm.
Please don't mix $RPM_BUILD_ROOT and %{buildroot}.
UGH sorry that's what I get for copy and pasting :( is $RPM_BUILD_ROOT ok? I prefer to use that one.
Updated spec to use just $RPM_BUILD_ROOT. http://jmrodri.fedorapeople.org/pkgs/tpp-1.3.1-4.fc14.src.rpm http://jmrodri.fedorapeople.org/pkgs/tpp.spec
Hello Jesus, yes, it is ok to use $RPM_BUILD_ROOT, just don't mix them. Furthermore, copy-paste is totally fine when it comes to packaging, however, you should really use a better template next time. ;-) Here are a few things that should be fixed before a review. - Development/Languages is maybe not the best choice for this package. I mean it is more like an application, isn't it? - There is a little confusion about the license. The tpp source file states GPLv2, while the README states GPLv2+. Upstream should be approached for clarification. If they don't react, GPLv2 is the correct choice. - It is not wise to hard-code the name and version in the Source0 tag, when you can use %{name} and %{version} instead and save you some work in the future. - Just a little typo in the description: [...] _a_ ncurses-based [...] - The %install section is a bit ugly. You create a few directories just to extract the data with the more than simple Makefile, which you even have to "fedorarize" before usage. For that you create the unneeded dir $RPM_BUILD_ROOT%{_datadir}/%{name} (and you do it twice, just to be sure it really worked) and you create the docdir yourself, which you don't have to, when packaging the docs appropriately. My suggestion: Why not just throw the Makefile away, copy tpp to $RPM_BUILD_ROOT/%{_bindir} (using install -p or cp -p), add the vim and emacs files to the directories where they can be useful (have a look at the package vim-filesystem, which the package could require and at [1] for emacs). Finally, add the docfiles by using %doc and then just list the appropriate files and subdirs, the docdir is created automatically then. Do not, however, add the manpage as %doc, this is also done automatically. Use %{_mandir}/man1/tpp.1.* instead. (the * makes sure that the manpage is packaged correctly, even when the compression method would be changed or even dropped). - Why do you repeat your mail address in the changelog entries? I am referring to the second occurrence in the actual descriptions, for example: * Wed May 11 2011 jesus m. rodriguez <jesusr> 1.3.1-4 # why is the e-mail address repeated here? - don't use RPM_BUILD_ROOT and buildroot --> (jesusr) <-- - You do know rpmlint, right? It reveals (among some false positives): rpmlint tpp-1.3.1-4.fc14.noarch.rpm tpp.noarch: W: spelling-error Summary(en_US) ncurses -> nurses, curses, n curses tpp.noarch: W: summary-not-capitalized C ncurses-based presentation tool tpp.noarch: W: spelling-error %description -l en_US ncurses -> nurses, curses, n curses tpp.noarch: W: spelling-error %description -l en_US framebuffer -> frame buffer, frame-buffer, framework tpp.noarch: E: incorrect-fsf-address /usr/share/doc/tpp-1.3.1/COPYING tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/ac-am.tpp tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/slidein.tpp tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/tpp-features.tpp tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/test.tpp tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/bold.tpp tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/huge.tpp tpp.noarch: W: file-not-utf8 /usr/share/doc/tpp-1.3.1/examples/debian-packaging.tpp 1 packages and 0 specfiles checked; 1 errors, 11 warnings. Concerning the utf8 encoding warnings refer to [2] The FSF address error means that an outdated address of the FSF is found in the license file. This should be fixed and probably even be reported upstream. --- This may seem like a lot, but must issues can by fixed quite quickly. And when you are finished, we do the review and you have a better template next time. :) --- [1] http://fedoraproject.org/wiki/Packaging:Emacs#Case_II:_Package_also_provides_auxiliary_.28X.29Emacs_files [2] http://fedoraproject.org/wiki/Common_Rpmlint_issues#file-not-utf8
Thanks Golo. I'll get on this in the next day or so.
The email address is repeated because I used tito to build the package. https://github.com/dgoodwin/tito It was built to built Spacewalk packages which have many contributors and thus the commits contain the email addresses of the committers.
I don't think I understand the mandir, because if I do just this in my spec file rpmbuild gives me an error: %files %defattr(-, root, root, -) %{_bindir}/tpp %{_mandir}/man1/tpp.1* RPM build errors: File not found by glob: /home/devel/jesusr/rpmbuild/BUILDROOT/tpp-1.3.1-4.fc13.x86_64/usr/share/man/man1/tpp.1* Only way I can get around this is having the %install section contain an install command: %install install -d -m 755 $RPM_BUILD_ROOT%{_mandir}/man1/ install -p doc/tpp.1 $RPM_BUILD_ROOT%{_mandir}/man1/tpp.1 %files %{_mandir}/man1/tpp.1* The above seems to make rpmbuild happy.
I have a few other packages installed that don't require vim-filesystem, and put their .vim files in /usr/share/doc. For example, /usr/share/doc/glusterfs-common-2.0.9/glusterfs.vim /usr/share/doc/subversion-1.6.16/tools/dev/svn-dev.vim /usr/share/doc/tpp-1.3.1/contrib/tpp.vim I have nothing under /usr/share/vim/vimfiles/syntax/ and everything under /usr/share/vim/vim73/syntax/ is provided by vim-common. So I'm confused as to where I should put the contrib files. I'm inclined to leave them in /usr/share/doc/tpp-1.3.1/contrib/.
Updated spec. http://jmrodri.fedorapeople.org/pkgs/tpp.spec http://jmrodri.fedorapeople.org/pkgs/tpp-1.3.1-6.fc14.src.rpm rpmlint still shows these warnings, but I think their fine. --- rpmlint /tmp/tito/noarch/tpp-1.3.1-6.fc14.noarch.rpm tpp.noarch: W: spelling-error Summary(en_US) ncurses -> nurses, curses, n curses tpp.noarch: W: spelling-error %description -l en_US ncurses -> nurses, curses, n curses tpp.noarch: W: spelling-error %description -l en_US framebuffer -> frame buffer, frame-buffer, framework 1 packages and 0 specfiles checked; 0 errors, 3 warnings.
Hi Jesus, there are several places where Vim looks for syntax files. The versioned folders /usr/share/vim/vimXX usually contain only the files bundled with the corresponding Vim release. If you want to install third-party syntax files, you should put them into /usr/share/vim/vimfiles/syntax/ owned by package vim-filesystem (available since Fedora 14). Shipping them as doc files forces the user to manually copy them to ~/.vim/syntax. The emacs files should be placed in %{_emacs_sitelispdir}/ (owned by emacs-filesystem starting with Fedora 15). You also have to byte-compile it as described in http://fedoraproject.org/wiki/Packaging:Emacs#Case_II:_Package_also_provides_auxiliary_.28X.29Emacs_files Please move the iconv loop to the %prep section. If you don't plan to maintain the package for EPEL < 6, you can drop all the buildroot stuff (BuildRoot field, %clean section, initial cleaning of the buildroot in %install). I suggest to add -m 644 to the install statement of the manpage and drop %attr in %files.
Thanks Martin, added emacs (bytecompiled and plain) & vim files. Removed the buildroot stuff, put iconv in proper place. http://jmrodri.fedorapeople.org/pkgs/tpp-1.3.1-7.fc14.src.rpm http://jmrodri.fedorapeople.org/pkgs/tpp.spec
Obviously I was wrong about the mandir being created automatically, I apologize for that. The package looks much better now! I will do the review.
For proper directory ownership, I recommend to add %if 0%{?fedora} >= 15 Requires: emacs-filesystem >= %{_emacs_version} %endif
Added Martin's change to the spec. http://jmrodri.fedorapeople.org/pkgs/tpp-1.3.1-8.fc13.src.rpm http://jmrodri.fedorapeople.org/pkgs/tpp.spec
Anything else I need to do?
Yes, there are still some issues with this package. - Right now, the Requirements (i.e. emacs-filesystem) guarantee the ownership of %{_emacs_sitelispdir} only for F15, since the package is not available for F<15. There are packages, which do own %{_emacs_sitelispdir} there, however, none of that is required by tpp. So tpp has to own it as well, otherwise after the uninstall of tpp that dir might remain and pollute the user's system. So, in the %files section we need %if 0%{?fedora} < 15 %dir %{_emacs_sitelispdir}/ %endif - You can think of the install command as a cp, so in the build section you copy all files from contrib to the emacs and vim related directories. Then, however, you add the contrib folder to the %doc files. Not only is this useless, but a file is not allowed to be packaged more than once without good reason. - In the newest %changelog entry the mail address is now missing completely. http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs - rpmlint still complains about the wrong fsf address, this won't block tpp I guess, however, upstream should really be informed in order to fix this.
* added %if 0%{?fedora} < 15 %dir %{_emacs_sitelispdir}/ %endif * removed doc contrib * added email address to the changelog * I've notifed upstream about the license.
Packages updated http://jmrodri.fedorapeople.org/pkgs/tpp-1.3.1-9.fc14.src.rpm http://jmrodri.fedorapeople.org/pkgs/tpp.spec
Everything looks fine to me now. Here is the review: ----- [+] = ok [o] = does not apply [-] = needs work ----- [+] rpmlint output: rpmlint RPMS/noarch/tpp-1.3.1-9.fc15.noarch.rpm SRPMS/tpp-1.3.1-9.fc15.src.rpm SPECS/tpp.spec tpp.noarch: W: spelling-error Summary(en_US) ncurses -> nurses, curses, n curses tpp.noarch: W: spelling-error %description -l en_US ncurses -> nurses, curses, n curses tpp.noarch: W: spelling-error %description -l en_US framebuffer -> frame buffer, frame-buffer, framer tpp.noarch: E: incorrect-fsf-address /usr/share/doc/tpp-1.3.1/COPYING tpp.src: W: spelling-error Summary(en_US) ncurses -> nurses, curses, n curses tpp.src: W: spelling-error %description -l en_US ncurses -> nurses, curses, n curses tpp.src: W: spelling-error %description -l en_US framebuffer -> frame buffer, frame-buffer, framer 2 packages and 1 specfiles checked; 1 errors, 6 warnings. The spelling errors are all ok, the fsf address has to be fixed upstream and is not a blocker. [+] The package is named according to the guidelines [+] Spec file name matches base package name [+] The package follows the Packaging Guidelines [+] The license is an approved licence (GPLv2) [+] The License field matches the actual licence [+] License file from source file is included in %doc [+] The spec file is written in American English [+] The spec file is legible [+] Packaged sources match with upstream sources (md5) md5sum tpp-1.3.1.tar.gz.{packaged,upstream} 35eebb38497e802df1faa57077dab2d1 tpp-1.3.1.tar.gz.packaged 35eebb38497e802df1faa57077dab2d1 tpp-1.3.1.tar.gz.upstream [+] Package build at least on one primary architecture [+] ExecludeArch is not known to be needed. [+] All build dependencies are listed in the BuildRequires section [o] No locales for the package [o] Package stores no shared libraries [+] Package does not bundle copies of system libraries [o] Package is not relocatable [+] Package owns all directories it installs. [+] No files are listed more then once in the %files section [+] File permissions are set properly (%defattr(...) is used) [+] Consistent use of macros [+] Package contains code and documentation only, no content [+] No large documentation files in the base package [+] %doc files do not affect runtime [o] No header files packaged [o] No static libraries included [o] library files ending with .so included in devel subpackage [o] no -devel subpackage [+] No libtool .la archives included [o] No GUI application, no need for a .desktop file [+] Package does not own files or directories that are owned by other packages (well, it owns %{_emacs_sitelispdir}, but this is a necessary exception, since emacs is not required) [+] All filenames are valid UTF-8 SHOULD items: [o] Source package does already include license text as a separate file from upstream. [o] No other Non-English languages supported. [+] The package builds in mock. [+] koji scratch build successful. http://koji.fedoraproject.org/koji/taskinfo?taskID=3127759 [+] Tested: tpp starts and an example presentation from the homepage is properly displayed. [+] No "exotic" scriptlets used. [o] No subpackages. [o] No pkgconfig(.pc) files. [o] No file dependencies. [+] Man page included. ----- ================ PACKAGE APPROVED ================
Ping. Hi, this package seems to be approved a couple of days ago. Is someone requesting an official koji build? Currently I'm tpp by making a scratch build from the srpm linked above.
typo in the previous comment: s/I'm/I'm using*
This seems stuck after approval(in June). I intend to close it in two weeks if I don't hear back.
I got busy and haven't requested the koji build. I will get that done this week.
Hi Jesus, you still need to request the fedora-cvs flaq (https://fedoraproject.org/wiki/Package_SCM_admin_requests). I'd be interested in seeing tpp for el6, can you please include that in your request? Thanks, Niels
Ping again. No update here. It's been nearly *6 months*. I don't think there is much involved to be done here. Can we file the koji build request, and move on please?
As a side note: Can you please do an SCM request, I can help you to build this package. Also, I'd like to co-maintain this package if it's fine with you. Jesus, for your convenience, I'm pasting the SCM admin request here. Please file this one and also turn the flag(in this bugzilla) 'fedora_cvs' to ? New Package SCM Request ======================= Package Name: tpp Short Description:A ncurses-based presentation tool Owners: jmrodri Branches: f16 f17 InitialCC: kashyapc
Oh, and please also request the el6 branch. I'm happy to (co-)maintain that one.
Ok I'll work on this now.
New Package SCM Request ======================= Package Name: tpp Short Description:A ncurses-based presentation tool Owners: jmrodri Branches: f16 f17 InitialCC: kashyapc
New Package SCM Request ======================= Package Name: tpp Short Description:A ncurses-based presentation tool Owners: jmrodri Branches: f16 f17 el6 InitialCC: kashyapc
Git done (by process-git-requests).
This review seems to be finished and the package built, thus closing this bug.