Skip to content

CMake: Set plugins runtime output directory even when the rest are not set#123

Closed
hugoam wants to merge 3 commits into
mosra:masterfrom
hugoam:cmake-importer-runtime-output
Closed

CMake: Set plugins runtime output directory even when the rest are not set#123
hugoam wants to merge 3 commits into
mosra:masterfrom
hugoam:cmake-importer-runtime-output

Conversation

@hugoam

@hugoam hugoam commented Jun 13, 2022

Copy link
Copy Markdown

Hey :)
Sibling plugins PR with this one in main Magnum repo: mosra/magnum#570

Hello !
Our project only sets CMAKE_RUNTIME_OUTPUT_DIRECTORY but not the other two CMake variables, because those tend to mess with the way Visual Studio produces and consumes .lib so we don't want to open that can of worms.

This wasn't an issue before as we only consumed Magnum in a prebuilt way, but now I'm including magnum as part of our CMake tree for development convenience, and I'd still like the .dll to end up in the correct plugin subfolder without having to do some ugly stuff.

So this change proposes to set the RUNTIME_OUTPUT_DIRECTORY of the plugins when only CMAKE_RUNTIME_OUTPUT_DIRECTORY is set even if the other two variables are not set. In order to facilitate the change, I also factored the directory selection logic of the plugins to a single place since it was relatively complex code that was copied in more than 20 locations.

Let me know what you think and if you see issues with this approach, thanks :)

@mosra mosra added this to the 2022.0a milestone Jun 13, 2022
@hugoam hugoam force-pushed the cmake-importer-runtime-output branch from c503982 to 96f5d45 Compare June 14, 2022 10:31
@hugoam hugoam force-pushed the cmake-importer-runtime-output branch from 96f5d45 to d6910cd Compare June 14, 2022 12:27
@hugoam hugoam force-pushed the cmake-importer-runtime-output branch from d6910cd to 90ae08d Compare June 15, 2022 11:21
@mosra

mosra commented Jun 15, 2022

Copy link
Copy Markdown
Owner

Merged as 9f37ac5, thank you!

@mosra mosra closed this Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants