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




src/CMakeLists.txt (lines 160 - 163)
<https://git.reviewboard.kde.org/r/127822/#comment64673>

    This change is problematic, and apparently needs to be platform-specific 
(if done here):
    
    - on OS X it works as intended, despite the fact one should use `INCLUDES 
DESTINATION` instead of `INCLUDES` alone
    - on Linux, it adds an additional path to the search path 
($PREFIX/INCLUDES), which raises an error if inexistent. The intended behaviour 
is obtained by removing the `INCLUDES` keyword.
    
    This is probably because of this extract from `ECM/KDEInstallDirs.cmake`:
    
    ```
    set(KF5_INSTALL_TARGETS_DEFAULT_ARGS  RUNTIME DESTINATION 
"${CMAKE_INSTALL_BINDIR}"
                                          LIBRARY DESTINATION 
"${CMAKE_INSTALL_LIBDIR}"
                                          ARCHIVE DESTINATION 
"${CMAKE_INSTALL_LIBDIR}" COMPONENT Devel
                                          INCLUDES DESTINATION 
"${CMAKE_INSTALL_INCLUDEDIR_KF5}"
    )
    
    # on the Mac support an extra install directory for application bundles
    if(APPLE)
        set(KDE_INSTALL_TARGETS_DEFAULT_ARGS  
${KDE_INSTALL_TARGETS_DEFAULT_ARGS}
                                              BUNDLE DESTINATION 
"${BUNDLE_INSTALL_DIR}" )
        set(KF5_INSTALL_TARGETS_DEFAULT_ARGS  
${KF5_INSTALL_TARGETS_DEFAULT_ARGS}
                                              BUNDLE DESTINATION 
"${BUNDLE_INSTALL_DIR}" )
    endif()
    ```
    
    IOW, it *may* be that OS X ("APPLE") requires a dedicated change to 
`src/CMakelists.txt`, or else there'd be special cases for OS X, MSWin and the 
default (anything that always has a case-sensitive platform).
    
    I don't see an alternative, except possibly at the level of 
`CMAKE_INSTALL_INCLUDEDIR_KF5`, but I cannot find where that variable is 
defined.


- René J.V. Bertin


On May 3, 2016, 3:29 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127822/
> -----------------------------------------------------------
> 
> (Updated May 3, 2016, 3:29 p.m.)
> 
> 
> Review request for Build System, KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: attica
> 
> 
> Description
> -------
> 
> Attica has long adhered to an install layout that relies on case to 
> distinguish directories. For instance:
> 
> ```
> include/KF5/Attica/Attica/AccountBalance
> include/KF5/Attica/attica/accountbalance.h
> ```
> 
> Depending on the order in which those files are installed on a FS like HFS+ 
> (in case-insensitive-but-case-preserving mode), you will end up with 
> `Attica/Attica` or `Attica/attica` directories; the directory name case 
> changes to reflect the last write.
> 
> Basic calls like fopen() will succeed regardless of case because the 
> filesystem ignores case in such operations. However, compilers (clang at 
> least) do not simply try to open a requested include file in each element of 
> the header directory search path. They use a search algorithm to locate the 
> file first ... and that algorithm takes case into account. So `#include 
> <Attica/AccountBalance>` will fail if the file is installed as 
> `include/KF5/Attica/attica/AccountBalance`.
> 
> This issue is delicate to fix: the most trivial solution (installing all 
> headers in a single directory) would still require changes in all dependent 
> code.
> 
> I'm just proposing a fix that builds on the assumption that the 
> `<Attica/Foo>` style is part of C++ semantics (as opposed to a more 
> traditional `<attica/foo.h>`). The fix installs headers in 
> `KF5/Attica/attica` and `KF5/Attica/c++/Attica`, and adds the additional 
> `Attica/c++` component to the header search path. It also corrects the 
> pkg-config file.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 984f7ff 
>   src/cmake/libKF5Attica.pc.cmake 75387fa 
> 
> Diff: https://git.reviewboard.kde.org/r/127822/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.9.5 with FWs. 5.20.0 installed under /opt/local . As a test-case I 
> rebuilt `knewstuff` after changing its `src/uploaddialog_p.h` to include 
> `<Attica/ListJob>` instead of `<attica/listjob.h>` and `<Attica/Provider>` 
> instead of `<attica/provider.h>`.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to