This service will be undergoing maintenance at 00:00 UTC, 2016-09-28. It is expected to last about 1 hours
Bug 225849 - Merge Review: gnuplot
Merge Review: gnuplot
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 13:56 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-07 07:10:27 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
pertusus: fedora‑review+


Attachments (Terms of Use)
fixes for the spec file (4.10 KB, patch)
2007-09-21 16:46 EDT, Patrice Dumas
no flags Details | Diff

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 13:56:34 EST
Fedora Merge Review: gnuplot

http://cvs.fedora.redhat.com/viewcvs/devel/gnuplot/
Initial Owner: varekova@redhat.com
Comment 1 Patrice Dumas 2007-02-05 06:43:26 EST
* BuildRoot isn't the right one

* Wouldn't it be a good idea to add directories holding truetype 
fonts located in %{_datatdir}/fonts to the gnuplot-4.0.0-refers_to.patch
patch?

* A suggestion: depend on the font packages. It has pros and cons.

* unuseful requires:
Requires: libpng

* I don't like the License tag. It appears not to be free software as it
is now, although it is. Maybe it would be better to have something like 
License: Gnuplot
And in a comment say something along
# changes must be distributed as patches

* RPM_OPT_FLAGS is unuseful on make command line, and %{?_smp_mflags}
is missing. I tested that replacing with
 make %{?_smp_mflags}

seems right.

Is
PATH=$RPM_BUILD_DIR/gnuplot-%{version}:$PATH
really needed?

* The %post scriptlet should be run everytime not only at install time

* A suggestion: in %{files} use wildcard to handle different compression or
no compression:

%{_mandir}/man1/gnuplot.1*
%{_infodir}/gnuplot.info*


* The doc in psdoc isn't generated rightly. I suggested cd to the directory
and
make ps_symbols.ps ps_fontfile_doc.pdf

Then distribute only 
ps_fontfile_doc.pdf ps_guide.ps  ps_symbols.ps ps_file.doc

It implies a BuildRequires on latex for pdflatex

Comment 2 José Matos 2007-03-08 08:07:14 EST
Note that a new version 4.2.0 is available. I would be interesting to have 
this version in F7.

http://sourceforge.net/forum/forum.php?forum_id=671915
Comment 3 Ivana Varekova 2007-03-15 09:20:00 EDT
(In reply to comment #1)
> * BuildRoot isn't the right one
fixed

> * Wouldn't it be a good idea to add directories holding truetype 
> fonts located in %{_datatdir}/fonts to the gnuplot-4.0.0-refers_to.patch
> patch?
The fonts directories are changed too often to have them here.

> * A suggestion: depend on the font packages. It has pros and cons.
I'm not sure I like to discuss this with gnuplot upstream.

> * unuseful requires:
> Requires: libpng
fixed

> * I don't like the License tag. It appears not to be free software as it
> is now, although it is. Maybe it would be better to have something like 
> License: Gnuplot
> And in a comment say something along
> # changes must be distributed as patches
fixed 

> * RPM_OPT_FLAGS is unuseful on make command line, and %{?_smp_mflags}
> is missing. I tested that replacing with
>  make %{?_smp_mflags} 
> seems right.
fixed

> Is
> PATH=$RPM_BUILD_DIR/gnuplot-%{version}:$PATH
> really needed?
fixed 

> * The %post scriptlet should be run everytime not only at install time
fixed
> 
> * A suggestion: in %{files} use wildcard to handle different compression or
> no compression:
> 
> %{_mandir}/man1/gnuplot.1*
> %{_infodir}/gnuplot.info*
I think the present version is right and it will help to reveal changes in new
versions.
> 
> 
> * The doc in psdoc isn't generated rightly. I suggested cd to the directory
> and
> make ps_symbols.ps ps_fontfile_doc.pdf
>
> Then distribute only 
> ps_fontfile_doc.pdf ps_guide.ps  ps_symbols.ps ps_file.doc
fixed
 
> It implies a BuildRequires on latex for pdflatex
> 
> 

Add comment #2
version 4.2.0 will be put to fc8 now it is too late to upgrade.
Comment 4 Patrice Dumas 2007-03-15 19:47:29 EDT
(In reply to comment #3)

> > * Wouldn't it be a good idea to add directories holding truetype 
> > fonts located in %{_datatdir}/fonts to the gnuplot-4.0.0-refers_to.patch
> > patch?
> The fonts directories are changed too often to have them here.

What do you exactly mean here? I think that at least the 
/usr/share/fonts/default/ghostscript/
/usr/share/fonts/default/Type1/
directories should be there. And certainly
/usr/share/fonts/bitstream-vera/

> > * A suggestion: depend on the font packages. It has pros and cons.
> I'm not sure I like to discuss this with gnuplot upstream.

Why do you have to discuss with upstream? It is a packaging 
issue.

> > * I don't like the License tag. It appears not to be free software as it
> > is now, although it is. Maybe it would be better to have something like 
> > License: Gnuplot
> > And in a comment say something along
> > # changes must be distributed as patches
> fixed 

You didn't exactly do what I said, but it is not a problem.

> I think the present version is right and it will help to reveal changes in new
> versions.

As you like.
 
> > 
> > * The doc in psdoc isn't generated rightly. I suggested cd to the directory
> > and
> > make ps_symbols.ps ps_fontfile_doc.pdf
> >
> > Then distribute only 
> > ps_fontfile_doc.pdf ps_guide.ps  ps_symbols.ps ps_file.doc
> fixed

From what I see it isn't fixed at all...
Comment 5 Ivana Varekova 2007-04-05 03:38:16 EDT
Patrice what preciesly are the problems you need to change before you proved
this review?
Comment 6 Patrice Dumas 2007-04-05 07:58:22 EDT
I think that the font directories are important for the integration in 
fedora.

The docs are not a blocker, but I gave all the informations needed
to implement it. Do you want a patch for that?

Otherwise at the moment I don't have any time to devote to merge
reviews (it is also for other reviews I am involved in), so somebody
else should take the review, or you'll have to wait (notice that I 
never formally took the review, because I knew that I could be taken
by 'real life' tasks).

A side note is that the comments I made in the gd merge review are
in fact useful for the new gnuplot png driver, and also to avoid
bogus dependencies in gnuplot.
Comment 7 Patrice Dumas 2007-06-01 04:32:52 EDT
I have a bit more time currently, but it seems that you haven't
responded to my last comment. I think that having the right 
fonts is very important for gnuplot usability. Assigning to me.
Comment 8 Jonathan Underwood 2007-07-14 07:52:54 EDT
# rpm -ql gnuplot-emacs
/usr/share/emacs/site-lisp/gnuplot-gui.el
/usr/share/emacs/site-lisp/gnuplot-gui.elc
/usr/share/emacs/site-lisp/gnuplot.el
/usr/share/emacs/site-lisp/gnuplot.elc
/usr/share/emacs/site-lisp/info-look.20.2.el
/usr/share/emacs/site-lisp/info-look.20.3.el
/usr/share/emacs/site-lisp/site-start.d/gnuplot-init.el

0) Subpackage gnuplot-emacs should really be called emacs-gnuplot for
consistency with other emacs add-on packages.

