Issues using corrade as a submodule and via CMake Package Manager

[bruno-j-nicoletti]

Hi,

I’m evaluating Magnum to see if I can use it for my project. I’ve hit two separate problems while trying to integrate it into my CMake set up. I’ve replicated the issues in a cut down git repo just using corrade. Please have a look.

Problem 1 is if I include corrade as a sub_directory, it sets the directory to place programs into as build/bin/, not where I originally wanted them. Seeing as I have 20 or so unit test programs all called 'unitTest', this causes problems.

Problem 2 is if I include corrade in my project via the CPM package manager, it fails because it can’t parse the checkout tag as a year/month.

I can make a work around for 2 if you are happy for me to, but I’m not sure what is happening with Problem 1.

Any help appreciated.

Bruno

[bruno-j-nicoletti]

Having slept on it, I see you are overriding CMAKE_RUNTIME_OUTPUT_DIRECTORY along with CMAKE_LIBRARY_OUTPUT_DIRECTORY and CMAKE_ARCHIVE_OUTPUT_DIRECTORY if they are not already set. This breaks my build if I include corrade or magnum. I'm going to work on a patch for you cmakes to get around this.

[mosra]

Hello! I think I know what's up.

Problem 1

This is set by default if you don't override CMAKE_RUNTIME_OUTPUT_DIRECTORY or any of the related variables, and the reason is to make life more bearable for users on Windows (as, due to the setting being set for the outer project as well, their .exes are then next to all their .dlls and they "just work") as well as make usage with dynamic plugins (for image / model loading and conversion, font support, ...) easier. The plugins are loaded dynamically at runtime from a location relative to the particular Corrade/Magnum library, so this ensures the library is placed next to the plugins.

This is the code that does it, plus some more background details:

corrade/CMakeLists.txt

Lines 540 to 575 in ed451d5

	# A single output location. After a decade of saying NO THIS IS A NON-SOLUTION
	# TO A NON-PROBLEM I reconsidered my views and enabled this, because:
	#
	# - On Windows (which don't have RPATH), this makes test execution finally
	# possible without having to install all the stuff first (including the
	# test-only libs, which is ugh).
	# - With CMake subprojects, this makes it finally possible to use dynamic
	# plugins directly from the build dir (again without installing anything) ---
	# all plugins are put into the same place, so PluginManager has a single
	# place to look into; and thanks to the dynamic libraries being there as
	# well, this location can be automagically detected as relative to
	# Utility::Path::libraryLocation().
	# - Thanks to the $<CONFIG> being part of the output path, you are always sure
	# you never accidentally mix up debug/release libraries when switching
	# CMAKE_BUILD_TYPE in an existing build dir.
	#
	# The runtime location is set to CMAKE_BINARY_DIR and not PROJECT_BINARY_DIR
	# because have one runtime location per CMake subproject would not solve much
	# either. If the user already provides CMAKE_RUNTIME_OUTPUT_DIRECTORY (even
	# empty), it's respected and nothing is being done.
	#
	# Explicitly using a generator expression to ensure plugins are added to e.g.
	# <CONFIG>/lib/corrade/fs/ instead of lib/corrade/fs/<CONFIG>. Also adding this
	# to cache, making superprojects pick that up implicitly as well, without
	# forcing them to explicitly mirror this setting.
	if(NOT DEFINED CMAKE_RUNTIME_OUTPUT_DIRECTORY AND NOT DEFINED CMAKE_LIBRARY_OUTPUT_DIRECTORY AND NOT DEFINED CMAKE_ARCHIVE_OUTPUT_DIRECTORY)
	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/$<CONFIG>/bin CACHE PATH "" FORCE)
	set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/$<CONFIG>/lib CACHE PATH "" FORCE)
	set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/$<CONFIG>/lib CACHE PATH "" FORCE)
	# There should be no need for the "90% use case" user to adjust these, so
	# don't show them in the default view
	mark_as_advanced(
	CMAKE_RUNTIME_OUTPUT_DIRECTORY
	CMAKE_LIBRARY_OUTPUT_DIRECTORY
	CMAKE_ARCHIVE_OUTPUT_DIRECTORY)
	endif()

And to override, it should be enough to set any of those variables. An empty value is the default, resetting the behavior to what you expect. OTOH for plugins to be found I still recommend that you keep the library location set the same way, for Corrade & Magnum at least, and then you can unset it after. So for example this:

set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "")
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/$<CONFIG>/lib)
add_subdirectory(submodules/corrade EXCLUDE_FROM_ALL)
# And Magnum, Magnum Plugins etc.
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "")

I have 20 or so unit test programs all called 'unitTest'

Funnily enough, my problem that led to setting this up, among other things, was an inverse of yours -- I have several hundreds of unit tests, placed in Test/ subdirectories next to the code they test, and running them manually was a giant pain when they were scattered deep inside the directory structure 😅

Problem 2

The fatal: No tags can describe 'ed451d54ed4c7a20d0c4c5b6bc87ede089152b20'. is because by default CPM fetches a shallow copy, and the v2020.06 tag is quite far back, so the cloned repository doesn't have that tag. A fix for that would be telling CPM to clone the full repo, i.e. the following. I'm doing my best to keep the Git history clean without huge files being commited by accident, so this should be fine from an efficiency PoV. I'm also aiming to finally make another release so you can pin to a concrete tag, that work is tracked in mosra/magnum#453.

CPMAddPackage(NAME corrade
 GITHUB_REPOSITORY mosra/corrade
 GIT_TAG master
 EXCLUDE_FROM_ALL YES
 GIT_SHALLOW NO) # add this option

In any case, it's a CMake warning, not a build-preventing error, it's just that the generated Corrade/version.h won't contain the exact Git hash the library was built out of.

Hope this helps!

By the way, thanks a lot for taking the time to set up the repo, that made everything a lot easier for me to verify. 👍

[mosra]

Heh, managed to send a reply at the exact time as you.

[bruno-j-nicoletti]

Thanks for the prompt responses, very much appreciated. I came up with the same fix for #1 as you suggested. As for #2, it will still built corrade even with the CMake failure, which is odd. I'll try your suggestion tomorrow.

[mosra]

It isn't odd -- as I'm saying, it's just a warning, not an error. Because for example you can have the repository downloaded as a ZIP file, at which point it'll have no Git history to query the version from. It'd be silly if the build required presence of full Git history just to generate a version header.

If it fails to fetch version info, the generated version.h won't have as much info. Which might or might not be a problem depending on the target use case. For Corrade itself it's not a problem if the version info is missing as nothing in the code is using it. On the other hand, if the final application really needs a concrete revision info, it can include Corrade/version.h and make a hard failure if it isn't complete enough.

[mosra]

In e7dd8d5 I tried to clarify the Git warning message to not make it look like it's a fatal error; similar change is being pushed to other repos as well. Unfortunately, Git itself saying fatal doesn't really help there 😅

Additionally, as of 24502d1, there's new documentation for using CPM, emphasising that one should disable GIT_SHALLOW to avoid that warning. Magnum docs will get the same addition as well.

That's I think all left to be done here, so closing as resolved.