Skip to content

[joltphysics] Add shared lib as build option - #53317

Draft
Bruno S Marques (bmarques1995) wants to merge 1 commit into
microsoft:masterfrom
bmarques1995:master
Draft

[joltphysics] Add shared lib as build option#53317
Bruno S Marques (bmarques1995) wants to merge 1 commit into
microsoft:masterfrom
bmarques1995:master

Conversation

@bmarques1995

Copy link
Copy Markdown

Continues #53241

@BillyONeal

Copy link
Copy Markdown
Member

In general, please try to avoid closing and reopening PRs with the same content, it makes following comments back to where they came from difficult.

In addition, GPT 5.6 Sol reports:

  • The Windows shared package only links when consumers receive JPH_SHARED_LIBRARY. The exported CMake target supplies it, but directly including include/ and linking lib/Jolt.lib produces unresolved allocator and Factory::sInstance symbols in both Release and Debug. The required direct-link integration may not rely on an extra build-system macro; the installed headers/package layout must make this usage work.
  • Normalize ports/joltphysics/portfile.cmake to LF. It is CRLF-only, and git diff --check origin/master...HEAD reports both added lines as trailing whitespace.

Would you consider merging bmarques1995#1 ?

@BillyONeal
Billy O'Neal (BillyONeal) marked this pull request as draft August 8, 2026 05:41
@bmarques1995

Copy link
Copy Markdown
Author

In general, please try to avoid closing and reopening PRs with the same content, it makes following comments back to where they came from difficult.

In addition, GPT 5.6 Sol reports:

  • The Windows shared package only links when consumers receive JPH_SHARED_LIBRARY. The exported CMake target supplies it, but directly including include/ and linking lib/Jolt.lib produces unresolved allocator and Factory::sInstance symbols in both Release and Debug. The required direct-link integration may not rely on an extra build-system macro; the installed headers/package layout must make this usage work.
  • Normalize ports/joltphysics/portfile.cmake to LF. It is CRLF-only, and git diff --check origin/master...HEAD reports both added lines as trailing whitespace.

Would you consider merging bmarques1995#1 ?

Okay, i tried to merge the changes, bad idea

@dg0yt Kai Pastor (dg0yt) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Upstream JPH_BUILD_SHARED_LIBS defaults to BUILD_SHARED_LIBS, so the extra option is not needed at all after changing vcpkg_check_linkage.
https://github.com/jrouwe/JoltPhysics/blob/e77f175595e64cb44218cc9d9d56fc365ad0e36a/Build/CMakeLists.txt#L30

)

string(COMPARE EQUAL "${VCPKG_CRT_LINKAGE}" "static" USE_STATIC_CRT)
string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "dynamic" JPH_BUILD_SHARED_LIBS)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "dynamic" JPH_BUILD_SHARED_LIBS)

-DTARGET_VIEWER=OFF
-DCROSS_PLATFORM_DETERMINISTIC=OFF
-DINTERPROCEDURAL_OPTIMIZATION=OFF
-DJPH_BUILD_SHARED_LIBS=${JPH_BUILD_SHARED_LIBS}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
-DJPH_BUILD_SHARED_LIBS=${JPH_BUILD_SHARED_LIBS}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants