Bug 171268 - Review Request: kdissert
Summary: Review Request: kdissert
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Adrian Reber
QA Contact: David Lawrence
URL: http://freehackers.org/~tnagy/kdissert/
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2005-10-20 06:49 UTC by Konstantin Ryabitsev
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-10-29 16:19:49 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Spec changes (1.41 KB, patch)
2005-10-20 08:00 UTC, Adrian Reber
no flags Details | Diff

Description Konstantin Ryabitsev 2005-10-20 06:49:37 UTC
Spec Name or Url: http://linux.duke.edu/~icon/misc/fe/kdissert.spec
SRPM Name or Url: http://linux.duke.edu/~icon/misc/fe/kdissert-1.0.5-0.1.fc4.src.rpm
Description: 
kdissert is a mindmapping-like tool to help students to produce complicated 
documents very quickly and efficiently : presentations, dissertations, 
thesis, reports, etc. The concept is innovative : mindmaps produced using 
kdissert are processed to output near-ready-to-use documents. While 
targetted mostly at students, kdissert can also help teachers, decision 
makers, engineers and businessmen.

Comment 1 Adrian Reber 2005-10-20 07:53:06 UTC
Any special reason for installing the .desktop file into the kde sub-directory
of .../applications? I have not seen this in any other FE package. I don't
really care just wanted to know if there is a special reason for this?

Comment 2 Adrian Reber 2005-10-20 08:00:35 UTC
Created attachment 120184 [details]
Spec changes

The icon does not have to be copied to the pixmap directory if it is only for
the menu entry. To see the icon I have added the gtk-update-icon-cache to the
scripts and as the desktop file has a mime type entry the desktop database is
also updated in the scriptlets.

Comment 3 Adrian Reber 2005-10-20 08:02:00 UTC
I just saw that the patch has to be applied with -R. Sorry.

Comment 4 Konstantin Ryabitsev 2005-10-20 16:47:03 UTC
Hi, Adrian:

The .desktop file is where the upstream installs it. I've checked with the stuff
in core, and there's quite a few KDE-related .desktop files in the kde subdir or
applications, so it seems it's a common thing. It doesn't interfere with the way
the application works, so I have left the way it is.

Thanks for the patch, it's been applied. See:
http://linux.duke.edu/~icon/misc/fe/kdissert.spec
http://linux.duke.edu/~icon/misc/fe/kdissert-1.0.5-0.2.fc4.src.rpm

Comment 5 Adrian Reber 2005-10-21 06:55:12 UTC
rpmlint complains about rpath. Can be removed with

sed -i -e "/env.KDEuse(\"environ rpath\")/d" SConstruct

before calling scons

Comment 6 Konstantin Ryabitsev 2005-10-22 03:54:00 UTC
Nice. Done!

Same place: http://linux.duke.edu/~icon/misc/fe/

Comment 7 Adrian Reber 2005-10-24 07:13:12 UTC
md5sum of the tarball in the SRPM and the upstream tarball do not match.
According to diff the content seems to be same.

md5sum downloaded:
18ff5d04d633cf3b4e3fbf869c18dd2f  kdissert-1.0.5.tar.bz2

md5sum from SRPM:
c0a4ff4de929bfcc9bf80f7f58c5b4fe  kdissert-1.0.5.tar.bz2

Could you make a new version of the SRPM with the new tarball?

Comment 8 Konstantin Ryabitsev 2005-10-24 16:15:27 UTC
Hmm... This is odd. I recall distinctly checking the md5sum of the tarball
before packaging it. It's on my packaging checklist. I'll email the maintainers
to see what's up.

Comment 9 Konstantin Ryabitsev 2005-10-24 18:08:16 UTC
From the author:

--
Checksum should be 18ff5d04d633cf3b4e3fbf869c18dd2f

There was a mistake in the archive (documentation
installation) so the tarball was updated some time
after.
--

I've updated the SRPM to contain the matching tarball. See:
http://linux.duke.edu/~icon/misc/fe/

Comment 10 Adrian Reber 2005-10-25 08:51:29 UTC
- MUST: Packages must NOT contain any .la libtool archives, these should be
removed in the spec.

$ rpm -ql kdissert | grep la$
/usr/lib/kde3/libkdissOOOdoc.la
/usr/lib/kde3/libkdissOOOimpress.la
/usr/lib/kde3/libkdissapplet.la
/usr/lib/kde3/libkdissasciidoc.la
/usr/lib/kde3/libkdissbeamerslides.la
/usr/lib/kde3/libkdisshtmldoc.la
/usr/lib/kde3/libkdisspdflatexarticle.la
/usr/lib/kde3/libkdisspdflatexbook.la
/usr/lib/kde3/libkdissprosperslides.la
/usr/lib/kde3/libkdissstx.la

Seeing the .la discussions all the time you should probably remove these files.

Comment 11 Konstantin Ryabitsev 2005-10-25 14:02:56 UTC
It doesn't work without them. Try it. When .la files are removed, exporting
tools break.

Comment 12 Konstantin Ryabitsev 2005-10-25 20:58:42 UTC
Yes, after further investigation, making it not require .la files for exporting
plugins would require patching and be generally counter-productive, since it
would diverge from upstream. I think the opposition to .la files is only when
they are included as part of -devel packages (and thus never used), not when
they are required for program functioning during runtime.

Comment 13 Adrian Reber 2005-10-26 05:14:23 UTC
* rpmlint is happy
* builds in mock
* clean installation and removal
* spec looks good
* scripts are sane
* works as expected
* source matches upstream

* the .la files are required during runtime and therefore no blocker

APPROVED

Comment 14 Warren Togami 2005-10-28 16:17:15 UTC
> It doesn't work without them. Try it. When .la files are removed, exporting
> tools break.

Is this true of FC5 too?

Comment 15 Konstantin Ryabitsev 2005-10-28 16:35:42 UTC
I'm pretty sure. I've tried playing around to patch the requirement for .la
files out, but opening .so files didn't work, potentially because of a bug in
KDE (i.e. when you tell it to dlopen "libfoo", it will look for libfoo.la, and
ignore libfoo.so. If you specifically instruct to open "libfoo.so", it will fail
because it will tell that "init_libfoo.so not found", though it should have been
trying to find "init_libfoo" in the .so file as well. It's possible that I'm
just dumb. I'm not a C++/KDE programmer).

Comment 16 Konstantin Ryabitsev 2005-10-29 16:19:49 UTC
Thanks, Adrian!


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