D7255: Remove application directory from QCoreApplication::libraryPaths()
This revision was automatically updated to reflect the committed changes. Closed by commit R120:cf572c3b5224: Remove application directory from QCoreApplication::libraryPaths() (authored by fvogt). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7255?vs=18013=18282 REVISION DETAIL https://phabricator.kde.org/D7255 AFFECTED FILES shell/main.cpp To: fvogt, #plasma, mart Cc: broulik, graesslin, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7255: Remove application directory from QCoreApplication::libraryPaths()
mart accepted this revision. This revision is now accepted and ready to land. REPOSITORY R120 Plasma Workspace BRANCH libfix REVISION DETAIL https://phabricator.kde.org/D7255 To: fvogt, #plasma, mart Cc: broulik, graesslin, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7255: Remove application directory from QCoreApplication::libraryPaths()
fvogt added a comment. In https://phabricator.kde.org/D7255#134745, @broulik wrote: > > I'll look whether it's possible to override that in a sane place as well. > > KDeclarative QmlObject, maybe? I think the single QuickViewSharedEngine used in most of Plasma should already give a significant improvement. I'd rather not change that in any framework, as applications linking to it can be deployed in whatever way they prefer. IMO it should be specified by the application itself. This diff to plasma-framework seems to work for some quick experiments (pun not intended) though: diff --git a/src/plasmaquick/appletquickitem.cpp b/src/plasmaquick/appletquickitem.cpp index 39facc613..29f0f7e69 100644 --- a/src/plasmaquick/appletquickitem.cpp +++ b/src/plasmaquick/appletquickitem.cpp @@ -66,6 +66,12 @@ void AppletQuickItemPrivate::init() PackageUrlInterceptor *interceptor = new PackageUrlInterceptor(qmlObject->engine(), Plasma::Package()); qmlObject->engine()->setUrlInterceptor(interceptor); } + +// Remove the unnecessary applicationDirPath from the front of the import path list +auto importPathList = qmlObject->engine()->importPathList(); +// Only set the import path if it actually changed, we might have removed it already. +if (importPathList.removeAll(QCoreApplication::applicationDirPath()) > 0) +qmlObject->engine()->setImportPathList(importPathList); } void AppletQuickItemPrivate::connectLayoutAttached(QObject *item) REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D7255 To: fvogt, #plasma Cc: broulik, graesslin, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7255: Remove application directory from QCoreApplication::libraryPaths()
broulik added a comment. > I'll look whether it's possible to override that in a sane place as well. KDeclarative QmlObject, maybe? I think the single QuickViewSharedEngine used in most of Plasma should already give a significant improvement. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D7255 To: fvogt, #plasma Cc: broulik, graesslin, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7255: Remove application directory from QCoreApplication::libraryPaths()
fvogt added a comment.
In https://phabricator.kde.org/D7255#134678, @graesslin wrote:
> Reading through your description I think that KWin must also be affected
and many, many more applications. Actually probably everything distributed in
the classic "sysadmin style" (to reference a today's blog post). I'm wondering
whether it would make sense to have this in either Qt directly (maybe not?) or
a wrapper in KCoreAddons which can easily handle situations like run as
flatpack where it's not needed or not wanted.
>
> Anyway: +1 to the change, that sounds awesome improvement.
It gets even "better": QQmlEngines have the applicationDirPath set as
first(!) entry of the importPathList as well by default, so strace gets spammed
with
[pid 21876] stat("/usr/bin/QtQuick.2.1", 0x7f8faf5a42b0) = -1 ENOENT (No
such file or directory)
[pid 21876] stat("/usr/bin/QtQuick.2", 0x7f8faf5a42b0) = -1 ENOENT (No such
file or directory)
[pid 21876] stat("/usr/bin/QtQuick/Controls.1.0", 0x7f8faf5a42b0) = -1
ENOENT (No such file or directory)
[pid 21876] stat("/usr/bin/QtQuick.1.0/Controls", 0x7f8faf5a42b0) = -1
ENOENT (No such file or directory)
I'll look whether it's possible to override that in a sane place as well.
I would also like to see this fixed directly in Qt as well, but it's not so
easy IMO.
The default addition of the path to the various path lists is only useful for
applications shipped as a single directory,
not supposed to be installed linux-style. I'm not sure how Qt could
distinguish those cases... At least I'm against hardcoding
blacklisted paths inside Qt.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D7255
To: fvogt, #plasma
Cc: graesslin, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart, lukas
D7255: Remove application directory from QCoreApplication::libraryPaths()
graesslin added a comment. Reading through your description I think that KWin must also be affected and many, many more applications. Actually probably everything distributed in the classic "sysadmin style" (to reference a today's blog post). I'm wondering whether it would make sense to have this in either Qt directly (maybe not?) or a wrapper in KCoreAddons which can easily handle situations like run as flatpack where it's not needed or not wanted. Anyway: +1 to the change, that sounds awesome improvement. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D7255 To: fvogt, #plasma Cc: graesslin, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D7255: Remove application directory from QCoreApplication::libraryPaths()
fvogt created this revision. Restricted Application added a project: Plasma. REVISION SUMMARY The directory containing the main application (read through argv[0]) is by default added to the list of paths containing libraries. This causes various methods to iterate through all entries of the plasmashell install location, usually /usr/bin. By explicitly removing the path, those unnecessary lookups can be avoided, resulting in around 100ms quicker startup on a system with 4000 entries in /usr/bin. TEST PLAN Ran plasmashell with and without this fix, no changes except for a slightly quicker startup and much less strace noise. REPOSITORY R120 Plasma Workspace BRANCH libfix REVISION DETAIL https://phabricator.kde.org/D7255 AFFECTED FILES shell/main.cpp To: fvogt, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
