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

[libc++] Fix how we pass /MANIFEST flag on Windows without clang-cl #96967

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Jun 27, 2024

If we're compiling with Clang (not clang-cl) on Windows, we need to use -Xlinker to pass the /MANIFEST option.

Fixes #96430

If we're compiling with Clang (not clang-cl) on Windows, we need
to use -Xlinker to pass the /MANIFEST option.

Fixes llvm#96430
@ldionne ldionne requested a review from a team as a code owner June 27, 2024 21:00
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jun 27, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jun 27, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

If we're compiling with Clang (not clang-cl) on Windows, we need to use -Xlinker to pass the /MANIFEST option.

Fixes #96430


Full diff: https://github.com/llvm/llvm-project/pull/96967.diff

1 Files Affected:

  • (modified) libcxx/src/CMakeLists.txt (+7-2)
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 9e6c70335a794..8ba6f00bf9dad 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -275,8 +275,13 @@ if (LIBCXX_ENABLE_SHARED)
     # Since we most likely do not have a mt.exe replacement, disable the
     # manifest bundling.  This allows a normal cmake invocation to pass which
     # will attempt to use the manifest tool to generate the bundled manifest
-    set_target_properties(cxx_shared PROPERTIES
-                          APPEND_STRING PROPERTY LINK_FLAGS " /MANIFEST:NO")
+    if (${CMAKE_CXX_COMPILER_FRONTEND_VARIANT} STREQUAL "MSVC")
+      set_target_properties(cxx_shared PROPERTIES
+                            APPEND_STRING PROPERTY LINK_FLAGS " /MANIFEST:NO")
+    else()
+      set_target_properties(cxx_shared PROPERTIES
+                            APPEND_STRING PROPERTY LINK_FLAGS " -Xlinker /MANIFEST:NO")
+    endif()
   endif()
 endif()
 

@ldionne ldionne merged commit 57dabc1 into llvm:main Jun 28, 2024
54 of 57 checks passed
@ldionne ldionne deleted the review/fix-Xlinker-msvc branch June 28, 2024 15:38
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
…lvm#96967)

If we're compiling with Clang (not clang-cl) on Windows, we need to use
-Xlinker to pass the /MANIFEST option.

Fixes llvm#96430
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libcxx] it should pass -Xlinker when user uses clang instead of clang-cl for build libcxx
2 participants