Review Request 126711: ECMAddAppIcon: Use absolute path when operating on icons

[Gleb Popov]

> On Jan. 11, 2016, 5:53 p.m., Alex Merry wrote:
> > The commands that deal with the icons are run from CMAKE_CURRENT_SOURCE_DIR with the intention of making relative paths work. I take it this isn't sufficient?
> > 
> > It would be nice to have tests for this module, including ones covering this case. I realise it would be hard to check that icons are embedded properly, but it would be able to check that it didn't fail, and that it generated the files that it should, and updated the variable that was passed to it. Is this something you would be willing to look at?

>The commands that deal with the icons are run from CMAKE_CURRENT_SOURCE_DIR with the intention of making relative paths work. I take it this isn't sufficient?

Yes, the `if(EXISTS $[icon}` check was failing for relative path. This is why Kate, for instance, prepends ${CMAKE_CURRENT_SOURCE_DIR} for its icons. See https://quickgit.kde.org/?p=kate.git&a=blob&h=3b24e088073f7f8893c0bec4d58894957982f28d&hb=371e7a2fda5fe5e98173020cd8553b2181e125c9&f=kate%2FCMakeLists.txt#l75

>It would be nice to have tests for this module
>Is this something you would be willing to look at?

Not willing, to be honest, but can do this if you want.


- Gleb


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126711/#review90901
-----------------------------------------------------------


On Jan. 10, 2016, 11 p.m., Gleb Popov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126711/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2016, 11 p.m.)
> 
> 
> Review request for Extra Cmake Modules and Kevin Funk.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> This makes it optional to list icons with ${CMAKE_CURRENT_SOURCE_DIR}/ prefix.
> 
> 
> Diffs
> -----
> 
>   modules/ECMAddAppIcon.cmake 5233a5f 
> 
> Diff: https://git.reviewboard.kde.org/r/126711/diff/
> 
> 
> Testing
> -------
> 
> Built KDevelop on Windows, the icon is here.
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20160111/7a41724a/attachment.html>