Review Request 110962: Switch to an external LibRaw

[Pino Toscano]

> On June 11, 2013, 6:11 p.m., Rolf Eike Beer wrote:
> > cmake/modules/FindLibRaw.cmake, line 36
> > <http://git.reviewboard.kde.org/r/110962/diff/1/?file=149621#file149621line36>
> >
> >     file(STRING ... REGEX ...) will greatly reduce the noise in the temporary variable. Also you can directly put the match into the target variable using string(REGEX REPLACE). And since you don't unset the version variables otherwise people may eventually start using them. In this case they should use the default names, which would be LibRaw_VERSION_{MAJOR,MINOR,PATCH} then.

I didn't want to make use of the LibRaw_VERSION_{MAJOR,MINOR,PATCH} variables, so I won't adopt them.
Regarding the temporary variables, if people use temporary undocumented variables (even prefixed with underscore), that's their problem.


> On June 11, 2013, 6:11 p.m., Rolf Eike Beer wrote:
> > cmake/modules/FindLibRaw.cmake, lines 3-7
> > <http://git.reviewboard.kde.org/r/110962/diff/1/?file=149621#file149621line3>
> >
> >     It would be good if a new Find*.cmake module would follow the convention that CMake defines for new modules to make it easier for everyone to use them.
> >     
> >     See here: http://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/readme.txt

Will change, even if it seems mostly a pedantic change than an actually useful one...


- Pino


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


On June 11, 2013, 5:50 p.m., Pino Toscano wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110962/
> -----------------------------------------------------------
> 
> (Updated June 11, 2013, 5:50 p.m.)
> 
> 
> Review request for KDE Graphics, Release Team and Gilles Caulier.
> 
> 
> Description
> -------
> 
> Instead of using an embedded copy of LibRaw, look for an external LibRaw as mandatory dependency with a new CMake module and using its variables.
>     
> Considering some LibRaw versions seem to be underlinked and not linking to OpenMP, link it manually in libkdcraw to overcome such lack.
>     
> Switch back to the MAKE_KDCRAW_LIB define (i.e. the default set by KDE4_ADD_LIBRARY) as the one used to check whether it is being built, as otherwise LIBRAW_BUILDLIB would conflict with LibRaw.
> 
> Once this RR is approved, I will remove the libraw code copy and the CMake modules (FindLCMS2.cmake and FindPthreads.cmake) needed for it.
> 
> 
> This addresses bug 307146.
>     http://bugs.kde.org/show_bug.cgi?id=307146
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt f2f269609feb10947ec3bac10125b379c6c821dd 
>   cmake/modules/FindLibRaw.cmake PRE-CREATION 
>   libkdcraw/CMakeLists.txt cce5d6dba690fb5182638ccd1f10488bbd6ec2ce 
>   libkdcraw/libkdcraw_export.h 1a222a03502a0e068bdba4f03b7ff4961c4a8f2b 
> 
> Diff: http://git.reviewboard.kde.org/r/110962/diff/
> 
> 
> Testing
> -------
> 
> Compiles fine with both LibRaw 0.14.7 and 0.15.1.
> 
> 
> Thanks,
> 
> Pino Toscano
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/release-team/attachments/20130611/99fc448f/attachment-0001.html>