Exporting the executables will make the cmake config file check for their
presence when used, which isn't really necessary, and is actually a problem
for cross-compilation, where the helper executables don't exist for the
target but for the host.
Details
- Reviewers
cordlandwehr apol - Group Reviewers
Frameworks - Commits
- R246:2141e4215747: Don't export internal helper executables
Diff Detail
- Repository
- R246 Sonnet
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Why are those executables even installed? For crosscompiling maybe? I'm confused about how that would work. One would build sonnet for host, install it, and then build sonnet for target, importing targets from the host build?
Isn't that "import" what this commit just broke?
Maybe the INSTALL lines should be inside the else, there's no need to install the compiled-for-target executables....
(AFAICS they are internal, not used outside sonnet at all)
As per the if below the proposed change it is used for cross compiling.
Subscribing original author and committer.
(kind of off topic: This code should really be changed to use the KF5_HOST_TOOLING we use elsewhere, where the tools get put into its own Targets.cmake file separate from the libs and then loaded from KF5_HOST_TOOLING when xcompiling https://phabricator.kde.org/source/kconfig/browse/master/KF5ConfigConfig.cmake.in;023e3ecfe985e09f786134fc28793d24383998f8$11 when not xcompiling the targets are fairly useless and don't need to get imported at all)
What content does the file included by
include("${KCONFIGCOMPILER_PATH}")
should have to be usable ?
The exported executable. Basically: CMake has two Targets.cmake files. One for the library artifacts and one completely separate one for the helpers. This allows including either/or depending on what is needed and by extension including the host's helpers when cross compiling
Further references:
- here we install the binary and EXPORT it into KF5ConfigCompilerTargets https://phabricator.kde.org/source/kconfig/browse/master/src/kconfig_compiler/CMakeLists.txt;023e3ecfe985e09f786134fc28793d24383998f8$23
- here we install the targets https://phabricator.kde.org/source/kconfig/browse/master/CMakeLists.txt;023e3ecfe985e09f786134fc28793d24383998f8$81
- note that KF5ConfigCompilerTargets (helper binaries) is different from KF5ConfigTargets (library artifacts)
- in our Config.cmake we always include our library targets https://phabricator.kde.org/source/kconfig/browse/master/KF5ConfigConfig.cmake.in;023e3ecfe985e09f786134fc28793d24383998f8$5
- but depending on xcompiling we'll either include the helper binary from the host's KF5ConfigCompilerTargets or ours when not xcompiling https://phabricator.kde.org/source/kconfig/browse/master/KF5ConfigConfig.cmake.in;023e3ecfe985e09f786134fc28793d24383998f8$11
This ultimately results in find_package(KF5Config) always importing KF5::kconfig_compiler, but depending on the xcompiling stuff it may be the host's.
This would work pretty much exactly the same for sonnet with the additional change that the xcompile if inside the source itself (https://phabricator.kde.org/source/sonnet/browse/master/data/CMakeLists.txt;c4c007cc3c0b9ee0ee46e91d026752662dd867a4$18) would change to find_file; include boilerplate of https://phabricator.kde.org/source/kconfig/browse/master/KF5ConfigConfig.cmake.in;023e3ecfe985e09f786134fc28793d24383998f8$12
I think @apol wrote the initial implementation of this magic and can probably answer questions better than me though :)
I think @apol wrote the initial implementation of this magic and can probably answer questions better than me though :)
I guess no. He also did not provide a working example see https://git.reviewboard.kde.org/r/128971/
This patch is correct, EXPORT is only useful if the target is used within cmake, AFAIU.
Furthermore, we should modify sonnet to install these applications into LIBEXEC rather than bin, otherwise we can't have both architectures installed along with the library that is going to use it (at least on some linux distros, e.g. debian based), but this we can do in a separate patch.
Or as @dfaure says, don't install at all, because I don't see who uses this. Or remove altogether?π₯
That works for me on Yocto, but in general this seems wrong, as target-sonnet will need to use host-sonnet's executable(s).
The helpers are used by sonnet at build time, so to cross compile sonnet you need the helpers of the host (of e.g. x64) while you xbuild (for aarch64) as you cannot build the helpers for aarch64 and then run them on x64. Much the same as kconfig compliation, except the only user is the framework itself.
Mind you, one could solve this "properly" without having to install the helpers. I am not sure the time investment is worth the effort though given we know the kconfig approach is sane and working in production.
One could convert the helpers into a sepearate cmake "subproject" which gets built by the sonnet build (i.e. sonnet source contains sonnet-helpers source and builds them during build but disregarding the cross compile configs and never installing the helpers). Which could probably be done via ExternalProject. It does however raise the question of how do you pass cmake configuration to that subproject (different prefix so it can find libraries etc). So that may open another can of worms.