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

changed min cmake ver and CMAKE_SOURCE_DIR to PROJECT_SOURCE_DIR #170

Merged
merged 1 commit into from
Mar 17, 2022

Conversation

brightprogrammer
Copy link
Contributor

There was a build error when building CSFML stating that it failed to find files in cmake folder. Also there were some warnings for cmake version so changed cmake version too. This fixed the build for me.

Copy link
Member

@eXpl0it3r eXpl0it3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change!

Just two small comments 😉

@eXpl0it3r
Copy link
Member

Can you change the following line

if(CMAKE_CXX_COMPILER MATCHES ".*clang[+][+]" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang")

to something like this?

if(CMAKE_CXX_COMPILER MATCHES "clang[+][+]" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")

Not sure if it's the newer CMake version, but seems like it fails to detect AppleClang on macOS now.


Additionally, could you squash the commits into one?

@brightprogrammer
Copy link
Contributor Author

I'm sorry, I'm not that experienced with git, I think the squashing didn't go well but I made the requested changes

@brightprogrammer
Copy link
Contributor Author

I'm sorry, I'm not that experienced with git, I think the squashing didn't go well but I made the requested changes

ok squashing worked, can you please check now? 😄

@brightprogrammer
Copy link
Contributor Author

is it possibble that the problem is in the machine on which builds are running on?

@eXpl0it3r
Copy link
Member

I managed to get it working, by using the code from SFML's 2.6.x branch 😄

@eXpl0it3r eXpl0it3r merged commit 980a165 into SFML:master Mar 17, 2022
@eXpl0it3r
Copy link
Member

Thanks a lot for this fix! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants