<div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace;font-size:small"><br></div><div class="gmail_extra"><br><div class="gmail_quote">On 2 February 2017 at 00:49, Harald Sitter <span dir="ltr"><<a href="mailto:sitter@kde.org" target="_blank">sitter@kde.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hola<br>
<br>
breeze-icons uses lots of symlinks. Unfortunately, ever so often our<br>
icon designers make a mistake and create a bad symlink. To mitigate<br>
this I added a bunch of tests making sure everything is nice and<br>
dandy.<br>
<br>
In the mean parts of the build were changed to not tolerate broken<br>
symlinks. While I don't really have a problem with that in of itself,<br>
the code largely simply ignores the possibility of broken symlinks and<br>
fails with the most shitty error messages you could think of. Given<br>
our artists are not software engineers they are having a hard time<br>
figuring out what is going on. And TBH, I too had to stat files to get<br>
to the bottom of the errors. This is a fairly shit situation as on the<br>
one hand we want lovely icons and on the other hand the people working<br>
on the icons can't understand what needs fixing without having to find<br>
a developer they aren't too afraid of to talk to.<br>
<br>
This really needs fixing.<br>
<br>
Notably offenders I had a fight with today:<br>
<br>
<br>
# breeze-validate-svg (introduced by Jos)<br>
<br>
This is a bash script running xmllint.<br>
<br>
## Problem 1: Sources<br>
<br>
The custom target sets `SOURCES ${SVGS}` this has no notable advantage<br>
other than making the svgs show up in an IDE, it does however mean<br>
that cmake will try to inspect them (as a build tool does when they<br>
get told something is a source) and then falls flat on the face when<br>
it encounters a broken symlink as it now can't determine the source<br>
type resulting in this lovely error<br>
<br>
```<br>
21:39:07 CMake Error at CMakeLists.txt:70 (add_custom_target):<br>
21:39:07   Cannot find source file:<br>
21:39:07<br>
21:39:07     /home/jenkins/sources/breeze-<wbr>icons/kf5-qt5/icons-dark/<wbr>categories/32/applications-<wbr>other.svg<br>
```<br>
<br>
## Problem 2: The code<br>
<br>
The bash code in of itself runs find with -exec on xmllint. Problem<br>
being that if the symlink is broken xmllint will (rightfully) complain<br>
<br>
```<br>
warning: failed to load external entity<br>
"./icons-dark/categories/32/<wbr>applications-other.svg"<br>
```<br>
<br>
While that is much better than the earlier problem it's still plenty<br>
unobvious what the underlying cause for this is. Supposedly the script<br>
should skip bogus symlinks.<br>
<br>
## Problem 3: Oh but really<br>
<br>
This isn't really related to the issue of bad symlink handling:<br>
<br>
- apparently this didn't get a review. why ever would this not get a review?<br>
- this should be a test and only run when testing is enabled<br>
- when xmllint is not present it should report this in some form or<br>
fashion during the test run so one knows linting is not done<br>
- it should record its complaints via ctest so jenkins can display them properly<br>
- I fail to appreciate the reason this needs to depend on bash (versus<br>
sh, or well, neither)<br>
<br>
## Fix Suggestion<br>
<br>
- don't needlesly set SOURCE<br>
- don't pass bad paths to xmllint<br>
- deal with stuff in problem 3<br>
<br>
<br>
# RCC generation (introduced by Gleb, enabled by Jaroslaw)<br>
<br>
This is a bunch of cmake rigging and helper binaries to assemble the<br>
icons into an rcc.<br>
<br>
## Problem<br>
<br>
`cmake -E copy_directory` is used to copy the src tree of the themes<br>
into the build dir from which the resource file gets assembled. I am<br>
guessing copy_directory does not preserver, but resolve symlinks<br>
because it greets us with the ever so opaque error:<br>
<br>
```<br>
Error copying directory from<br>
"/home/me/src/git/breeze-<wbr>icons/icons-dark" to<br>
"/home/me/src/git/breeze-<wbr>icons/build/icon<br>
s-dark/res".<br>
```<br>
<br>
## Fix Suggestion<br>
<br>
This is slightly less trivial since the rcc/qrc helpers seem to depend<br>
on resolved symlinks, so I am guessing a way to deal with this would<br>
be to use cmake's `file(COPY...)` instead of copy_directory and then<br>
have another helper run through the directories to flatten out the<br>
symlinks (dropping broken symlinks).<br>
<br></blockquote><div><br><div style="font-family:monospace,monospace;font-size:small;display:inline" class="gmail_default">​Thanks for so detailed research, Harald. <br></div><div style="font-family:monospace,monospace;font-size:small;display:inline" class="gmail_default">For the problem #3 I am wondering why the copying should be performed at all if a​</div> <div style="font-family:monospace,monospace;font-size:small;display:inline" class="gmail_default">​symlink is invalid. If I understand correctly, how about checking the symlinks first and if there are no issues, copying which will go OK?<br></div><div style="font-family:monospace,monospace;font-size:small;display:inline" class="gmail_default">That's assuming the symlinks check is not a part of autotests but the actual build.<br></div></div></div><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">regards, Jaroslaw Staniek<br><br>KDE:<br>: A world-wide network of software engineers, artists, writers, translators<br>: and facilitators committed to Free Software development - <a href="http://kde.org" target="_blank">http://kde.org</a><br>Calligra Suite:<br>: A graphic art and office suite - <a href="http://calligra.org" target="_blank">http://calligra.org</a><br>Kexi:<br>: A visual database apps builder - <a href="http://calligra.org/kexi" target="_blank">http://calligra.org/kexi</a><br>Qt Certified Specialist:<br>: <a href="http://www.linkedin.com/in/jstaniek" target="_blank">http://www.linkedin.com/in/jstaniek</a></div>
</div></div>