Review Request 127822: address the potential name/case clash of the Attica/Attica and Attica/attica header dirs
René J.V. Bertin
rjvbertin at gmail.com
Fri Sep 30 16:36:31 UTC 2016
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20160930/65e2c9b7/attachment-0001.html>
More information about the Kde-buildsystem
mailing list