Review Request 110670: fixes for qml import paths order

Oliver Henshaw oliver.henshaw at gmail.com
Mon May 27 17:01:59 BST 2013



> On May 27, 2013, 1:25 p.m., Lamarque Souza wrote:
> > ksmserver/shutdowndlg.cpp, line 235
> > <http://git.reviewboard.kde.org/r/110670/diff/1/?file=146608#file146608line235>
> >
> >     What is exactly the bug this patch is trying to solve? Is there any bug entry in bugzilla for it (or them)?

There's no bug filed but I can see (with lsof) that e.g. kscreenlocker_greet loads plasma libraries from the system when KGlobal::dirs()->findDirs("module", "imports") is ("/path/to/local/lib64/kde4/imports", "/usr/lib64/kde4/imports"). I thought that it might contribute to problems I'd been having with testing screenlocker qml changes, but my problem probably lies elsewhere.

Anyway, it's the same problem as that solved in kdelibs 400b9f2e9d10386bb175b6123fe0cdaafeaffe61 for KDeclarative.


- Oliver


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110670/#review33217
-----------------------------------------------------------


On May 27, 2013, 1:14 p.m., Oliver Henshaw wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110670/
> -----------------------------------------------------------
> 
> (Updated May 27, 2013, 1:14 p.m.)
> 
> 
> Review request for kde-workspace, kwin and Plasma.
> 
> 
> Description
> -------
> 
> QDeclarativeImportDatabase::addImportPath() prepends paths to the list, so paths must be added in reverse order (lowest preference -> highest preference).
> 
> To further complicate things, only paths not already in the list are added - hopefully this behaviour can be corrected in qt at some point. Nevertheless, these fixes should give the desired results with or without the further qt fix.
> 
> 
> NB: kwin/kcmkwin/kwintabbox/layoutpreview.cpp is also affected by this issue but is not yet included in this RR, as it needs extra fixes and proper testing.
> 
> NB #2: there may be other files affected outside kde-workspace, but I haven't checked.
> 
> 
> Branch for this RR can be found at clones/kde-workspace/oliverhenshaw/kde-workspace at review/qml-imports-v1 which comprises:
> 
> [1/3] Drop unneeded duplicate addImportPath
> 
> Let KDeclarative::setupBindings() add the import paths: it too takes
> paths from KGlobal::dirs()->findDirs("module", "imports"); it adds paths
> in the correct (reverse) order.
> 
> [2/3] Replace foreach with java-style iterator
> 
> In preparation for reversing the loop.
> 
> [3/3] add qml import paths in correct order
> 
> addImportPath prepends the path to importPathList so we must add our
> paths in reverse order.
> 
> Based on the fix for kdeclarative.cpp in kdelibs
> 400b9f2e9d10386bb175b6123fe0cdaafeaffe61
> 
> 
> Diffs
> -----
> 
>   ksmserver/screenlocker/greeter/greeterapp.cpp b70c9d6c005aa66d2f85a42ef4f1dcb04ea44667 
>   ksmserver/shutdowndlg.cpp 247c8777f060a614436b4f757ba5d588035f3bf5 
>   kwin/clients/aurorae/src/aurorae.cpp f7771c4cbee34194dcc8987a789712ab41898ec0 
>   kwin/kcmkwin/kwindecoration/kwindecoration.cpp 591a913c4032885474040e93e75d5f6f91bb9535 
>   kwin/scripting/scripting.cpp 13a77bc346b09f4b99f782370e8e0873332fad3d 
>   kwin/tabbox/declarative.cpp f759512c76a2b589323614b412844a0ce8e4dd54 
>   plasma/screensaver/shell/savercorona.cpp 9cd207ffc8039cd1e41fdbbf9a4095426824f694 
> 
> Diff: http://git.reviewboard.kde.org/r/110670/diff/
> 
> 
> Testing
> -------
> 
> Compiles and runs. Checked that the resulting importPathList is correct for ksmserver/screenlocker/greeter/greeterapp.cpp and for kwin/clients/aurorae/src/aurorae.cpp
> 
> 
> Thanks,
> 
> Oliver Henshaw
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130527/5881e353/attachment.htm>


More information about the kde-core-devel mailing list