1) These files should be installed in their own sub-directory
/usr/share/emacs/site-lisp/gnuplot

2) The source elisp (*.el) files should be packaged in a emacs-gnuplot-el
subpackage - they are not needed at runtime (basically they are the equivalent
to devel files). That is except for gnuplot-init.el.

3) info-look.20.2.el and info-look.20.3.el should be byte compiled.
Comment 9 Ivana Varekova 2007-09-06 06:46:11 EDT
Thanks for your help.
Patrice,
your comments are incorporate in gnuplot-4.2.0-4.fc8. 
Jonathan, I'm not sure about your comments but 
0,2/ I don't think this should be done (0/ the common system is to call
subpackage the way it is now - there are only two exceptions (emacs-git,
emacs-gtypist).)
1,3/ I will look to this point more precisely - now I'm not sure about the right
directory/what should be done with attached files
Could you please look to gnuplot-4.2.0-4.fc8 whether this version is good enough
to get fedora-review flag?

Comment 10 Ivana Varekova 2007-09-07 06:01:23 EDT
I just build gnuplot-4.2.0-5.fc8 when emacs  files are moved to
/usr/share/emacs/site-lisp/gnuplot subdirectory.
Comment 11 Patrice Dumas 2007-09-21 16:46:18 EDT
Created attachment 202811 [details]
fixes for the spec file

Here is a spec file patch with the following changes:

- don't BuildRequires itself (and remove gnuplot-4.2.0-ver.patch)
- put X11 app-defaults file with right name in the right directory
- don't try to make ps_fontfile_doc.ps, Makefile is broken
- keep timestamps
- ship more doc files
- own latex directories, since latex may not be installed
- ship more docs
- add needed Requires(..) and minor fixes in info scriptlets
Comment 12 Patrice Dumas 2007-09-21 16:54:12 EDT
I am in vacation for the next week, in case somebody is waiting
for responses...
Comment 13 Ivana Varekova 2007-09-24 08:44:31 EDT
Thanks. Fixed.
Comment 14 Patrice Dumas 2007-10-24 18:15:34 EDT
You should add to the directories searched for fonts:

/usr/share/fonts/dejavu-lgc
/usr/share/fonts/liberation

Hopefully the font directory won't change too often or gnuplot
will try to use more generic font mechanisms. But in the mean time
it is required.

You should have a look at
http://fedoraproject.org/wiki/Packaging/Emacs
your emacs add-ons packaging doesn't follow the guideline.

* unuseful requires:
Requires: libpng

RPM_OPT_FLAGS="$RPM_OPT_FLAGS" is not used during build.

