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.

Sign up for GitHub

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

Jump to bottom

Do not install extlibs if SFML_USE_SYSTEM_DEPS is true. #1237

Merged
merged 1 commit into from Jul 10, 2017
Merged

Do not install extlibs if SFML_USE_SYSTEM_DEPS is true. #1237

merged 1 commit into from Jul 10, 2017

Conversation

hanetzer
Copy link
Contributor

@hanetzer hanetzer commented Jun 2, 2017

If those libs are present at build time, they would be clobbered at install time.
Will close #1236 (at least for me)
Signed-off-by: Marty Plummer ntzrmtthihu777@gmail.com

MarioLiebisch
MarioLiebisch requested changes Jun 2, 2017
View reviewed changes
Copy link
Member

@MarioLiebisch MarioLiebisch left a comment

Choose a reason for hiding this comment

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

While this change fixes the problem for Windows, it doesn't address the same problem for MacOS X and Android. Could you expand the fix, considering it's trivial to add?

@hanetzer
Copy link
Contributor Author

hanetzer commented Jun 2, 2017

@MarioLiebisch unless I'm misunderstanding the OSX code shortly after my change, it will only install those frameworks if they were used as part of the build (meaning, if system libraries for that are either not present or were manually prevented from use with that cmake arg).

Android would be a simple fix.

@MarioLiebisch
Copy link
Member

Yes, you're right. Doesn't apply to Mac OS X (although I'm not a real fan of those string comparisons), but the two mobile targets.

@hanetzer
Copy link
Contributor Author

hanetzer commented Jun 2, 2017

@MarioLiebisch does this work for you?

MarioLiebisch
MarioLiebisch reviewed Jun 2, 2017
View reviewed changes
CMakeLists.txt Outdated
# install Android.mk so the NDK knows how to set up SFML
install(FILES src/SFML/Android.mk DESTINATION .)
# install Android.mk so the NDK knows how to set up SFML
install(FILES src/SFML/Android.mk DESTINATION .)
Copy link
Member

Choose a reason for hiding this comment

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

This install command has to stay even for SFML_USE_SYSTEM_DEPS. This Android.mk works similar to pkg-config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Achk, misread that. one second.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed up now.

@hanetzer
Copy link
Contributor Author

hanetzer commented Jun 2, 2017 •
edited

ah, it looks like I need to tweak few more things to prevent building against bundled libs... give me a bit.
nvm. I was wrong; issue arose from me patching sfml-2.4.2 with my changes and not using the git repo.
also caught a missing ()

Do not install extlibs if SFML_USE_SYSTEM_DEPS is true.
bd479c4 
Signed-off-by: Marty Plummer <ntzrmtthihu777@gmail.com>
@eXpl0it3r eXpl0it3r added this to the 2.5 milestone Jun 3, 2017
@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.5.0 Jun 3, 2017
@eXpl0it3r eXpl0it3r moved this from Discussion to Ready in SFML 2.5.0 Jun 3, 2017
@eXpl0it3r eXpl0it3r merged commit bd479c4 into SFML:master Jul 10, 2017
@eXpl0it3r eXpl0it3r moved this from Ready to Merged in SFML 2.5.0 Jul 10, 2017
@hanetzer hanetzer deleted the no-install-sys-libs branch May 19, 2018 01:06
@SFML SFML deleted a comment from eXpl0it3r Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarioLiebisch MarioLiebisch MarioLiebisch approved these changes

Assignees

@MarioLiebisch MarioLiebisch

Labels
bug m:config p:macos p:windows
Projects
No open projects
SFML 2.5.0
  
Merged / Superseded
Milestone
2.5
Development

Successfully merging this pull request may close these issues.

SFML installs extlibs even if SFML_USE_SYSTEM_DEPS is set
3 participants
@hanetzer @MarioLiebisch @eXpl0it3r