Bug 181534 - Review Request: kst - plots scientific data
Summary: Review Request: kst - plots scientific data
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Orion Poplawski
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-02-14 20:49 UTC by Matthew Truch
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-03-01 21:43:02 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Matthew Truch 2006-02-14 20:49:03 UTC
Spec Name or Url: http://matt.truch.net/fedora/kst.spec
SRPM Name or Url: http://matt.truch.net/fedora/kst-1.2.0-1.src.rpm
Description: 
Kst is a real-time data viewing and plotting tool with basic data analysis
functionality. Kst contains many powerful built-in features and is
expandable with plugins and extensions. Kst is a KDE application.

Main features of kst include:
  * Robust plotting of live "streaming" data.
  * Powerful keyboard and mouse plot manipulation.
  * Powerful plugins and extensions support.
  * Large selection of built-in plotting and data manipulation functions,
    such as histograms, equations, and power spectra.
  * Color mapping and contour mapping capabilities for three-dimensional data.
  * Monitoring of events and notifications support.
  * Filtering and curve fitting capabilities.
  * Convenient command-line interface.
  * Powerful graphical user interface.
  * Support for several popular data formats.
  * Multiple tabs or windows.

------------------------------------------------

I've split kst up into kst, kst-docs, kst-devel, kst-fits, and kst-netcdf.
I'm not sure if I should do more or less.  

Thanks in advance for the review!

Comment 1 Gabriel Somlo 2006-02-15 15:18:25 UTC
In the main package file list, you have:

%{_libdir}/lib*.so.*

Then, in the devel package file list, you have this:

%{_libdir}/lib*.so

Without building the package myself, I'm wondering if this is right. Typically,
programs look for the *.so files, which end up being symlinks to the *.so.X.Y.Z
files, and therefore they all (both *.so symlinks AND *.so.X.Y.Z library files)
belong in the main package. Typically, the devel package will contain only the
static libraries (*.a) in addition to the includes.

Anyway, just wondering... :)

Comment 2 Matthew Truch 2006-02-15 15:37:39 UTC
(In reply to comment #1)
> In the main package file list, you have:
> 
> %{_libdir}/lib*.so.*
> 
> Then, in the devel package file list, you have this:
> 
> %{_libdir}/lib*.so
> 
> Without building the package myself, I'm wondering if this is right. Typically,
> programs look for the *.so files, which end up being symlinks to the *.so.X.Y.Z
> files, and therefore they all (both *.so symlinks AND *.so.X.Y.Z library files)
> belong in the main package. Typically, the devel package will contain only the
> static libraries (*.a) in addition to the includes.
> 
> Anyway, just wondering... :)

I'm not a library package expert, and don't know which .so files the program
looks for.   However, I'm following the protocol outlined in
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines , specifically the line
"MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel
package."  Furthermore, I have tested that kst runs fine without the kst-devel
pacakge installed.  

Comment 3 Ralf Corsepius 2006-02-15 15:41:26 UTC
In reply to comment #1)
> In the main package file list, you have:
> 
> %{_libdir}/lib*.so.*
> 
> Then, in the devel package file list, you have this:
> 
> %{_libdir}/lib*.so
> 
> Without building the package myself, I'm wondering if this is right. 
Unless the package is broken and correctly applies SONAMES, it normally is.

> Typically,
> programs look for the *.so files, 
No. Very oversimplified, ld.so at runtime looks for a library providing an SONAME.

A properly build shared library normally uses libXXXX.so.<cipher> as SONAME.
This is where the *.so symlink points to.

The *.so symlink normally only is being used by the linker, when linking a package.
=> *.so go to *-devel
=> *.so.* to the run-time package.

> Anyway, just wondering... :)
You're in error ;)




Comment 4 Orion Poplawski 2006-02-15 19:01:24 UTC
Good:

- package meets naming guidelines
- package meets packaging guidelines
- license (GPL) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- no missing BR
- uses %find_lang
- not relocatable
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- nothing in %doc affects runtime
- devel package ok
- post/postun ldconfig ok
- devel requires base package n-v-r 

Bad:

- This needs to compile on devel first, so it needs to handle modular X. BR
xorg-x11-devel needs to be removed.
- Unnecessary BR:  qt-devel,kdelibs-devel (required by kdebindings-devel)
- Needs to owns all directories that it creates:
  %{_datadir}/apps/kst/
  %{_datadir}/services/kst/
  %{_datadir}/servicetypes/
  %{_libdir}/kde3/kstplugins/

Other notes:

- Might want to change Source0 to:

ftp://ftp.kde.org/pub/kde/stable/apps/KDE3.x/scientific/kst-%{version}.tar.gz

- I'll need to take a look at the desktop files later
- Still working on building in mock and running rpmlint.

Comment 5 Matthew Truch 2006-02-15 21:16:47 UTC
Updated spec to address above (I hope):
http://matt.truch.net/fedora/kst.spec
http://matt.truch.net/fedora/kst-1.2.0-2.src.rpm

Removed un-needed build requires.
Ownes directories.
Changed Source0 as requested.

I don't have access to a machine running devel, so I can't confirm that it can
build there.  :-(

Thanks for the comments thus far.

Comment 6 Ed Hill 2006-02-15 21:24:21 UTC
Hi Matt, with mock you can specify a buildroot such as:

  mock -r fedora-5-i386-core ${YOUR_SRPM}

so that, even on an FC4 system, you can test builds on a devel/FC5 system

Comment 7 Orion Poplawski 2006-02-15 21:31:15 UTC
It builds fine for me on devel with xorg-x11-devel removed.

- Get rid of:

  --disable-debug and CXXFLAGS=-g CFLAGS=-g

debuginfo packages are created automatically and your mucking with the RPM
optimization flags.

$ rpmlint kst-docs
W: kst-docs dangling-symlink /usr/share/doc/HTML/sv/kst/common
/usr/share/doc/HTML/sv/common
W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/sv/kst/common
/usr/share/doc/HTML/sv/common
W: kst-docs dangling-symlink /usr/share/doc/HTML/et/kst/common
/usr/share/doc/HTML/et/common
W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/et/kst/common
/usr/share/doc/HTML/et/common
W: kst-docs dangling-symlink /usr/share/doc/HTML/en/kst/common
/usr/share/doc/HTML/en/common
W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/en/kst/common
/usr/share/doc/HTML/en/common
W: kst-docs dangling-symlink /usr/share/doc/HTML/fr/kst/common
/usr/share/doc/HTML/fr/common
W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/fr/kst/common
/usr/share/doc/HTML/fr/common
W: kst-docs dangling-symlink /usr/share/doc/HTML/pt/kst/common
/usr/share/doc/HTML/pt/common
W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/pt/kst/common
/usr/share/doc/HTML/pt/common
W: kst-docs dangling-symlink /usr/share/doc/HTML/it/kst/common
/usr/share/doc/HTML/it/common
W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/it/kst/common
/usr/share/doc/HTML/it/common
W: kst-docs dangling-symlink /usr/share/doc/HTML/nl/kst/common
/usr/share/doc/HTML/nl/common
W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/nl/kst/common
/usr/share/doc/HTML/nl/common
W: kst-docs dangling-symlink /usr/share/doc/HTML/da/kst/common
/usr/share/doc/HTML/da/common
W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/da/kst/common
/usr/share/doc/HTML/da/common

I generally fix with:

#Fix doc link
ln -sf ../common $RPM_BUILD_ROOT%{_defaultdocdir}/HTML/en/%{name}


Looks like I end up with a new menu tree: Applications -> Sciences -> kst.  Not
sure if this is kosher for FE.  I'll poke around.

Comment 8 Matthew Truch 2006-02-16 00:12:40 UTC
(In reply to comment #7)
> It builds fine for me on devel with xorg-x11-devel removed.

Great.

> - Get rid of:
> 
>   --disable-debug and CXXFLAGS=-g CFLAGS=-g
> 
> debuginfo packages are created automatically and your mucking with the RPM
> optimization flags.

Unfortunatly, with kst, if you don't have --disable-debug, then a bunch of
'crap' may be spewed out on stdout and/or stderr when using kst.  I don't know
how else to disable this 'debugging text output' while keeping the debugging
info in the binaries (so debuginfo packages) are useful.  

> $ rpmlint kst-docs
> W: kst-docs dangling-symlink /usr/share/doc/HTML/sv/kst/common
> /usr/share/doc/HTML/sv/common
> W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/sv/kst/common
> /usr/share/doc/HTML/sv/common
> W: kst-docs dangling-symlink /usr/share/doc/HTML/et/kst/common
> /usr/share/doc/HTML/et/common
> W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/et/kst/common
> /usr/share/doc/HTML/et/common
> W: kst-docs dangling-symlink /usr/share/doc/HTML/en/kst/common
> /usr/share/doc/HTML/en/common
> W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/en/kst/common
> /usr/share/doc/HTML/en/common
> W: kst-docs dangling-symlink /usr/share/doc/HTML/fr/kst/common
> /usr/share/doc/HTML/fr/common
> W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/fr/kst/common
> /usr/share/doc/HTML/fr/common
> W: kst-docs dangling-symlink /usr/share/doc/HTML/pt/kst/common
> /usr/share/doc/HTML/pt/common
> W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/pt/kst/common
> /usr/share/doc/HTML/pt/common
> W: kst-docs dangling-symlink /usr/share/doc/HTML/it/kst/common
> /usr/share/doc/HTML/it/common
> W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/it/kst/common
> /usr/share/doc/HTML/it/common
> W: kst-docs dangling-symlink /usr/share/doc/HTML/nl/kst/common
> /usr/share/doc/HTML/nl/common
> W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/nl/kst/common
> /usr/share/doc/HTML/nl/common
> W: kst-docs dangling-symlink /usr/share/doc/HTML/da/kst/common
> /usr/share/doc/HTML/da/common
> W: kst-docs symlink-should-be-relative /usr/share/doc/HTML/da/kst/common
> /usr/share/doc/HTML/da/common
> 
> I generally fix with:
> 
> #Fix doc link
> ln -sf ../common $RPM_BUILD_ROOT%{_defaultdocdir}/HTML/en/%{name}

This only fixes the "symlink-should-be-relative" warning, but doesn't remove the
"dangling-symlink" warning.  Is that correct?

Comment 9 Orion Poplawski 2006-02-16 17:31:13 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > - Get rid of:
> > 
> >   --disable-debug and CXXFLAGS=-g CFLAGS=-g
> > 
> > debuginfo packages are created automatically and your mucking with the RPM
> > optimization flags.
> 
> Unfortunatly, with kst, if you don't have --disable-debug, then a bunch of
> 'crap' may be spewed out on stdout and/or stderr when using kst.  I don't know
> how else to disable this 'debugging text output' while keeping the debugging
> info in the binaries (so debuginfo packages) are useful.  
> 

Well, CCFLAGS and CXXFLAGS automatically have -g in them when you use
%configure, but by setting them you wipe out the optimization flags.  So keep
--disable-debug but drop the flags.

> This only fixes the "symlink-should-be-relative" warning, but doesn't remove the
> "dangling-symlink" warning.  Is that correct?

Well, I think 

ln -sf ../../common $RPM_BUILD_ROOT%{_defaultdocdir}/HTML/sv/kst/%{name}

should handle that, right?  What is the /sv/kst/ directory for though?  I don't
have any other packages using %{_defaultdocdir}/HTML/sv/.  You'll need to own
that too.

Comment 10 Matthew Truch 2006-02-16 20:11:47 UTC
(In reply to comment #9)
> 
> Well, CCFLAGS and CXXFLAGS automatically have -g in them when you use
> %configure, but by setting them you wipe out the optimization flags.  So keep
> --disable-debug but drop the flags.

Done.

> > This only fixes the "symlink-should-be-relative" warning, but doesn't remove the
> > "dangling-symlink" warning.  Is that correct?
> 
> Well, I think 
> 
> ln -sf ../../common $RPM_BUILD_ROOT%{_defaultdocdir}/HTML/sv/kst/%{name}
> 
> should handle that, right?  What is the /sv/kst/ directory for though?  I don't
> have any other packages using %{_defaultdocdir}/HTML/sv/.  You'll need to own
> that too.

Directories owned.  The sv (and other) directories are for different
translations of the documentation.  If people have other documentation (in
specific) languages installed, those directories will contain translated docs
for other apps.  The dangling symlinks will be un-dangling if kde documentation
for the specific language is installed.  I don't know if this is the best thing,
but I'd really rather not have a separate package for each documentation
translation.  

Updated spec and SRPM:
http://matt.truch.net/fedora/kst.spec
http://matt.truch.net/fedora/kst-1.2.0-4.src.rpm

Comment 11 Orion Poplawski 2006-02-17 21:23:32 UTC
Needs work:
* Use of buildroot is not consistant
  (wiki: PackagingGuidelines#UsingBuildRootOptFlags)
  = sorry - this was introduced by my ln suggestion (I use RPM_BUILD_ROOT
instead of %buildroot).  You might try:

for x in %{buildroot}%{_defaultdocdir}/HTML/*/%{name}/
do
  ln -sf ../common $x
done

Will handle any new languages automatically in the future.  I think the dangling
link for other languages is fine.


- You need to install the .desktop file properly.  See
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28.desktop%29#head-254ddf07aae20a23ced8cecc219d8f73926e9755
I would also use --delete-original to remove the original.  The
Applications/Scientific directory structure is not used in Fedora.  e.g. (from
my kdesvn package):

desktop-file-install --vendor=fedora \
       --add-category=Qt \
       --add-category=KDE \
       --add-category=Application \
       --add-category=Development \
       --add-category=X-Fedora \
       --delete-original --dir $RPM_BUILD_ROOT%{_datadir}/applications \
       $RPM_BUILD_ROOT%{_datadir}/applications/kde/%{name}.desktop



I think that may be it :-)!



Comment 12 Matthew Truch 2006-02-18 00:19:28 UTC
(In reply to comment #11)
> Needs work:
> * Use of buildroot is not consistant
> 
> for x in %{buildroot}%{_defaultdocdir}/HTML/*/%{name}/
> do
>   ln -sf ../common $x
> done

Done.  I like that.  

> - You need to install the .desktop file properly.  See

Done.  

> I think that may be it :-)!

Cool.  New packages at the usual place:
http://matt.truch.net/fedora/kst.spec
http://matt.truch.net/fedora/kst-1.2.0-6.src.rpm

Comment 13 Orion Poplawski 2006-02-24 20:51:05 UTC
Looks good.  APPROVED.

Comment 14 Matthew Truch 2006-03-01 21:43:02 UTC
Thanks for the APPROVED.  After a brief issue with the buildsystem (the circular
dep), kst is in the system. 

Comment 15 Matthew Truch 2007-08-27 15:21:09 UTC
I'd like kst in EPEL as well, I'll maintain them:

====================
Package Name: kst
Owners: mtruch
Branches: EL-4 EL-5
Cvsextras Commits: yes

Comment 16 Kevin Fenzi 2007-08-27 17:00:08 UTC
cvs done.


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