Bug 478674 - Review Request: pp3 - Creation of sky charts in Postscript or PDF format
Review Request: pp3 - Creation of sky charts in Postscript or PDF format
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-03 05:38 EST by Marek Mahut
Modified: 2009-02-19 03:58 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-19 03:58:58 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Marek Mahut 2009-01-03 05:38:37 EST
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.
Comment 1 Lubomir Rintel 2009-01-04 18:22:41 EST
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!
Comment 2 Marek Mahut 2009-01-05 04:28:27 EST
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
Comment 3 Lubomir Rintel 2009-01-05 06:50:40 EST
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.
Comment 4 Marek Mahut 2009-01-05 09:15:14 EST
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
Comment 5 Lubomir Rintel 2009-01-05 10:11:37 EST
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
Comment 6 Marek Mahut 2009-01-05 10:40:57 EST
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
Comment 7 Kevin Fenzi 2009-01-06 19:08:28 EST
cvs done.
Comment 8 Lubomir Rintel 2009-01-18 14:24:35 EST
Marek: ping. Please import && build this.

Note You need to log in before you can comment on or make changes to this bug.