Bug 316727 - using icon which is not in hicolor
Summary: using icon which is not in hicolor
Status: RESOLVED INTENTIONAL
Alias: None
Product: telepathy
Classification: Unmaintained
Component: contactlist (show other bugs)
Version: 0.5.80
Platform: Other Linux
: NOR normal
Target Milestone: Future
Assignee: Telepathy Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-14 17:05 UTC by Alin M Elena
Modified: 2013-03-16 20:39 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
KDE without oxygen (110.14 KB, image/png)
2013-03-16 16:46 UTC, Christophe Marin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alin M Elena 2013-03-14 17:05:01 UTC
I get this error

[  308s] ERROR: Icon file not installed: /home/abuild/rpmbuild/BUILDROOT/ktp-contact-list-0.6.66.git.1363267847-178.1.x86_64//usr/share/applications/kde4/ktp-contactlist.desktop (telepathy-kde)
[  308s] Errors in installed desktop file detected. Please refer to http://en.opensuse.org/SUSE_Package_Conventions/RPM_Macros
[  308s] error: Bad exit status from /var/tmp/rpm-tmp.duRBJH (%install)

which seems to be correct at least according to this
the error seems tobe correct at least according to my understanding of this 
http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html#install_icons

Alin




Reproducible: Always
Comment 1 Alin M Elena 2013-03-15 08:09:16 UTC
numpty reporter ignore the bug...
was a packaging issue.
Comment 2 Christophe Marin 2013-03-16 10:40:06 UTC
reopening, this is a valid issue. kwhiteboard is also affected.

To summarize :

- One application = one icon (using the same icon multiple time is a lazy and wrong approach
- http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html#install_icons indicates that icons have to be installed in the hicolor namespace.

I'll fix kwhiteboard and ktp-contact-list
Comment 3 Martin Klapetek 2013-03-16 10:43:34 UTC
What's wrong with reusing the same icon?
Comment 4 Christophe Marin 2013-03-16 11:05:05 UTC
reusing the same icon is fine (until you get something better for $givenApp), it's reusing the same icon name that's incorrect.

From a packaging pov, we have validators that ensure desktop files icons are installed (what Alin pasted in his first comment)

It detects that the telepathy-kde icon is not installed when building kwhitebard, ktp-contact-list, ktp-ssh-contact and maybe other ones and throws an error.

This is not a false positive, the ktp-common-internals icons are not supposed to be build dependencies. We only need headers and libktp*.so
Comment 5 David Edmundson 2013-03-16 12:53:04 UTC
kwhiteabord and ktp-ssh-contact are unreleased pieces of software.
We do not endorse packaging them for distributions.
Comment 6 Martin Klapetek 2013-03-16 13:00:19 UTC
> This is not a false positive, the ktp-common-internals icons are not supposed to be build 
> dependencies. We only need headers and libktp*.so

They are not build deps, but rather runtime deps (and optional, even). Also your checks should detect the icon being installed if you need ktp-common-internals before anything else, therefore the icons will always be installed in time of your checks, no?
Comment 7 Christophe Marin 2013-03-16 13:06:56 UTC
that's the issue, a given application should not use another application's icons (same name and installed by something else)
Comment 8 David Edmundson 2013-03-16 13:19:03 UTC
I've got confused.

We're not using another apps icon, we're using our icon from multiple places. Can you explain why this is wrong.
Comment 9 Rex Dieter 2013-03-16 14:06:27 UTC
As another distro packager, let me provide my own opinion and suggest this is not really a problem or bug in practice.

Let me see if I can summarize Issues raised here:
1.  apps resusing the same icon(s)
2.  apps using icons not necessarily available at build-time


1.   The statement: "that's the issue, a given application should not use another application's icons (same name and installed by something else)"  while nice in practice, and something to strive for, is not strictly a bug

2.  On the one hand, it is argued that 
"ktp-common-internals icons are not supposed to be build dependencies. We only need headers and libktp*.so"
then you turn around and claim it's a bug that they are not present due to how your distro packaging is done.  In short, the icons not being present at build time is a matter of your own choosing.  This can be fixed by fixing your packaging quite easily, and I humbly suggest you do so.
Comment 10 Christophe Marin 2013-03-16 14:11:22 UTC
to make it easier to understand:
* one application = (at least) one icon with a distinct name
* one application doesn't depend on a 3rd party dependencies/modules/whatever but install its icon(s).

What happens currently (from a packager pov):
ktp-common-internals is currently split in 3:
- ktp-common-internals which is the runtime package (contains the icons, qml stuff and so on)
- ktp-common-internals-devel which contains the headers and .so files
- libktpcommoninternals$SOVERSION which contains the runtime libraries.

The icons are logically in the runtime module. Since it's a KDE application, it has a hard dependency on kdebase-runtime and (implicitely) all kdebase-runtime's dependencies

If we add a build requirement on the package providing icons (which, again, is a bad idea since it's only a workaround), we have to wait for this package dependencies to be available. This means waiting for kdebase-runtime which itself might be waiting for it's dependencies to be built.

The correct situation would be: kwhiteboard is shipped with its icons and only depends on ktp-common-internals-devel (which then just needs kdelibs and the telepathy build dependencies instead of a bunch of runtime stuff).

This has a double benefit: makes rebuild faster and follows the specs.
Comment 11 Christophe Marin 2013-03-16 14:12:41 UTC
@Rex: Of course, I could fix it at packaging level by just removing the Icon lines from the concerned .desktop files and I'll happily send users to this bug report when they'll notice.
Comment 12 Rex Dieter 2013-03-16 15:07:15 UTC
The same argument could be made about all kde apps and oxygen-icons.  You're saying it's not ok to depend on oxygen-icons either?
Comment 13 Rex Dieter 2013-03-16 15:11:59 UTC
In short, your assertion:
- ktp-common-internals which is the runtime package (contains the icons, qml stuff and so on)

is false.  At least according to you, since your tests insist this be present at *build* time.  Either fix your tests, or change your packaging
Comment 14 Christophe Marin 2013-03-16 16:46:27 UTC
Created attachment 78113 [details]
KDE without oxygen

(In reply to comment #12)
> The same argument could be made about all kde apps and oxygen-icons.  You're
> saying it's not ok to depend on oxygen-icons either?

of course it's not ok. What do you think happens if a Gnome user (using a Gnome icons theme) wants to use a single KDE application ?

solution 1: force him to install 100MB icons (slightly sub-optimal)
solution 2: Let him have generic icons everywhere
solution 3: Follow the icons spec

See the attached screenshot, the systemsettings and ktp-contact are what the user will see with solution #2
and the nice icons in the kickoff menu are what happens with solution #3 (these icons are installed by their respective applications in the hicolor namespace)
Comment 15 Christophe Marin 2013-03-16 16:56:14 UTC
(In reply to comment #14)

> solution 1: force him to install 100MB icons (slightly sub-optimal)

auto-correcting: the oxygen icons theme without the scalable ones is 30 MB and more than 250 MB with those ones
Comment 16 David Edmundson 2013-03-16 17:02:42 UTC
I'm still confused, the icons that screenshot shows missing aren't part of oxygen, they're installed into hicolor so what does this have to do with oxygen-icons being missing?
Comment 17 Rex Dieter 2013-03-16 17:49:26 UTC
David, I had used kde apps and oxygen-icons only as another example of a similar issue (where runtime icons may not necessarily be present at build time), it's not directly relevant here.
Comment 18 Rex Dieter 2013-03-16 17:52:40 UTC
though yes, Christophe, most non-trivial kde applications *do* expect oxygen-icons to be present at runtime, it's a clearly documented requirement.  is it unfortunate for non-kde users to have to install oxygen-icons for a single kde application?  yes, but it's an expected component of any robust kde runtime environment.
Comment 19 David Edmundson 2013-03-16 18:29:34 UTC
So lets keep this on track.

From our perspective our options are:

1) Copy the same binary files across n repositories , updating all references of KIcon() to load the relevant icon for that application. This would make updating very difficult.

2) Put the icons in a common place that we _know_ will be installed at run time.

It should be obvious why we are choosing option 2.

When (from source) you install common internals you also install the data folder which ships these icons. We know everything else depends on common-internals. If you as packagers split it, you need to enforce that behaviour still remains.

It is a false positive, you are checking for something that is needed at run time, at compile time. It can't ever be wrong on a user's machine without a packaging error.

Note that not having the data folder from ktp-common-internals at run time would also mean that notifications fail to work.

I don't want to discuss this any further.
Comment 20 Christophe Marin 2013-03-16 20:17:52 UTC
> It is a false positive, you are checking for something that is needed at run time, at compile time. 

It is a positive warning, we are checking something that has to be *installed by the application* (the situation is as ridiculous as using Icon=firefox in all or browsers and requiring it at build time just because there's a vague relation between all these apps)

> updating all references of KIcon() to load the relevant icon for that application.
Unrelated. The bug report is about .desktop files.
Comment 21 Christophe Marin 2013-03-16 20:19:16 UTC
Using a correct resolution. the bug is valid, you just don't want to fix it
Comment 22 Martin Klapetek 2013-03-16 20:39:53 UTC
> the situation is as ridiculous as using Icon=firefox in all or browsers and requiring it at build time just because there's a vague relation between all these apps

ktp-contact-list has both build AND runtime dependency on ktp-common-internals. There is simply no way to have contact list (and other components) and don't have ktp-common-internals. So that is not a vague relation.

Browsers nor other apps have (or would have) no such dependencies among them. So it's not the same.