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

[gamenetworkingsockets] fix static build #19310

Closed
wants to merge 2 commits into from

Conversation

aizuon
Copy link
Contributor

@aizuon aizuon commented Aug 2, 2021

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 261c458af6e3eed5d099144aff95d2b5035f656b -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/g-/gamenetworkingsockets.json b/versions/g-/gamenetworkingsockets.json
index 3aeda7b..16be0f2 100644
--- a/versions/g-/gamenetworkingsockets.json
+++ b/versions/g-/gamenetworkingsockets.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "b5852cde2df253ed27a294c79cce2f10be04dedc",
+      "version-semver": "1.3.0",
+      "port-version": 1
+    },
     {
       "git-tree": "02453dad9eb5272c445ba8e22c3d1aa3383a1a11",
       "version": "1.3.0",

@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Aug 3, 2021
Comment on lines +13 to +15
if(VCPKG_LIBRARY_LINKAGE STREQUAL "static")
set(STATIC_LIB_LINKAGE_OPTS -DBUILD_SHARED_LIBS=OFF -DProtobuf_USE_STATIC_LIBS=ON -DOPENSSL_USE_STATIC_LIBS=ON)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

When the user manually sets the build type of protobuf and openssl, this is invalid.

Comment on lines +93 to +113
-add_library(GameNetworkingSockets SHARED "")
-add_library(GameNetworkingSockets::GameNetworkingSockets ALIAS GameNetworkingSockets)
-add_library(GameNetworkingSockets::shared ALIAS GameNetworkingSockets)
-gamenetworkingsockets_common(GameNetworkingSockets)
-
add_library(GameNetworkingSockets_s STATIC "")
add_library(GameNetworkingSockets::GameNetworkingSockets_s ALIAS GameNetworkingSockets_s)
add_library(GameNetworkingSockets::static ALIAS GameNetworkingSockets_s)
target_compile_definitions(GameNetworkingSockets_s INTERFACE STEAMNETWORKINGSOCKETS_STATIC_LINK)
+set(GAMENETWORKINGSOCKETS_INSTALL_TARGETS GameNetworkingSockets_s)
gamenetworkingsockets_common(GameNetworkingSockets_s)

+if(BUILD_SHARED_LIBS)
+ add_library(GameNetworkingSockets SHARED "")
+ add_library(GameNetworkingSockets::GameNetworkingSockets ALIAS GameNetworkingSockets)
+ add_library(GameNetworkingSockets::shared ALIAS GameNetworkingSockets)
+ gamenetworkingsockets_common(GameNetworkingSockets)
+ list(APPEND GAMENETWORKINGSOCKETS_INSTALL_TARGETS GameNetworkingSockets)
+else()
+ add_library(GameNetworkingSockets::GameNetworkingSockets ALIAS GameNetworkingSockets_s)
+endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

We should change it to:

if (BUILD_SHARED_LIBS)
    add_library(GameNetworkingSockets SHARED "")
    ...
else()
     add_library(GameNetworkingSockets_s STATIC "")
    ...
endif()

Comment on lines +11 to +15
+if(DEFINED VCPKG_CRT_LINKAGE)
+ set(MSVC_RUNTIME ${VCPKG_CRT_LINKAGE})
+else()
+ set(MSVC_RUNTIME "dynamic")
+endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace these code to:

vcpkg_configure_cmake(
...
    OPTIONS
        -DMSVC_RUNTIME=${VCPKG_CRT_LINKAGE}
    ...
)

Comment on lines +17 to +19
+if(MSVC_RUNTIME STREQUAL "static")
+ set(OPENSSL_MSVC_STATIC_RT TRUE)
+endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't find OPENSSL_MSVC_STATIC_RT in the upstream, can you double confirm this?

)
endif()

+option(BUILD_SHARED_LIBS "Build shared library" ON)
Copy link
Contributor

Choose a reason for hiding this comment

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

This option doesn't need to be declared since it's provided by cmake.

option(LTO "Enable Link-Time Optimization" OFF)
option(USE_STEAMWEBRTC "Build Google's WebRTC library to get ICE support for P2P" OFF)
option(Protobuf_USE_STATIC_LIBS "Link with protobuf statically" OFF)
+option(OPENSSL_USE_STATIC_LIBS "Link with OpenSSL statically" OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
+option(OPENSSL_USE_STATIC_LIBS "Link with OpenSSL statically" OFF)

I didn't find any implementation of this.

@aizuon aizuon marked this pull request as draft August 5, 2021 05:07
@JackBoosY
Copy link
Contributor

This PR has been inactive for a long time, please reopen it if there is any progress.

@JackBoosY JackBoosY closed this Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants