ned14 / outcome

Provides very lightweight outcome<T> and result<T> (non-Boost edition)
https://ned14.github.io/outcome
Other
676 stars 62 forks source link

When included by `add_subproject` automatic download and build of `quickcpplib` fails to pass CMAKE variables properly #215

Closed burnpanck closed 4 years ago

burnpanck commented 4 years ago

I am using outcome in a bare-metal ARM environment.

I have recently updated outcome to the latest version (tried both v2.1.2 and develop) in order to build with GCC trunk. Since that update, I'm unable to CMAKE configure my project even on clang 9 (which worked before). It fails after in -- Superbuilding missing dependency quickcpplib with config Debug, this make take a while ..., which refuses to even configure because it thinks my compilers are broken: This happens because it attempts to link a simple test program using the hosts sysroot rather than my ARM sysroot, which the compiler expects. Indeed, to make my toplevel CMAKE work, I pass a CMAKE_TOOLCHAIN_FILE that sets a number of paths and tells CMAKE to build a static library to test the compiler, not a program. This information is not passed on to quickcpplib.

I do not know what exactly has changed from before (probably around outcome v2.1.0), and that my setup is an uncommon one. The easiest solution for me would be to tell outcome where to find quickcpplib directly. Is that possible with something likeadd_subproject and a couple of CMAKE variables for outcome? Or do I finally have to turn to properly installing dependencies in a project-local installation prefix/environment? (system-wide installs are out of question!)

ned14 commented 4 years ago

We used to use a git submodule based dependency system, but it was incompatible with third party library packaging systems, and we got lots of complaints. So we moved to a traditional cmake find_package() system with superbuild.

It is this file which configures subproject superbuild: https://github.com/ned14/quickcpplib/blob/master/cmakelib/DownloadBuildInstall.cmake.in

Can you suggest what I ought to set CMAKE_TOOLCHAIN_FILE to in order to make things work for you?

burnpanck commented 4 years ago

Ah, I have never used CMake ExternalProject myself - not sure I can help much there. I'll give it a try. Could you give me a pointer to what "superbuild" is?

ned14 commented 4 years ago

Superbuild is when a cmake build launches another cmake build from within itself to build missing dependencies.

You may find reading the entire thread at https://cmake.org/pipermail/cmake-developers/2015-April/025090.html of use. It might be as simple as simply passing through CMAKE_TOOLCHAIN_FILE, but I gather that it may not be as simple as that either.

Note that if find_package() succeeds, we never superbuild. So if you need separately do custom builds of dependencies, you can ensure cmake finds them as a package beforehand.

burnpanck commented 4 years ago

Ok, thanks for the pointer! So far, I still manage all my dependencies using add_subproject and submodules. I'm aware that it has it's issues regarding packaging, but for applications (as opposed to libraries), it's just very convenient. At some point, I'll probably want to give conan as try though.

ned14 commented 4 years ago

You may find https://github.com/cpp-pm/hunter easier going. It supports custom toolchains.

ned14 commented 4 years ago

We now attempt to pass through CMAKE_TOOLCHAIN_FILE

burnpanck commented 4 years ago

Hey @ned14, sorry, have been working on non-software things in the past few weeks. I have just continued work on these things.

Unfortunately, the fix here doesn't fix the issues for me. While CMAKE_TOOLCHAIN_FILE is passed to the ExternalProject_Add within the generated DownloadBuildInstall super-build CMakeLists.txt, it never gets there - that cmake run again trips on the line project(DownloadBuildInstall), where super eager CMake looks for a C compiler and aborts. It seems that the simple solution is to replace that line with project(DownloadBuildInstall NONE) (within DownloadBuildInstall.cmake.in), at so it did in a quick check here. I'll submit a pull request later today.

Going forward, I will probably also turn to a super-build setup (Hunter is unmaintained as far as I know). Man, do I hate CMake - and yet we all have to use it.

burnpanck commented 4 years ago

Done. While https://github.com/ned14/quickcpplib/pull/14 does fix the original issue for me, the current master still does not compile in add_subdirectory mode for me. Now it fails with

-- Configuring done
CMake Error in deps/outcome/CMakeLists.txt:
  Target "outcome_hl" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/path/to/outer/source/directory/deps/outcome/include"

  which is prefixed in the source directory.

This seems to happen when target_include_directores is not using $<BUILD_INTERFACE:...> if I remember correctly - but your code IS using those. Any ideas? Also, should I open a new issue for this?

ned14 commented 4 years ago

FYI https://github.com/cpp-pm/hunter is the fork of cmake hunter which is definitely maintained. If anything, because there's now more than one person maintaining it, things are progressing there much quicker than before.

Regarding your second problem, the main project at my work uses add_subdirectory() for LLFIO, which also uses quickcpplib (and indeed Outcome internally). I don't see the problem you see, though I have seen the problem you see before when you try to tell cmake that an installed target must include headers from your source tree, instead of using installed headers.

My work project publishes install targets like this:

install(TARGETS
    mdxcore_hl mdxcore_sl mdxcore_dl
    mdxcde_hl mdxcde_sl mdxcde_dl
    mdxmock_hl mdxmock_sl mdxmock_dl
    twxmlib_hl
  EXPORT mdxExports
  INCLUDES DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
  ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}"
  LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}"
)
install(DIRECTORY
  "${CMAKE_CURRENT_SOURCE_DIR}/include/mdx/"
  DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/mdx"
)
configure_file(
  "${CMAKE_CURRENT_SOURCE_DIR}/cmake/ProjectConfig.cmake.in"
  "${CMAKE_CURRENT_BINARY_DIR}/mdxConfig.cmake"
  @ONLY
)
install(FILES
  "${CMAKE_CURRENT_BINARY_DIR}/mdxConfig.cmake"
  DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/mdx"
)
install(EXPORT mdxExports
  NAMESPACE mdx::
  DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/mdx"
)

Note that mdxcore_hl is link dependent on llfio_hl, mdxcore_sl is link dependent on llfio_sl, and mdxcore_dl is link dependent on llfio_dl. I can confirm no errors from cmake with this arrangement, and LLFIO is using quickcpplib exactly the same as Outcome does. As you mentioned, the $<BUILD_INTERFACE:...> stuff "does the right thing" here.

Perhaps there might be a clue in the above as to what's wrong with your cmake install?

burnpanck commented 4 years ago

Ok, now's my turn for the face palm slap: I had an additional target_include_directories in my outer project, modifying outcome_hl, adding the include directory in the source tree. With that fixed, outcome does compile in my add_subdirectory setup. Thank you for your support!

ned14 commented 4 years ago

Very easy to do what you did. Done it many times myself. Thanks for letting me know, and I'm glad we got all this working!