make services disqualification much stricter
ClosedPublic

Authored by sitter on Feb 2 2017, 6:06 PM.

Details

Summary

after the recent set of changes to disqualification we ended up with a bit
too lax disqualification. if we saw the exec OR the storageid we'd
hence forth disqualify a service with either of them being equal.

in case of system settings this causes a match problem. systemsettings
has two desktop files one !KDE and one OnlyKDE they are however exactly
the same except for a different name and storageid. as a result if the !KDE
one is encountered first it will be disqualified on account of
noDisplay=true and marked as seen. when the OnlyKDE systemsettings is then
iterated it will already have the Exec entry in the seen list and be
disqualified on account of that.

to prevent this type of confusion make it a requirement to match both
storageid AND exec before disqualifying.

it may actually be suitable to drop the exec altogether. say I copy firefox
into the user local applications dir and set it NoDisplay=true but change
the exec. this would still lead to the system-wide firefox being found as
it is not getting disqualified. long story short: maybe we should
disqualify services solely on the storageid?

Test Plan
  • new autotest case
  • fails without changes
  • passes with changes

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter updated this revision to Diff 10863.Feb 2 2017, 6:06 PM
sitter retitled this revision from to make services disqualification much stricter.
sitter updated this object.
sitter edited the test plan for this revision. (Show Details)
sitter added a reviewer: broulik.
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 2 2017, 6:06 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sitter updated this revision to Diff 10865.Feb 2 2017, 6:57 PM

dont ignore setenv's return value, verify it

broulik accepted this revision.Feb 3 2017, 9:50 AM
broulik edited edge metadata.
This revision is now accepted and ready to land.Feb 3 2017, 9:50 AM
This revision was automatically updated to reflect the committed changes.