Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify CMAKE_RUNTIME_OUTPUT_DIRECTORY from outside of the project #2745

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ChrisThrasher
Copy link
Member

@ChrisThrasher ChrisThrasher commented Oct 23, 2023

Description

Hardcoding this into the library itself means that outside consumers cannot choose a different value. This in particular breaks the CMake template project of ours which also tries to set this value. It resulted in the DLLs not being placed in build/bin as expected but rather build/_deps/sfml-build/bin.

As conventional wisdom dictates, it's best to not touch CMAKE_ variables unless necessary. Luckily our use of presents means we can easy move this into the development preset so there are no ergonomics disadvantages to this.

This problem is not present in SFML 2 because that version does not manipulate CMAKE_RUNTIME_OUTPUT_DIRECTORY. Luckily now that the template project defaults to static libs these DLL issues are harder to run into.

Thanks to @Bambo-Borris for reporting this.

Hardcoding this into the library itself means that outside consumers
cannot choose a different value. This in particular breaks the CMake
template project of ours which also tries to set this value.

As conventional wisdom dictates, its best to not touch CMAKE_ variables
unless necessary. Luckily our use of presents means we can easy move
this into the development preset.
@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #2745 (d11615d) into master (e45628e) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2745   +/-   ##
=======================================
  Coverage   38.32%   38.32%           
=======================================
  Files         237      237           
  Lines       20801    20801           
  Branches     5148     5148           
=======================================
  Hits         7973     7973           
- Misses      11795    11815   +20     
+ Partials     1033     1013   -20     

see 42 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e45628e...d11615d. Read the comment docs.

@Bambo-Borris
Copy link
Contributor

Found this whilst using the template project, altered to target master. I regularly use the current state of master. I noticed that the DLLs weren't being output to the expected directory build_directory/bin when using master but were when using 2.6.0. Thanks to @ChrisThrasher for the quick spot and potential resolution for this

@eXpl0it3r
Copy link
Member

What do you want to do about buildbot?

@ChrisThrasher
Copy link
Member Author

ChrisThrasher commented Oct 31, 2023

The Mesa3D stuff hardcodes a reference to ${PROJECT_BINARY_DIR}/bin. To make this PR work I think I need to figure out a more generic solution that doesn't depend on using that particular directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Planned
Development

Successfully merging this pull request may close these issues.

None yet

3 participants