Bug 248899 - Review Request: kdelibs3 - K Desktop Environment 3 - Libraries
Summary: Review Request: kdelibs3 - K Desktop Environment 3 - Libraries
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Kevin Kofler
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2007-07-19 15:13 UTC by Rex Dieter
Modified: 2015-04-02 14:44 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-12-17 15:38:15 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora-review+
puiterwijk: fedora-cvs+

Attachments (Terms of Use)
the missing patch (3.13 KB, patch)
2007-07-19 17:17 UTC, Ngo Than
no flags Details | Diff

Description Rex Dieter 2007-07-19 15:13:51 UTC
Spec URL: http://kdeforge.unl.edu/apt/kde-redhat/SOURCES/kdelibs3/kdelibs3.spec
SRPM URL: n/a, still missing kdelibs-3.5.7-kde3compat.patch
Libraries for the K Desktop Environment:
KDE Libraries included: kdecore (KDE core library), kdeui (user interface),
kfm (file manager), khtmlw (HTML widget), kio (Input/Output, networking),
kspell (spelling checker), jscript (javascript), kab (addressbook),
kimgio (image manipulation).

Comment 1 Rex Dieter 2007-07-19 15:15:13 UTC
Than, couldn't make a srpm, I'm missing kdelibs-3.5.7-kde3compat.patch (what's
that for?)

Comment 2 Ngo Than 2007-07-19 17:17:33 UTC
Created attachment 159597 [details]
the missing patch

the missing patch

Comment 3 Kevin Kofler 2007-07-19 17:19:38 UTC
I'll review this once I'm done with kdepimlibs.

Comment 4 Kevin Kofler 2007-07-19 17:38:18 UTC
Rex, are you preparing a SRPM or should I do it?

Comment 5 Kevin Kofler 2007-07-19 18:34:54 UTC
Before you prepare that SRPM, please make a pass of s/qt3/qt/g as we decided 
not to rename Qt. Or are you preparing Qt 3 for a future rename (that will need 
both Provides in the package name and a symlink for qt.sh->qt3.sh)?

Moreover, if Qt does get renamed to qt3, do we really need the epoch?

Comment 6 Rex Dieter 2007-07-20 12:53:40 UTC
OK, I think I've got something that works now, but I think we may have to
(re)enable arts support, at least for now, since there appear to be a bunch of
pkgs(*) that currently Requires: libartskde.so.1 which is lacking if built
--without-arts, so for now, I'll re-enable arts support, to make testing for
review purposes easier.  We can toggle that later, if needed.

(*) From a quick repoquery:

Comment 7 Kevin Kofler 2007-07-20 13:09:54 UTC
Then let's reenable it for now and look at disabling it again once the rest of 
KDE is updated. These aRts users will (our should, at least) all go away once 
KDE is fully upgraded:
(As I said in our e-mail discussion, nothing in kdegames3 requires aRts.)

These ones will have to be dealt with separately:
If those will have no KDE 4 version available in time for F8, they will have to 
be built without sound support if we want to disable aRts.

Comment 8 Rex Dieter 2007-07-20 13:37:23 UTC
Spec URL: http://kdeforge.unl.edu/apt/kde-redhat/SOURCES/kdelibs3/kdelibs3.spec

brief changelog from initial submission:
* actually builds now. :)
* (re)enabled arts support
* added kded
* dropped kde3compat patch (didn't build with it, don't think it was needed anyway)
* cups13 patch

Built(1) testing pkgs can be found in:
in kdelibs3/ and kdelibs/ (v4) folders.

(1) ok, I lied, kdelibs3 pkgs are building as I type this.

Comment 9 Rex Dieter 2007-07-20 14:16:36 UTC
doh, just realized since we're not providing kdelibs-apidocs anymore, that
kdelibs3 (or -devel?) probably
SHOULD: Obsoletes: kdelibs-apidocs < 6:%{version}-%{release}

Comment 10 Kevin Kofler 2007-07-20 14:18:32 UTC
I think we should be providing kdelibs-apidocs for kdelibs 4 instead. ;-)

Comment 11 Rex Dieter 2007-07-20 14:22:39 UTC
Until that happens, we have a broken upgrade path. So, when/if kdelibs-apidocs
>= 4 exists, we can omit the Obsoletes.

Comment 12 Kevin Kofler 2007-07-20 15:23:32 UTC
Maybe we should build apidocs in kdelibs3 and Provide/Obsolete kdelibs-apidocs 
there for now? Otherwise we'll break deps for kdevelop in test1.

Comment 13 Rex Dieter 2007-07-20 15:25:56 UTC
Ick, ok (didn't notice kdevelop was missing from the kde-3.91.0 alpha release)

another thing: avahi-kdnssd: dropping avahi-kdnssd support yields conflicts
with, ha, avahi-kdnssd, so either we need to keep 
Requires: avahi-kdnssd
+Obsoletes: avahi-kdnssd

Comment 14 Kevin Kofler 2007-07-20 15:28:19 UTC
For avahi-kdnssd, I think we should just keep shipping it for the KDE 3 compat 
libs. (KDE 4 now supports avahi natively, yay!)

KDevelop is going to be fun. KDevelop 4 won't really be ready at KDE 4.0 and 
Fedora 8 release. KDevelop 3 has some support for Qt 4 and KDE 4 development 
though, but we'll have to drop the hard deps on the KDE 3 devel stuff to make 
it usable for KDE 4 development.

Comment 15 Kevin Kofler 2007-07-20 15:35:20 UTC
("Support" for KDE 4 in KDevelop 3.4 is limited to:
* creating a blank CMake project
* support for Qt 4 .ui files
but it's better than nothing. We need to drop the hard KDE 3 deps to get around 
the -devel conflict to allow that use though.)

Comment 16 Rex Dieter 2007-07-20 16:46:44 UTC
generate -apidocs subpkg
keeping Requires: libkdnssd

* Fri Jul 20 2007 Rex Dieter <rdieter[AT]fedoraproject.org> - 6:3.5.7-15
- Obsoletes/Provides: kdelibs-apidocs (f8+)

Comment 17 Kevin Kofler 2007-07-20 19:35:37 UTC
Let's start with some fun from rpmlint:

Source RPM:
  > W: kdelibs3 strange-permission kde.sh 0755
  > W: kdelibs3 strange-permission kde.csh 0755
  Bad: While these are not strange for a shell script, these are /etc/profile.d 
scripts which are sourced, so they don't need executable perms (see below).
  > E: kdelibs3 invalid-spec-name kdelibs.spec
  Bad: The specfile needs renaming.
  > W: kdelibs3 mixed-use-of-spaces-and-tabs (spaces: line 25, tab: line 31)
  OK, who cares? ;-)
  > W: kdelibs3 patch-not-applied Patch139445: 
  > W: kdelibs3 patch-not-applied Patch41: kdelibs-3.5.6-utempter.patch
  > W: kdelibs3 patch-not-applied Patch502: kdelibs-3.5.0-101956.patch
  What about these?

i386 RPM:
  > W: kdelibs3 no-documentation
  Bad: At least the license should be shipped as %doc, possibly also other 
stuff (readme files etc.).
  > W: kdelibs3 devel-file-in-non-devel-package /usr/bin/kde-config
  OK, kde-config is used by some non-devel stuff too AFAIK, so this is normal.
  > E: kdelibs3 executable-marked-as-config-file /etc/profile.d/kde3.csh
  > E: kdelibs3 executable-sourced-script /etc/profile.d/kde3.csh 0755
  > E: kdelibs3 executable-marked-as-config-file /etc/profile.d/kde3.sh
  > E: kdelibs3 executable-sourced-script /etc/profile.d/kde3.sh 0755
  Bad: These should probably not be executable.
  > W: kdelibs3 dangling-relative-symlink /usr/bin/kfmexec kioexec
  OK, kioexec is in kdebase 4, and the symlink is needed for backwards 
  > E: kdelibs3 setuid-binary /usr/bin/kgrantpty root 04755
  > E: kdelibs3 non-standard-executable-perm /usr/bin/kgrantpty 04755
  > W: kdelibs3 incoherent-version-in-changelog 6:3.5.7-14 3.5.7-14.fc8
  OK, someone needs to teach rpmlint about epochs and disttags...
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_kaddprinterwizard.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_klauncher.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_kio_http_cache_cleaner.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_kbuildsycoca.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_kcmshell.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_kio_uiserver.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_kcookiejar.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_kded.so libkdeinit_kded.so
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_kconf_update.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_cupsdconf.so 
  > E: kdelibs3 invalid-soname /usr/lib/libkdeinit_dcopserver.so 
  OK, rpmlint doesn't understand kdeinit either.
  > E: kdelibs3 
file-in-usr-marked-as-conffile /usr/share/config/ui/ui_standards.rc
  OK too.
  > E: kdelibs3 
invalid-desktopfile /tmp/kdelibs3-3.5.7-14.fc8.i386.rpm.16144/usr/share/applications/kde/kresources.desktop
  Not sure about this one.

i386 -devel RPM:
  > W: kdelibs3-devel no-documentation
  OK, the documentation is in -apidocs where it belongs to.
  > W: kdelibs3-devel spurious-executable-perm /usr/lib/libkdefakes_nonpic.a
  Bad: Static libs aren't supposed to be executable, are they?
  > W: kdelibs3-devel summary-ended-with-dot Header files and documentation for 
compiling KDE applications.
  Bad, this is hairsplitting, but the guidelines explicitly say there should be 
no dot here. :-)

i386 -debuginfo RPM: no output

i386 -apidocs RPM: can't check because it hasn't been built yet (I'm looking 
at -14)

x86_64 RPMs: same as the corresponding i386 versions, plus some warnings about 
object file formats due to me running rpmlint on an i386 machine

Comment 18 Kevin Kofler 2007-07-20 20:26:11 UTC
I'm doing the review now. I'd like to discuss this file:
There are at least 2 bad things with this file:
1. It's a config file in /usr. The usual /usr/share/config problem. IMHO that's 
2. More seriously, this file conflicts with kdelibs 4! This is why Than made 
the compat patch you dropped. So the patch needs to be made to work, we can't 
import this without. While we're already patching this, we could also patch the 
location to a more FHS-compliant one, but that would have to be done on the KDE 
4 version too for consistency.

Comment 19 Kevin Kofler 2007-07-20 21:00:05 UTC
Now the fun part of the review: the manual checks (on -15). :-)

MUST Items:
! rpmlint output: see above (comment #17)
+ named and versioned according to the Package Naming Guidelines
! spec file name doesn't match base package name, please rename to 
+ Packaging Guidelines:
  + License LGPL OK, matches actual license
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
  + Complies with the FHS
  + proper changelog, tags, BuildRoot, Requires, BuildRequires, Summary, 
  ! except for the dot at the end of the -devel summary ;-)
  + no non-UTF-8 characters
  ! relevant documentation not included
    See rpmlint output.
  + RPM_OPT_FLAGS are used (%configure macro)
  + debuginfo package is valid
  * .la files:
    What's the status of these? Are they still needed with the lib loader 
  + I'll give the static library kdefakes_nonpic.a a pass, as I'm sure there's 
a reason this is in a .a file, and as creating a -static subpackage doesn't 
make sense because this lib is static only.
  + no duplicated system libraries
  + no rpaths (I ran readelf -d on the shared objects and binaries on both the 
i386 and x86_64 version)
  + giving the config file in /usr a pass, as KDE has always 
used /usr/share/config
  + no init scripts, so init script guideline doesn't apply
  + no GUI executables, so no .desktop file needed
  ! rpmlint doesn't like the one included desktop file (kresources.desktop, 
which calls kcmshell kresources). It isn't being installed according to the 
guidelines either (desktop-file-install).
  + no timestamp-clobbering file commands
  + _smp_mflags used
  + scriptlets are valid
  + not a web application, so web application guideline doesn't apply
  ! conflicts:
    + the -devel conflict (explicit Conflicts) is OK
    ! but the file conflict on /usr/share/config/ui/ui_standards.rc with the 
KDE 4 kdelibs isn't!
+ complies with all the legal guidelines
! license not included as %doc (see rpmlint output)
+ spec file written in American English
+ spec file is legible
+ source matches upstream:
  MD5SUM: 50ed644f2ec91963570fe2b155652957
  SHA1SUM: 45f278311f20d2eb317f2175259f861c0bcf17a9
+ builds on at least one arch (F8 i386 mock, F8 x86_64 mock)
+ no known non-working arches, so no ExcludeArch needed
+ all required BuildRequires listed (it wouldn't build in mock otherwise ;-) )
+ no translations in original tarball, so translation/locale guidelines don't 
  ! That makes me think: Do we need a compat kde-i18n too? Looks like we 
do. :-(
+ ldconfig correctly called in %post and %postun
+ package not relocatable
+ ownership correct (owns package-specific directories, doesn't own directories 
owned by another package)
! duplicate files in %files: %{_bindir}/dcopidl* (in -devel) not excluded 
from %{_bindir}/* in main package
! permissions: see rpmlint output
+ %clean section present and correct
+ macros used where possible
+ no non-code content
+ large API docs are already in -apidocs
+ no %doc files, so no possible issues with %doc files required at runtime
+ all header files in -devel
+ no -static package needed
+ no .pc files, so no Requires: pkgconfig needed
+ /usr/lib/*.so symlinks are correctly in -devel
+ /usr/lib/kde3/*.so plugins and /usr/lib/kdeinit_*.so (NOT symlinks) are 
correctly NOT in -devel
+ -devel requires %{name} = %{version}-%{release}
* .la files: These are OK iff they're really needed, thus my question whether 
they really are. ;-)
+ no GUI programs, so no .desktop file needed
+ buildroot is deleted at the beginning of %install
  (nitpick: mkdir $RPM_BUILD_ROOT to protect against symlink attack missing 
here too)
+ all filenames are valid UTF-8

+ license already included upstream
+ no translations for description and summary provided by upstream
+ package builds in mock (Rawhide i386 and x86_64)
* Skipping the "all architectures" test, no access to PPC.
* Not testing if package functions as expected.
+ scriptlets are sane
+ -apidocs subpackage Requires: %{name} = %{?epoch:%{epoch}:}:%{version} which 
is OK
+ no .pc files, so "placement of .pc files" is irrelevant
+ no file dependencies

To sum up:
* Can you please address the issues found by rpmlint?
* Are the .la files still needed with the lib loader patch?
* The file conflict on ui_resources.rc needs fixing.
* kresources.desktop isn't installed according to the guidelines, and isn't 
valid according to rpmlint.
* please %exclude %{_bindir}/dcopidl* from the main package

Comment 20 Rex Dieter 2007-07-21 01:04:45 UTC
* lib_loader patch doesn't apply, besides, most .la files are still required anyway.
* ui_resources.rc: I installed kdelibs-3.91... and k;delibs3 side-by-side, and
didn't see any file conflict wrt ui_resources.rc (did you?).  If this is indeed
a problem, we need a fixed patch (the provide one from Than didn't build).

MUST items remaining todo:
* rpmlint issues... I can sort that out, I'd prefer if you listed explicitly
what you require.
* kresources.desktop: I doubt this is really a prob, it's a kde-only thing, that
probably contains keywords are something not technically valid according to
spec.  Fixing this may well require patching kde.  imo, not worth it.
* exclude dcopidl: can do

Comment 21 Kevin Kofler 2007-07-21 01:14:23 UTC
> * lib_loader patch doesn't apply, besides, most .la files are still required 
> anyway.


> * ui_resources.rc: I installed kdelibs-3.91... and k;delibs3 side-by-side,
> and didn't see any file conflict wrt ui_resources.rc (did you?).  If this is
> indeed a problem, we need a fixed patch (the provide one from Than didn't
> build).

Maybe we get away with the conflict because it's a %config file?
I'll take you word for it that it installs anyway. We need to look at the 
problem sooner or later, but let's not make this a blocker for importing.

> MUST items remaining todo:
> * rpmlint issues... I can sort that out, I'd prefer if you listed explicitly
> what you require.

See list of TODOs below.

> * kresources.desktop: I doubt this is really a prob, it's a kde-only thing,
> that probably contains keywords are something not technically valid according
> to spec.  Fixing this may well require patching kde.  imo, not worth it.

Then let's give it a pass.

> * exclude dcopidl: can do


* remove execute permissions from /etc/profile.d/kde3.*sh
* add %doc COPYING
* remove dot at the end of -devel summary
* exclude dcopidl* from main package
* rename the specfile to the correct name

As these are all minor issues, I'm going ahead and approving this, but please 
address them before or immediately after the import.


Comment 22 Rex Dieter 2007-07-21 01:24:35 UTC
Thanks Kevin.

New Package CVS Request
Package Name: kdelibs3
Short Description: K Desktop Environment 3 - Libraries
Owners: than@redhat.com,rdieter@math.unl.edu

Comment 23 Kevin Kofler 2007-07-21 01:33:45 UTC
Whoops, correcting myself:
> * add %doc COPYING
Actually, we want COPYING.LIB and COPYING.BSD there. I don't know why there's 
even a copy of the ordinary GPL in kdelibs.

Comment 24 Rex Dieter 2007-12-17 15:38:15 UTC
closing, it's been in rawhide a little while now.

Comment 25 Jaroslav Reznik 2015-04-02 12:10:31 UTC
Package Change Request
Package Name: kdelibs3
New Branches: epel7
Owners: jreznik

Comment 26 Jaroslav Reznik 2015-04-02 13:35:02 UTC
To clarify this request - I'm KDE SIG member although not listed as co-maintainer for this package and this request was approved by rdieter and than, kkofler allows it explicitly.

Comment 27 Patrick Uiterwijk 2015-04-02 14:44:30 UTC
Git done (by process-git-requests).

Thanks for the explanation.

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