rpmint issues that are, in my opinion worth fixing:
gnuplot.i386: W: file-not-utf8 /usr/share/doc/gnuplot-4.2.0/ChangeLog
gnuplot.i386: W: spurious-executable-perm
/usr/share/doc/gnuplot-4.2.0/demo/html/webify.pl
gnuplot.i386: E: explicit-lib-dependency libpng
gnuplot.i386: W: doc-file-dependency
/usr/share/doc/gnuplot-4.2.0/demo/html/webify.pl perl(HTML::Entities)
gnuplot.i386: W: doc-file-dependency
/usr/share/doc/gnuplot-4.2.0/demo/html/webify.pl perl(ctime.pl)
gnuplot.i386: W: doc-file-dependency
/usr/share/doc/gnuplot-4.2.0/demo/html/webify.pl perl(Env)
gnuplot.i386: W: doc-file-dependency
/usr/share/doc/gnuplot-4.2.0/demo/html/webify.pl /usr/bin/perl
gnuplot-emacs.i386: W: no-documentation
gnuplot-debuginfo.i386: W: spurious-executable-perm
/usr/src/debug/gnuplot-4.2.0/src/getcolor.h
Comment 15 Ivana Varekova 2007-10-26 07:00:43 EDT
Thanks. Fixed in gnuplot-4.2.2-2.fc9.
Comment 16 Patrice Dumas 2007-10-27 07:06:01 EDT
The iconv on fontfile_latex.dem epslatex.dem epslatex.dem fontfile.dem
are completly wrong. The encoding is set in these files to latin1,
they should be kept as is.

Also I don't know what demo.edf is but running iconv on it is certainly
wrong, since it looks like there is some binary info in it.

You still don't follow the emacs guidelines, since there is a 
versionned dependency on emacs(bin) required. Also you should certainly
use the 
%define emacs_version %(pkg-config emacs --modversion)
%define emacs_lispdir %(pkg-config emacs --variable sitepkglispdir)
%define emacs_startdir %(pkg-coonfig emacs --variable sitestartdir)
with apropriate buildrequires.
Comment 17 Ivana Varekova 2007-11-02 07:40:17 EDT
Thanks fixed in gnuplot-4.2.2-3.fc9.
Comment 18 Jonathan Underwood 2007-11-02 07:50:17 EDT
Please note those macros are broken as used in the Emacs addon guidelines - I
have emailed Tom Callaway with the fixes twice, but he hasn't yet responded (and
he owns the wiki page).

Anyway, they should really be as below which (a) fixes the "coonfig" typo, and
also stops CVS tagging breaking.

# If the emacs-el package has installed a pkgconfig file, use that to determine
# install locations and Emacs version at build time, otherwise set defaults.
%if %($(pkg-config emacs) ; echo $?)
%define emacs_version 22.1
%define emacs_lispdir %{_datadir}/emacs/site-lisp
%define emacs_startdir %{_datadir}/emacs/site-lisp/site-start.d
%else
%define emacs_version %(pkg-config emacs --modversion)
%define emacs_lispdir %(pkg-config emacs --variable sitepkglispdir)
%define emacs_startdir %(pkg-config emacs --variable sitestartdir)
%endif
Comment 19 Jonathan Underwood 2007-11-02 08:02:57 EDT
For the interested, Tom has now updated the guidelines :)
Comment 20 Ivana Varekova 2007-11-02 08:10:38 EDT
Thanks for your help Jonathan, in gnuplot-4.2.2-3.fc9 there is used the similar
construction as you attach here. 
I wrote to Tom Callaway too :) (before he change the wiki).
Comment 21 Patrice Dumas 2007-11-03 06:23:45 EDT
There is still
gnuplot.i386: W: spurious-executable-perm
/usr/share/doc/gnuplot-4.2.2/demo/html/webify.pl
gnuplot.i386: W: doc-file-dependency
/usr/share/doc/gnuplot-4.2.2/demo/html/webify.pl perl(HTML::Entities)
gnuplot.i386: W: doc-file-dependency
/usr/share/doc/gnuplot-4.2.2/demo/html/webify.pl perl(ctime.pl)
gnuplot.i386: W: doc-file-dependency
/usr/share/doc/gnuplot-4.2.2/demo/html/webify.pl perl(Env)
gnuplot.i386: W: doc-file-dependency
/usr/share/doc/gnuplot-4.2.2/demo/html/webify.pl /usr/bin/perl

This can be fixed with chmod a-x on webify.pl, either in %prep 
or %install.


Also the file eg7.eps is referenced in tutorial.dvi, so I think
it is better to ship it in %docs, like

%docs tutorial/eg7.eps
Comment 22 Ivana Varekova 2007-11-06 09:07:45 EST
Thanks both problems are fixed in gnuplot-4.2.2-4.fc9.
Comment 23 Patrice Dumas 2007-11-06 15:01:34 EST
Source match upstream

b9dd34e8210a65336b19ee408766a66f  gnuplot-4.2.2.tar.gz

Source archive timestamp is not kept. It is too late now, but please
remember next time to use spectool -g or wget -N:
-rw-rw-r-- 1 dumas dumas 2832174 sep  1 05:36 gnuplot-4.2.2.tar.gz
-rw-rw-r-- 1 dumas dumas 2832174 oct 25 13:35 gnuplot-4.2.2.tar.gz-orig


Everything looks right now. I haven't checked precisely the
data files timestamps but there is no multilib issue here.

APPROVED.
Comment 24 Ivana Varekova 2007-11-07 07:10:27 EST
Thanks for your great work Patrice. I'm closing this bug.

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