Re: Review Request 110563: Crash fix: hide symbols from static lib QtUitools.a (generically by new macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS)

[Friedrich W. H. Kossebau]

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

(Updated May 22, 2013, 11:19 a.m.)


Review request for Build System, kdelibs, Alexander Neundorf, and Thiago 
Macieira.


Changes
---

Put "Crash" keyword in title, to get more attention.
Also set Alex and Thiago explicitely as reviewers.

Want to get this in ASAP, before 4.10.4


Summary (updated)
-

Crash fix: hide symbols from static lib QtUitools.a (generically by new macro 
KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS)


Description
---

Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple 
times into the same process (with Bsymbolic-functions flag)" on kde-core-devel 
( http://lists.kde.org/?t=13682986311&r=1&w=2 ) symbols from QtUitools.a 
are not hidden by default in Qt4 and thus will be added to the public symbols 
of the module/shared lib they are linked to. And thus can appear multiple times 
in the same process, resulting in symbol clashes and leading to problems at 
least with the Bsymbolic-functions flag or when being possibly incompatible 
versions.

Attached patch sees to solve that problem, by adding a macro 
KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags 
depending on the platform/linker used. 

Only issue is that instead of some variable I had to use "QtUiTools.a" as I 
found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} 
resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case 
another platform needs another name/prefix here?

Patch is against 4.10 branch, so I hope to get this in 4.10.4

http://lxr.kde.org/search?v=4.10-branch&filestring=&string=QT_QTUITOOLS_LIBRARY 
shows that there are some more places where the symbols need hiding, but I 
first want feedback on the proposed approach.


Diffs
-

  cmake/modules/FindKDE4Internal.cmake cb63285 
  cmake/modules/KDE4Macros.cmake 3db4e24 
  kjsembed/kjsembed/CMakeLists.txt d70f260 
  kross/modules/CMakeLists.txt d245fd8 
  kross/qts/CMakeLists.txt d8cb4a5 
  plasma/CMakeLists.txt 674550d 

Diff: http://git.reviewboard.kde.org/r/110563/diff/


Testing
---


Thanks,

Friedrich W. H. Kossebau

Re: Review Request 108845: add support for SSSE3 and SSE4.2 in cpufeatures and for msvc

[Patrick Spendrin]

> On May 6, 2013, 11:49 a.m., Kevin Ottens wrote:
> > My knowledge of the changes in the MMX/SSE landscape are slightly too 
> > limited for me to be 100% sure. But it looks like a fine patch.

I just at that patch again, and I just realized that I changed the flags for 
Linux too, so it might change some stuff there too. Did you actually look at 
this stuff, and can you check if this gives you SSE4.2 e.g. (probably on your 
computer available).


- Patrick


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


On Feb. 8, 2013, 1:09 a.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108845/
> ---
> 
> (Updated Feb. 8, 2013, 1:09 a.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs and Patrick von Reth.
> 
> 
> Description
> ---
> 
> This change implements cpu feature checks for MSVC. While at it, I added 
> support SSSE3 and SSE4.2 to the InstructionSets. I took the new values from 
> http://en.wikipedia.org/wiki/CPUID#EAX.3D1:_Processor_Info_and_Feature_Bits .
> 
> 
> Diffs
> -
> 
>   tier1/solid/src/solid/backends/shared/cpufeatures.cpp baa1af2 
>   tier1/solid/src/solid/processor.h ce4f0e1 
> 
> Diff: http://git.reviewboard.kde.org/r/108845/diff/
> 
> 
> Testing
> ---
> 
> Windows
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>

Re: Review Request 108845: add support for SSSE3 and SSE4.2 in cpufeatures and for msvc

[Lukáš Tinkl]

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



tier1/solid/src/solid/processor.h


Is it intentional that IntelSse4 and IntelSse41 have the same value?


- Lukáš Tinkl


On Feb. 8, 2013, 2:09 a.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108845/
> ---
> 
> (Updated Feb. 8, 2013, 2:09 a.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs and Patrick von Reth.
> 
> 
> Description
> ---
> 
> This change implements cpu feature checks for MSVC. While at it, I added 
> support SSSE3 and SSE4.2 to the InstructionSets. I took the new values from 
> http://en.wikipedia.org/wiki/CPUID#EAX.3D1:_Processor_Info_and_Feature_Bits .
> 
> 
> Diffs
> -
> 
>   tier1/solid/src/solid/backends/shared/cpufeatures.cpp baa1af2 
>   tier1/solid/src/solid/processor.h ce4f0e1 
> 
> Diff: http://git.reviewboard.kde.org/r/108845/diff/
> 
> 
> Testing
> ---
> 
> Windows
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>

Re: Review Request 110043: Proposed fix/workaround for legacy encoded filename handling

[Róbert Szókovács]

> On May 7, 2013, 10:11 a.m., Róbert Szókovács wrote:
> > The solution is intentionally "shy", I really don't want to fan the flames 
> > surrounding this issue. I just stumbled upon this location when it can be 
> > handled painlessly. Whether or not it should be turned on by default, in my 
> > opinion, can be left for distributors.
> 
> Thomas Lübking wrote:
> Then it's worthless.
> 
> When I encounter broken filenames on a rw device, i know it's time for a 
> fix.
> When I encounter broken filenames in joliet or rockridge (latter usually 
> caused by myself long ago - thank you, "wodim"...) i know it's time to mount 
> norock/nojoliet.
> Whether i do that or set a (KDE only affecting) env makes hardly a 
> difference.
> 
> When my little sister™ encounters broken filenames anywhere, she knows 
> that it's time to call her personal IT (me) with "these files won't open!" - 
> if she could not call me, she had no access to those files. Period.
> She won't think to google for "kde broken filenames", because she would 
> not think it's a "problem with the name" - the files have weird names, yes, 
> but essentially they won't open when she clicks them.
> That this could be due to some restrictions in UTF-8 and QString and 
> other terms she does not know, cannot be an expected consideration.
> 
> So either this is not a fixworthy issue at all, or it (as OPT-IN) only 
> becomes a way for distro discrimination (works on distro X but not on distro 
> Y) because fact is that the filenames are broken and if we want to assist in 
> that situation, we assist the unskilled *only* and the unskilled simply dont 
> set env vars. If they did, they were also skilled enough for convmv et al. to 
> deal with that issue correctly.
> 
> IOW *every* distro but Arch/Gentoo/LFS - ie. where you read a wiki for 
> setup - likely would *have* to set this anyway and those have the users to 
> turn it off at will.
> 
> /2¢
> 
> Róbert Szókovács wrote:
> OK, I'm all for making this on by default, but that would be a change 
> from the current situation, when the default is QFile's filename encoding, 
> basen on locale. If this becomes the default, it disrupts those who use a 
> non-UTF8 locale. The current code provides an enviroment variable to force 
> KDE to threat the filenames UTF8, this patch piggybacks that mechanism. 
> Should we check the locale the same way QFile does?
> 
> Thomas Lübking wrote:
> There should be no regression in regular use on non broken FS names for 
> no-one - not even those using non UTF-8 locales, so yes - testing the locale 
> to dis/enable this sounds reasonable.
> 
> Is the solution as simple as deactivating it if the tested env is set to 
> anything but "non_broken_names"?
> 
> Róbert Szókovács wrote:
> No, I'm affraid we would need heuristics similar to the one in QT, see 
> qtextcodec.cpp, setupLocaleMapper(): "Get the first nonempty value from 
> $LC_ALL, $LC_CTYPE, and $LANG environment variables.", then check the CODESET 
> part; if it's UTF8, enable this new functionality, otherwise do as before the 
> patch.

I uploaded the new version that checks the locale.


- Róbert


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


On May 17, 2013, 10:08 a.m., Róbert Szókovács wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110043/
> ---
> 
> (Updated May 17, 2013, 10:08 a.m.)
> 
> 
> Review request for kdelibs and Thiago Macieira.
> 
> 
> Description
> ---
> 
> This patch works around the problem of filenames that are not valid UTF8 
> strings:  in KLocalePrivate::initFileNameEncoding() KDE sets the QFile's 
> encoding/decoding function, to to/fromUTF8() in QString, which in turn calls 
> QUtf8's converter function (QUtf8 is not exported to developers, so I had to 
> use an inefficient method, I think it would be better if we could use the 
> state parameter for error detection). I replaced this with the said 
> functions' copy/pasted version and changed it, so when it encounters an 
> invalid UTF8 string, it will encode it byte by byte, mapping the lower 128 
> their normal unicode place and the upper 128 to U+18000-U+1807F, and of 
> course the decoder reverses it. 
> To make this actually work you have to define the KDE_UTF8_FILENAMES 
> enviroment variable to a specific value ("broken_names").
> 
> To test it, do the following: .kde/env/KDE_UTF8_FILENAMES.sh with this 
> content: 
> export KDE_UTF8_FILENAMES=broken_names
> logout, login, try dolphin on faulty files. (instead of the usual boxed "?" 
> you'll see just boxes)
> 
> 
> This addresses bug 165044.

Re: Review Request 108845: add support for SSSE3 and SSE4.2 in cpufeatures and for msvc

[Patrick von Reth]

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



tier1/solid/src/solid/backends/shared/cpufeatures.cpp


gcc will warn about undefed _MSVC_VER


- Patrick von Reth


On Feb. 8, 2013, 1:09 a.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108845/
> ---
> 
> (Updated Feb. 8, 2013, 1:09 a.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs and Patrick von Reth.
> 
> 
> Description
> ---
> 
> This change implements cpu feature checks for MSVC. While at it, I added 
> support SSSE3 and SSE4.2 to the InstructionSets. I took the new values from 
> http://en.wikipedia.org/wiki/CPUID#EAX.3D1:_Processor_Info_and_Feature_Bits .
> 
> 
> Diffs
> -
> 
>   tier1/solid/src/solid/backends/shared/cpufeatures.cpp baa1af2 
>   tier1/solid/src/solid/processor.h ce4f0e1 
> 
> Diff: http://git.reviewboard.kde.org/r/108845/diff/
> 
> 
> Testing
> ---
> 
> Windows
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>

Re: Review Request 108845: add support for SSSE3 and SSE4.2 in cpufeatures and for msvc

[Patrick Spendrin]

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

(Updated May 22, 2013, 3:31 p.m.)


Review request for KDE Frameworks, kdelibs and Patrick von Reth.


Description
---

This change implements cpu feature checks for MSVC. While at it, I added 
support SSSE3 and SSE4.2 to the InstructionSets. I took the new values from 
http://en.wikipedia.org/wiki/CPUID#EAX.3D1:_Processor_Info_and_Feature_Bits .


Diffs (updated)
-

  tier1/solid/src/solid/processor.h ce4f0e1 
  tier1/solid/src/solid/backends/shared/cpufeatures.cpp baa1af2 

Diff: http://git.reviewboard.kde.org/r/108845/diff/


Testing
---

Windows


Thanks,

Patrick Spendrin

Re: Review Request 110262: KRandomSequence::randomize: use the Fisher-Yates Algorithm to achieve O(N) complexity

[Commit Hook]

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


This review has been submitted with commit 
183e59b8e9fe938c209493aa7aa98382ccd08e71 by Frank Reininghaus to branch master.

- Commit Hook


On May 1, 2013, 8:34 p.m., Frank Reininghaus wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110262/
> ---
> 
> (Updated May 1, 2013, 8:34 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> ---
> 
> The current algorithm that is used to shuffle lists is rather inefficient. It 
> works by removing the first item of the list repeatedly and inserting it at a 
> random position in a new list, which is finally used to replace the original 
> list. Unfortunately, this results in O(N^2) run time complexity because 
> inserting into a list, which is done N itmes, is O(N).
> 
> I propose to replace this algorithm by the Fisher-Yates algorithm, which 
> works by swapping items N - 1 times. One could modify the entire thing 
> further, like providing randomization also for other containers and not only 
> QList, but that would probably be frameworks material. 
> 
> 
> Diffs
> -
> 
>   kdecore/util/krandomsequence.h 46949b4 
> 
> Diff: http://git.reviewboard.kde.org/r/110262/diff/
> 
> 
> Testing
> ---
> 
> I wrote a small benchmark: http://paste.kde.org/735914/
> 
> I got the following results with the existing algorithm:
> 
> RESULT : Benchmark::randomSequenceBenchmark():"n=0":
>  0.15 msecs per iteration (total: 66, iterations: 4194304)
> RESULT : Benchmark::randomSequenceBenchmark():"n=1":
>  0.000192 msecs per iteration (total: 101, iterations: 524288)
> RESULT : Benchmark::randomSequenceBenchmark():"n=3":
>  0.00070 msecs per iteration (total: 93, iterations: 131072)
> RESULT : Benchmark::randomSequenceBenchmark():"n=10":
>  0.0025 msecs per iteration (total: 83, iterations: 32768)
> RESULT : Benchmark::randomSequenceBenchmark():"n=30":
>  0.0070 msecs per iteration (total: 58, iterations: 8192)
> RESULT : Benchmark::randomSequenceBenchmark():"n=100":
>  0.023 msecs per iteration (total: 97, iterations: 4096)
> RESULT : Benchmark::randomSequenceBenchmark():"n=300":
>  0.077 msecs per iteration (total: 79, iterations: 1024)
> RESULT : Benchmark::randomSequenceBenchmark():"n=1000":
>  0.35 msecs per iteration (total: 90, iterations: 256)
> RESULT : Benchmark::randomSequenceBenchmark():"n=3000":
>  1.8 msecs per iteration (total: 58, iterations: 32)
> RESULT : Benchmark::randomSequenceBenchmark():"n=1":
>  18 msecs per iteration (total: 72, iterations: 4)
> RESULT : Benchmark::randomSequenceBenchmark():"n=3":
>  283 msecs per iteration (total: 283, iterations: 1)
> RESULT : Benchmark::randomSequenceBenchmark():"n=10":
>  3,823 msecs per iteration (total: 3,823, iterations: 1)
> 
> Here are the numbers for the proposed new algorithm:
> 
> RESULT : Benchmark::randomSequenceBenchmark():"n=0":
>  0.15 msecs per iteration (total: 65, iterations: 4194304)
> RESULT : Benchmark::randomSequenceBenchmark():"n=1":
>  0.15 msecs per iteration (total: 65, iterations: 4194304)
> RESULT : Benchmark::randomSequenceBenchmark():"n=3":
>  0.00018 msecs per iteration (total: 98, iterations: 524288)
> RESULT : Benchmark::randomSequenceBenchmark():"n=10":
>  0.00079 msecs per iteration (total: 52, iterations: 65536)
> RESULT : Benchmark::randomSequenceBenchmark():"n=30":
>  0.0025 msecs per iteration (total: 83, iterations: 32768)
> RESULT : Benchmark::randomSequenceBenchmark():"n=100":
>  0.0084 msecs per iteration (total: 69, iterations: 8192)
> RESULT : Benchmark::randomSequenceBenchmark():"n=300":
>  0.025 msecs per iteration (total: 52, iterations: 2048)
> RESULT : Benchmark::randomSequenceBenchmark():"n=1000":
>  0.085 msecs per iteration (total: 88, iterations: 1024)
> RESULT : Benchmark::randomSequenceBenchmark():"n=3000":
>  0.25 msecs per iteration (total: 66, iterations: 256)
> RESULT : Benchmark::randomSequenceBenchmark():"n=1":
>  0.85 msecs per iteration (total: 55, iterations: 64)
> RESULT : Benchmark::randomSequenceBenchmark():"n=3":
>  2.6 msecs per iteration (total: 86, iterations: 32)
> RESULT : Benchmark::randomSequenceBenchmark():"n=10":
>  10 msecs per iteration (total: 81, iterations: 8)
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

Re: Review Request 110262: KRandomSequence::randomize: use the Fisher-Yates Algorithm to achieve O(N) complexity

[Commit Hook]

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

(Updated May 22, 2013, 5:19 p.m.)


Status
--

This change has been marked as submitted.


Review request for kdelibs.


Description
---

The current algorithm that is used to shuffle lists is rather inefficient. It 
works by removing the first item of the list repeatedly and inserting it at a 
random position in a new list, which is finally used to replace the original 
list. Unfortunately, this results in O(N^2) run time complexity because 
inserting into a list, which is done N itmes, is O(N).

I propose to replace this algorithm by the Fisher-Yates algorithm, which works 
by swapping items N - 1 times. One could modify the entire thing further, like 
providing randomization also for other containers and not only QList, but that 
would probably be frameworks material. 


Diffs
-

  kdecore/util/krandomsequence.h 46949b4 

Diff: http://git.reviewboard.kde.org/r/110262/diff/


Testing
---

I wrote a small benchmark: http://paste.kde.org/735914/

I got the following results with the existing algorithm:

RESULT : Benchmark::randomSequenceBenchmark():"n=0":
 0.15 msecs per iteration (total: 66, iterations: 4194304)
RESULT : Benchmark::randomSequenceBenchmark():"n=1":
 0.000192 msecs per iteration (total: 101, iterations: 524288)
RESULT : Benchmark::randomSequenceBenchmark():"n=3":
 0.00070 msecs per iteration (total: 93, iterations: 131072)
RESULT : Benchmark::randomSequenceBenchmark():"n=10":
 0.0025 msecs per iteration (total: 83, iterations: 32768)
RESULT : Benchmark::randomSequenceBenchmark():"n=30":
 0.0070 msecs per iteration (total: 58, iterations: 8192)
RESULT : Benchmark::randomSequenceBenchmark():"n=100":
 0.023 msecs per iteration (total: 97, iterations: 4096)
RESULT : Benchmark::randomSequenceBenchmark():"n=300":
 0.077 msecs per iteration (total: 79, iterations: 1024)
RESULT : Benchmark::randomSequenceBenchmark():"n=1000":
 0.35 msecs per iteration (total: 90, iterations: 256)
RESULT : Benchmark::randomSequenceBenchmark():"n=3000":
 1.8 msecs per iteration (total: 58, iterations: 32)
RESULT : Benchmark::randomSequenceBenchmark():"n=1":
 18 msecs per iteration (total: 72, iterations: 4)
RESULT : Benchmark::randomSequenceBenchmark():"n=3":
 283 msecs per iteration (total: 283, iterations: 1)
RESULT : Benchmark::randomSequenceBenchmark():"n=10":
 3,823 msecs per iteration (total: 3,823, iterations: 1)

Here are the numbers for the proposed new algorithm:

RESULT : Benchmark::randomSequenceBenchmark():"n=0":
 0.15 msecs per iteration (total: 65, iterations: 4194304)
RESULT : Benchmark::randomSequenceBenchmark():"n=1":
 0.15 msecs per iteration (total: 65, iterations: 4194304)
RESULT : Benchmark::randomSequenceBenchmark():"n=3":
 0.00018 msecs per iteration (total: 98, iterations: 524288)
RESULT : Benchmark::randomSequenceBenchmark():"n=10":
 0.00079 msecs per iteration (total: 52, iterations: 65536)
RESULT : Benchmark::randomSequenceBenchmark():"n=30":
 0.0025 msecs per iteration (total: 83, iterations: 32768)
RESULT : Benchmark::randomSequenceBenchmark():"n=100":
 0.0084 msecs per iteration (total: 69, iterations: 8192)
RESULT : Benchmark::randomSequenceBenchmark():"n=300":
 0.025 msecs per iteration (total: 52, iterations: 2048)
RESULT : Benchmark::randomSequenceBenchmark():"n=1000":
 0.085 msecs per iteration (total: 88, iterations: 1024)
RESULT : Benchmark::randomSequenceBenchmark():"n=3000":
 0.25 msecs per iteration (total: 66, iterations: 256)
RESULT : Benchmark::randomSequenceBenchmark():"n=1":
 0.85 msecs per iteration (total: 55, iterations: 64)
RESULT : Benchmark::randomSequenceBenchmark():"n=3":
 2.6 msecs per iteration (total: 86, iterations: 32)
RESULT : Benchmark::randomSequenceBenchmark():"n=10":
 10 msecs per iteration (total: 81, iterations: 8)


Thanks,

Frank Reininghaus

Re: Review Request 110563: Crash fix: hide symbols from static lib QtUitools.a (generically by new macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS)

[Alexander Neundorf]

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


The documentation for the macro should be at the top of FindKDE4Internal.cmake, 
where all the documentation is.

For the technical side: this flag is new to me. Is it the only possible 
solution ? Thiago ?

- Alexander Neundorf


On May 22, 2013, 11:19 a.m., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110563/
> ---
> 
> (Updated May 22, 2013, 11:19 a.m.)
> 
> 
> Review request for Build System, kdelibs, Alexander Neundorf, and Thiago 
> Macieira.
> 
> 
> Description
> ---
> 
> Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple 
> times into the same process (with Bsymbolic-functions flag)" on 
> kde-core-devel ( http://lists.kde.org/?t=13682986311&r=1&w=2 ) symbols 
> from QtUitools.a are not hidden by default in Qt4 and thus will be added to 
> the public symbols of the module/shared lib they are linked to. And thus can 
> appear multiple times in the same process, resulting in symbol clashes and 
> leading to problems at least with the Bsymbolic-functions flag or when being 
> possibly incompatible versions.
> 
> Attached patch sees to solve that problem, by adding a macro 
> KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags 
> depending on the platform/linker used. 
> 
> Only issue is that instead of some variable I had to use "QtUiTools.a" as I 
> found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} 
> resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case 
> another platform needs another name/prefix here?
> 
> Patch is against 4.10 branch, so I hope to get this in 4.10.4
> 
> http://lxr.kde.org/search?v=4.10-branch&filestring=&string=QT_QTUITOOLS_LIBRARY
>  shows that there are some more places where the symbols need hiding, but I 
> first want feedback on the proposed approach.
> 
> 
> Diffs
> -
> 
>   cmake/modules/FindKDE4Internal.cmake cb63285 
>   cmake/modules/KDE4Macros.cmake 3db4e24 
>   kjsembed/kjsembed/CMakeLists.txt d70f260 
>   kross/modules/CMakeLists.txt d245fd8 
>   kross/qts/CMakeLists.txt d8cb4a5 
>   plasma/CMakeLists.txt 674550d 
> 
> Diff: http://git.reviewboard.kde.org/r/110563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

Re: Review Request 110563: Crash fix: hide symbols from static lib QtUitools.a (generically by new macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS)

[Friedrich W. H. Kossebau]

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

(Updated May 22, 2013, 6:45 p.m.)


Review request for Build System, kdelibs, Alexander Neundorf, and Thiago 
Macieira.


Changes
---

documentation for the macro moved to top of FindKDE4Internal.cmake


Description
---

Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple 
times into the same process (with Bsymbolic-functions flag)" on kde-core-devel 
( http://lists.kde.org/?t=13682986311&r=1&w=2 ) symbols from QtUitools.a 
are not hidden by default in Qt4 and thus will be added to the public symbols 
of the module/shared lib they are linked to. And thus can appear multiple times 
in the same process, resulting in symbol clashes and leading to problems at 
least with the Bsymbolic-functions flag or when being possibly incompatible 
versions.

Attached patch sees to solve that problem, by adding a macro 
KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags 
depending on the platform/linker used. 

Only issue is that instead of some variable I had to use "QtUiTools.a" as I 
found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} 
resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case 
another platform needs another name/prefix here?

Patch is against 4.10 branch, so I hope to get this in 4.10.4

http://lxr.kde.org/search?v=4.10-branch&filestring=&string=QT_QTUITOOLS_LIBRARY 
shows that there are some more places where the symbols need hiding, but I 
first want feedback on the proposed approach.


Diffs (updated)
-

  cmake/modules/FindKDE4Internal.cmake cb63285 
  cmake/modules/KDE4Macros.cmake 3db4e24 
  kjsembed/kjsembed/CMakeLists.txt d70f260 
  kross/modules/CMakeLists.txt d245fd8 
  kross/qts/CMakeLists.txt d8cb4a5 
  plasma/CMakeLists.txt 674550d 

Diff: http://git.reviewboard.kde.org/r/110563/diff/


Testing
---


Thanks,

Friedrich W. H. Kossebau

Re: Review Request 110563: Crash fix: hide symbols from static lib QtUitools.a (generically by new macro KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS)

[Thomas Lübking]

> On May 22, 2013, 6:28 p.m., Alexander Neundorf wrote:
> > The documentation for the macro should be at the top of 
> > FindKDE4Internal.cmake, where all the documentation is.
> > 
> > For the technical side: this flag is new to me. Is it the only possible 
> > solution ? Thiago ?

> For the technical side: this flag is new to me.
I wasn't aware it's used and grepping the Makefiles of kdelibs, workspace and 
runtime didn't show it here, btw.

What it does:
http://www.technovelty.org/c/what-exactly-does-bsymblic-do.html

My 2¢
There're various issues with it and i dare to claim that you more or less want 
to use it alongside --dynamic-list only.
Alternatively one would use __attribute__((visibility("[hidden|protected]"))) 
or the "-version-script" switch to protect/accelerate *certain* funtion/calls. 
(protected visibility is afair gcc only, though)

If "downstream" applies it, i'd frankly tell "downstream" to rtfm and not just 
push everything claimed to "make it fasta!!!"

This is by no means sth. one should apply "uninformed" since it has impact on 
the runtime behavior that the developer needs to know about ("whoops, my trick 
to shadow friend qt_foo() to gain access to protected/private d->bar only works 
sometiems" - yes, one should not hack, but one should also be aware that this 
hack can "legally" fail and not wonder why it works here and on distro X but 
fails on distro Y)


- Thomas


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


On May 22, 2013, 6:45 p.m., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110563/
> ---
> 
> (Updated May 22, 2013, 6:45 p.m.)
> 
> 
> Review request for Build System, kdelibs, Alexander Neundorf, and Thiago 
> Macieira.
> 
> 
> Description
> ---
> 
> Like discussed in the thread "Crashes with libQtUiTools.a if linked multiple 
> times into the same process (with Bsymbolic-functions flag)" on 
> kde-core-devel ( http://lists.kde.org/?t=13682986311&r=1&w=2 ) symbols 
> from QtUitools.a are not hidden by default in Qt4 and thus will be added to 
> the public symbols of the module/shared lib they are linked to. And thus can 
> appear multiple times in the same process, resulting in symbol clashes and 
> leading to problems at least with the Bsymbolic-functions flag or when being 
> possibly incompatible versions.
> 
> Attached patch sees to solve that problem, by adding a macro 
> KDE4_HIDE_SYMBOLS_FROM_STATIC_LIBS which should add any needed linker flags 
> depending on the platform/linker used. 
> 
> Only issue is that instead of some variable I had to use "QtUiTools.a" as I 
> found no variable which would resolve to that. E.g. ${QT_QTUITOOLS_LIBRARY} 
> resolves to "Qt4::QtUiTools" for me. Any idea what to use there, in case 
> another platform needs another name/prefix here?
> 
> Patch is against 4.10 branch, so I hope to get this in 4.10.4
> 
> http://lxr.kde.org/search?v=4.10-branch&filestring=&string=QT_QTUITOOLS_LIBRARY
>  shows that there are some more places where the symbols need hiding, but I 
> first want feedback on the proposed approach.
> 
> 
> Diffs
> -
> 
>   cmake/modules/FindKDE4Internal.cmake cb63285 
>   cmake/modules/KDE4Macros.cmake 3db4e24 
>   kjsembed/kjsembed/CMakeLists.txt d70f260 
>   kross/modules/CMakeLists.txt d245fd8 
>   kross/qts/CMakeLists.txt d8cb4a5 
>   plasma/CMakeLists.txt 674550d 
> 
> Diff: http://git.reviewboard.kde.org/r/110563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

KF5 Update Meeting 2013-w21

[Kevin Ottens]

Hello all,

Yesterday we had the second update meeting on #kde-devel. Like the last time 
I'll provide a summary of the meeting.

Were present for this first meeting: Aaron Seigo (aseigo), Benjamin Port 
(ben2367), David Faure (dfaure), Kurt Hindenburg (khindenburg), Marco Martin 
(notmart), Martin Sandsmark(sandsmark), Wojciech Kapuscinski (wojtask9_) and 
myself.

Topics discussed:
 * QCommandLineParser is still in development (slow progress because Qt 
Project is slow...), but it's available for KF5 already as it got imported in 
kdeqt5staging;
 * Adding debug areas to kDebug is next in line for dfaure, in the meantime we 
just port to qDebug;
 * qdbusmenu seems broken with Qt5, needs investigation;
 * More components of kde-workspace got ported on top of KF5 which extend the 
test coverage;
 * Two new setups (by khindenburg and ben2367), unlike the previous ones those 
two had issues mostly coming from the qt5.git setup, it's really the number 1 
pain point to get people on board;
 * Reminder: KDE UI Crumble page is currently the main source of tasks for 
people to pick up, if you got questions please contact us;
 * QFileDialog work is still on-going.

Actions:
 * [wojtask9_] Get a KDE account to be able to push his patches himself
 * [sandsmark] Finish the work on KPtyProcess
 * [ervin + dfaure] Send meeting reminders on mondays
 * [steveire] Write a "CMake for frameworks" guideline in the wiki
 * [ervin] Talk to Ben about having our own qt5.git with a kf5 branch
 * [notmart] Look at cleaning up the CMake files in plasma-framework (if time 
permits)
 * [notmart + aseigo] Investigate the dbusmenu breakage
 * [dfaure] Document the unixisms which should be cleaned up to get people to 
help
 * [ervin + dfaure] Review the build instructions

Announcement: I'll be completely unavailable for at least the following week 
(I don't know yet for sure how long it will be though). During my absence 
David will review the patches, answer questions and hold the meetings.

Next meeting will be Tuesday 28th 4pm Paris time.

Regards.
-- 
Kévin Ottens, http://ervin.ipsquad.net

Sponsored by BlueSystems and KDAB to work on KDE Frameworks


signature.asc
Description: This is a digitally signed message part.

Re: Review Request 106300: add kimgio WebP image format plugin

[Albert Astals Cid]

> On May 18, 2013, 12:44 p.m., Commit Hook wrote:
> > This review has been submitted with commit 
> > df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master.
> 
> Christoph Feck wrote:
> Just noticed this has been committed to kolourpaint instead of kdelibs. 
> Did I miss a discussion about this change?

If you addressed Allan's comment i'd sincerely put it where it belongs, which 
is obviously not kolourpaint :D


- Albert


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


On May 18, 2013, 12:44 p.m., Martin Koller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106300/
> ---
> 
> (Updated May 18, 2013, 12:44 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> ---
> 
> This patch adds support for the new Google invented WebP image format. See 
> https://developers.google.com/speed/webp/?hl=ru
> 
> 
> This addresses bug 267365.
> http://bugs.kde.org/show_bug.cgi?id=267365
> 
> 
> Diffs
> -
> 
>   kimgio/CMakeLists.txt 26329c0 
>   kimgio/webp.cpp PRE-CREATION 
>   kimgio/webp.desktop PRE-CREATION 
>   kimgio/webp.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106300/diff/
> 
> 
> Testing
> ---
> 
> some KDE apps, including read/write with a modified kolourpaint (to be able 
> to change the quality)
> 
> 
> Thanks,
> 
> Martin Koller
> 
>