<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/125192/">https://git.reviewboard.kde.org/r/125192/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 13th, 2015, 9:27 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/125192/diff/1/?file=402974#file402974line187" style="color: black; font-weight: bold; text-decoration: underline;">modules/ECMInstallIcons.cmake</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">187</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c"># Updates the mtime of the icon theme directory, so caches that</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">187</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c"># Updates the mtime of the icon theme directory, so caches that</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Olivier, I was wrong! You can remove the recursive mtime check from Qt, if the rule is always that any icon installation must touch the toplevel dir. Which clearly we were doing already. Sorry for the wrong advice on my part.</p></pre>
</blockquote>
<p>On September 14th, 2015, 4:26 p.m. UTC, <b>Olivier Goffart</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">David: but does the package manager that contains icons will touch the theme directory?</p></pre>
</blockquote>
<p>On September 15th, 2015, 9:50 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ah, good question. No idea though. That's a question for release-team@kde.org, where the packagers will be able to answer you.</p></pre>
</blockquote>
<p>On September 16th, 2015, 9:19 p.m. UTC, <b>Maciej Mrozowski</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Icon cache is cretaed at package install phase by package manager. cmake install (where you invoke gtk-update-icon-cache) is source install phase. Source install phase happens in sandboxed environment most of the time so runnig gtk update icon cache will just work for kdebuild and local installations. For distro packagers this will be a command to sed-out from CMakeLists.</p></pre>
</blockquote>
<p>On September 16th, 2015, 9:44 p.m. UTC, <b>Maciej Mrozowski</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Small update. Icon cache is movable (does not contain absolute paths) so icon cache will work when copied from package installation image to target filesystem. Still generating it in buildsystem is stepping a bit too far into packager area. With my Gentoo hat on. Worth asking jriddel what he thinks.</p></pre>
</blockquote>
<p>On September 17th, 2015, 7:33 a.m. UTC, <b>Volker Krause</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is purely meant for developer builds, not distros. There this is better done in post install hooks indeed. The DESTDIR check around this should disable this for normal distro builds, same as we do already for e.g. updating the mime database.</p></pre>
</blockquote>
<p>On September 19th, 2015, 9:29 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And this is all somewhat besides the point. The question wasn't "can/should packages run gtk-update-icon-cache in the future" (the answer is yes).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The question is... when you install some package that was made in the last 8 years and that isn't about a GTK application, let's say firefox, libreoffice or whatever else,
if the package contains icons, does the package manager touch the toplevel icon theme directory or not.
If not, then we are going to miss the new icons. But then again, I suppose GTK would miss them too..... So I guess packages do update the dir (or even run the gtk tool).</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry David, I missed that this question was still open. Yes, this can be messed up in packaging. The distros we looked at so far either run gtk-update-icon-cache in post install hooks correctly (ie. same way as update-mime-db, whenever a package installs stuff into the shared locations), or not at all. Compromising performance for a hypothetical distro with broken packaging looks like an unnecessary pessimisation IMHO.</p></pre>
<br />
<p>- Volker</p>
<br />
<p>On September 12th, 2015, 12:23 p.m. UTC, Volker Krause wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Extra Cmake Modules and Olivier Goffart.</div>
<div>By Volker Krause.</div>
<p style="color: grey;"><i>Updated Sept. 12, 2015, 12:23 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
extra-cmake-modules
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Despite the name, Qt is also using this, and it considerably speeds up
icon lookup.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Built and installed kdepim, cache files are generated in the expected places.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>modules/ECMInstallIcons.cmake <span style="color: grey">(79dc5150e8a966db2a9fdd39cbd5ce8c2f842e18)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125192/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>