> On May 11, 2016, 4:05 p.m., René J.V. Bertin wrote: > > src/CMakeLists.txt, lines 160-163 > > <https://git.reviewboard.kde.org/r/127822/diff/1/?file=464053#file464053line160> > > > > 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.
I realise that the above comment isn't very clear. First: the INSTALL() documentation instructs to use `INCLUDES DESTINATION` while what works (on Mac OS) is to use `INCLUDES` alone, as written in the patch. Why is unclear to me. The difference with Linux becomes clear when we look at `KF5_INSTALL_TARGETS_DEFAULT_ARGS`: - Linux : `RUNTIME;DESTINATION;bin;LIBRARY;DESTINATION;lib/x86_64-linux-gnu;ARCHIVE;DESTINATION;lib/x86_64-linux-gnu;COMPONENT;Devel;INCLUDES;DESTINATION;include/KF5` - OS X : `RUNTIME;DESTINATION;bin;LIBRARY;DESTINATION;lib;ARCHIVE;DESTINATION;lib;COMPONENT;Devel;INCLUDES;DESTINATION;include/KF5;BUNDLE;DESTINATION;/Applications/MacPorts/KF5` IOW, on Linux the additional path can be appended directly to the preexisting `INCLUDES DESTINATION`, whereas on OS X there are additional elements in the list which make that impossible. I tried adding the `BUNDLE DESTINATION` to `KF5_INSTALL_TARGETS_DEFAULT_ARGS` unconditionally and that makes the attached patch work as expected on Linux too. Of course it would be more elegant to have a macro to modify `KF5_INSTALL_TARGETS_DEFAULT_ARGS` and add additional paths to `INCLUDES DESTINATION`. Or some other method to add elements to the `INTERFACE_INCLUDE_DIRECTORIES` variable defined by the frameworks' `KF5*Targets.cmake` module. - René J.V. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127822/#review95384 ----------------------------------------------------------- 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 > >
