Skip to content

[libjxl] Remove libm dependency when building with MSVC - #46177

Merged
Billy O'Neal (BillyONeal) merged 2 commits into
microsoft:masterfrom
ayeteadoe:jxl-msvc-remove-libm
Jun 27, 2025
Merged

[libjxl] Remove libm dependency when building with MSVC#46177
Billy O'Neal (BillyONeal) merged 2 commits into
microsoft:masterfrom
ayeteadoe:jxl-msvc-remove-libm

Conversation

@ayeteadoe

@ayeteadoe ayeteadoe commented Jun 26, 2025

Copy link
Copy Markdown
Contributor

This patches MSVC builds to not link with libm. Otherwise, when NOT BUILD_SHARED_LIBS is true, targets that depend on the libjxl target try to link with m.lib.

The following was the previous workaround I had to do before this fix:

diff --git a/Libraries/LibGfx/CMakeLists.txt b/Libraries/LibGfx/CMakeLists.txt
index 794271d014..2c55c49dfd 100644
--- a/Libraries/LibGfx/CMakeLists.txt
+++ b/Libraries/LibGfx/CMakeLists.txt
@@ -137,6 +137,11 @@ endif()
 
 if (NOT ANDROID)
     pkg_check_modules(Jxl REQUIRED IMPORTED_TARGET libjxl)
+    if (MSVC)
+        get_target_property(LIBJXL_INTERFACE_LINK_LIBS PkgConfig::Jxl INTERFACE_LINK_LIBRARIES)
+        list(REMOVE_ITEM LIBJXL_INTERFACE_LINK_LIBS "m")
+        set_target_properties(PkgConfig::Jxl PROPERTIES INTERFACE_LINK_LIBRARIES "${LIBJXL_INTERFACE_LINK_LIBS}")
+    endif()
     target_link_libraries(LibGfx PRIVATE PkgConfig::Jxl)
 else()
     find_package(libjxl REQUIRED)
  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@ayeteadoe

Copy link
Copy Markdown
Contributor Author

Is it possible to add a test for this?

@ayeteadoe

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@BillyONeal

Copy link
Copy Markdown
Member

The build failures appear legitimate. Perhaps the tools feature should be "supports": "native" or "supports": "!uwp"

Mengna-Li
Mengna-Li previously approved these changes Jun 27, 2025
Comment thread ports/libjxl/portfile.cmake Outdated
@Mengna-Li Mengna-Li added the category:port-bug The issue is with a library, which is something the port should already support label Jun 27, 2025

@BillyONeal Billy O'Neal (BillyONeal) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix!

-else()
- set(JPEGXL_REQUIRES_TYPE "Requires")
- set(JPEGXL_PUBLIC_LIBS "-lm ${PKGCONFIG_CXX_LIB}")
+# MSVCRT bundles math functions

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

IMO the change is incorrect: JPEGXL_REQUIRES_TYPE should be set regardless of MSVC.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO the change is incorrect: JPEGXL_REQUIRES_TYPE should be set regardless of MSVC.

Sigh; you're right and I missed this :/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dc6c

Copy link
Copy Markdown

Unfortunately, this also needs to patch up lib/jxl_cms.cmake and lib/threads/libjxl_threads.pc.in, or otherwise the linking against libm sneaks in via those routes.

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

Labels

category:port-bug The issue is with a library, which is something the port should already support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants