corrade icon indicating copy to clipboard operation
corrade copied to clipboard

Issues using corrade as a submodule and via CMake Package Manager

Open bruno-j-nicoletti opened this issue 1 year ago • 5 comments

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 avatar Oct 08 '24 16:10 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.

bruno-j-nicoletti avatar Oct 09 '24 10:10 bruno-j-nicoletti

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: https://github.com/mosra/corrade/blob/ed451d54ed4c7a20d0c4c5b6bc87ede089152b20/CMakeLists.txt#L540-L575

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 :sweat_smile:

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. :+1:

mosra avatar Oct 09 '24 10:10 mosra

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

mosra avatar Oct 09 '24 10:10 mosra

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.

bruno-j-nicoletti avatar Oct 09 '24 16:10 bruno-j-nicoletti

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 avatar Oct 09 '24 16:10 mosra

In e7dd8d579b92a24bfd72ed84b415c2b2cce81c14 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 :sweat_smile:

Additionally, as of 24502d1321aaf27f0b4006bc89c51e2500d2330f, 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.

mosra avatar Nov 04 '24 11:11 mosra