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

[sfml][imgui-sfml] Push SFML_STATIC_LIBRARIES setting into sfml #11800

Merged
merged 6 commits into from
Aug 22, 2020
Merged

[sfml][imgui-sfml] Push SFML_STATIC_LIBRARIES setting into sfml #11800

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1757f87
[sfml] Force SFML_STATIC_LIBRARIES to match how sfml was built
ras0219-msft Jun 5, 2020
e3f15be
[imgui-sfml] Rely on sfml to correctly set SFML_STATIC_LIBRARIES
ras0219-msft Jun 5, 2020
95f5ee0
[imgui-sfml] Add dependency on opengl to enable auto-linking on Windows
ras0219-msft Jun 5, 2020
d7a5071
Merge remote-tracking branch 'origin/master' into dev/roschuma/sfml-s…
BillyONeal Jun 11, 2020
206eca8
Merge remote-tracking branch 'origin/master' into dev/roschuma/sfml-s…
BillyONeal Jun 25, 2020
bf9aaff
Merge branch 'master' of https:/microsoft/vcpkg into dev/…
ras0219-msft Aug 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ports/imgui-sfml/CONTROL
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: imgui-sfml
Version: 2.1-1
Version: 2.1-2
Homepage: https:/eliasdaler/imgui-sfml
Description: ImGui binding for use with SFML
Build-Depends: sfml, imgui
Build-Depends: sfml, imgui, opengl
14 changes: 0 additions & 14 deletions ports/imgui-sfml/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,9 @@ vcpkg_from_github(
004-fix-find-sfml.patch
)

if (VCPKG_TARGET_IS_WINDOWS)
file(GLOB SFML_DYNAMIC_LIBS "${CURRENT_INSTALLED_DIR}/bin/sfml-*")
else()
file(GLOB SFML_DYNAMIC_LIBS "${CURRENT_INSTALLED_DIR}/bin/libsfml-*")
endif()

if (SFML_DYNAMIC_LIBS)
set(SFML_STATIC OFF)
else()
set(SFML_STATIC ON)
endif()

vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
PREFER_NINJA
OPTIONS
-DSFML_STATIC_LIBRARIES=${SFML_STATIC}
)
vcpkg_install_cmake()
vcpkg_copy_pdbs()
Expand Down
2 changes: 1 addition & 1 deletion ports/sfml/CONTROL
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: sfml
Version: 2.5.1-7
Version: 2.5.1-8
Homepage: https:/sfml/sfml
Description: Simple and fast multimedia library
Build-Depends: freetype, libflac, libogg, libvorbis, openal-soft, stb
4 changes: 3 additions & 1 deletion ports/sfml/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ vcpkg_install_cmake()
vcpkg_fixup_cmake_targets(CONFIG_PATH lib/cmake/SFML)
vcpkg_copy_pdbs()

FILE(READ ${CURRENT_PACKAGES_DIR}/share/sfml/SFMLConfig.cmake SFML_CONFIG)
if(VCPKG_LIBRARY_LINKAGE STREQUAL "static")
FILE(READ ${CURRENT_PACKAGES_DIR}/share/sfml/SFMLConfig.cmake SFML_CONFIG)
FILE(WRITE ${CURRENT_PACKAGES_DIR}/share/sfml/SFMLConfig.cmake "set(SFML_STATIC_LIBRARIES true)\ninclude(CMakeFindDependencyMacro)\nfind_dependency(Freetype)\n${SFML_CONFIG}")
else()
FILE(WRITE ${CURRENT_PACKAGES_DIR}/share/sfml/SFMLConfig.cmake "set(SFML_STATIC_LIBRARIES false)\n${SFML_CONFIG}")
Comment on lines +36 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get why you need this change. I also don't get why imgui-sfml required SFML_STATIC_LIBRARIES
since SFML seems to already set SFML_STATIC_LIBRARIES if required.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is potentially an issue in the reverse case -- if sfml is dynamic and imgui-sfml is static. This change ensures that SFML_STATIC_LIBRARIES is always set exactly according to how sfml was built, regardless of whether the outer project sets the variable or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Better make that a FATAL_ERROR instead of silently accepting it because it means a consumer needs to be patched instead and might be assuming something wrongly.

Copy link
Contributor Author

@ras0219 ras0219 Aug 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think that it's better to be optimistic here:

  1. Patching every single consumer is not realistic
  2. The vast majority of cases will work without patching
  3. The cases that don't work will have build failures, not silent misbehavior

Finally, it's essentially impossible for consumers to do the right thing; setting SFML_STATIC_LIBRARIES requires knowledge of whether SFML was built statically or not. Therefore, it's actually somewhat impossible to patch consumers correctly.

endif()

# move sfml-main to manual link dir
Expand Down