Spec URL: http://mmahut.fedorapeople.org/reviews/pp3/pp3.spec SRPM URL: http://mmahut.fedorapeople.org/reviews/pp3/pp3-1.3.3-1.fc8.src.rpm Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1030504 Description: PP3 creates celestial charts. It generates resolution independent maps of very high graphical quality in Postscript or PDF format. They can be used for example as illustrations in books or on web pages. You may use the databases of the distribution or your own databases converted to PP3's simple text format. PP3 uses LaTeX+pstricks as the backend for generating the vector graphics. You can add arbitrary labels to the map. The output is configurable in many ways.
1.) Please do not strip the packages while %install-ing The debuginfo is generated empty. export CXXFLAGS="%{optflags} -s" make %{?_smp_mflags} Why do you add -s there? You should not strip the debugging symbols off the binaries. Tip: Besides environment variables, make takes initial variable values also from command line: make %{?_smp_mflags} CXXFLAGS="%{optflags} 2.) The manual contains also images, but you include only *.html in the package. You may want to wget -p it, generate a tarball and Source it along with the comment on how did you generate it. Tip: Work with upstream -- they may be willing to include documentation in the source tarball in next release. They may also be willing to include information on how to install it in Fedora, since the manual has "RPM installation section" 3.) You patch for gcc 4.3 compatibility, not 3.4 (I guess) ;) 4.) You don't have to patch makefile just to override variables. Just unset them on command line! (see 1.) By the way -- why do you patch info installation away? Please comment!
1) Fixed. 2) Fixed, pp3-1.3.3-manual.tar.gz created. 3) Oops. Fixed. 4) Make info is trying to generate pdf files out of examples using pp3 itself - I thought adding BUILD_ROOT to PATH isn't the best idea as we don't need these files anyway if we are shipping it with pp3-1.3.3-manual.tar.gz - What do you think about it? Spec URL: http://mmahut.fedorapeople.org/reviews/pp3/pp3.spec SRPM URL: http://mmahut.fedorapeople.org/reviews/pp3/pp3-1.3.3-1.fc8.src.rpm
1.) You do not need to unpack the first (zero-th) tarball two times. And do not need to move the manual directory. %setup -q %setup -q -b1 cp -vr ../manual/ . This seems useless to me. Skip the first line. (In case you're keeping the manual tarball). And unpack the manual directory in place: %setup -q -a1 2.) Please do not use the dependency on obsolete package (tetex): Requires: tetex-latex this would be more appropriate: Requires: tex(latex) (In reply to comment #2) > 4) Make info is trying to generate pdf files out of examples using pp3 itself - > I thought adding BUILD_ROOT to PATH isn't the best idea as we don't need these > files anyway if we are shipping it with pp3-1.3.3-manual.tar.gz - What do you > think about it? I think you should be generating the manual, and adding build root to path is perfectly legal way to do that. Alternatively, you could patch the makefile to use ../pp3 instead of pp3 when building documentation.
All should be fixed, have a look: Spec URL: http://mmahut.fedorapeople.org/reviews/pp3/pp3.spec SRPM URL: http://mmahut.fedorapeople.org/reviews/pp3/pp3-1.3.3-1.fc8.src.rpm
Much better! 1.) Soooo... you actually strip the binary upon installation: install -s pp3 $(ROOT)$(DESTDIR) Please patch the -s away from that, so that debuginfo is not empty. 2.) You get a lot of these: pp3.i386: W: file-not-utf8 /usr/share/info/leo1.png.gz The character encoding of this file is not UTF-8. Consider converting it in the specfile for example using iconv(1). Please %exclude %{_infodir}/*.png* files You can fix those upon commit. APPROVED
Ok, I'll fix these before committing. Thank you for review. New Package CVS Request ======================= Package Name: pp3 Short Description: Creation of sky charts in Postscript or PDF format Owners: mmahut Branches: F-9 F-10 EL-5
cvs done.
Marek: ping. Please import && build this.