[llvmbot]

Backport 687e66c

Requested by: @charles-zablit

[llvmbot]

@mstorsjo What do you think about merging this PR to the release branch?

[llvmorg-github-actions]

@llvm/pr-subscribers-lldb

Author: llvmbot

Changes

Backport 687e66c

Requested by: @charles-zablit

---

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

2 Files Affected:

• (modified) clang/tools/libclang/CMakeLists.txt (+1-1)
• (modified) lldb/source/API/CMakeLists.txt (+1-1)

diff --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt
index b0105f5a5f79f..19011c8a64096 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -120,7 +120,7 @@ if (MSVC AND ENABLE_SHARED AND ENABLE_STATIC)
   unset(ENABLE_STATIC)
 endif()
 
-if(MSVC)
+if(WIN32 AND NOT MINGW)
   set(output_name "libclang")
 else()
   set(output_name "clang")
diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt
index e27f90f2e873d..186f3b3b65299 100644
--- a/lldb/source/API/CMakeLists.txt
+++ b/lldb/source/API/CMakeLists.txt
@@ -265,7 +265,7 @@ elseif (LLDB_EXPORT_ALL_SYMBOLS)
   add_llvm_symbol_exports(liblldb ${exported_symbol_file})
 endif()
 
-if (NOT MSVC)
+if(NOT WIN32 OR MINGW)
   set_target_properties(liblldb
     PROPERTIES
     OUTPUT_NAME lldb

[mstorsjo]

LGTM

[dyung]

I don't think this meets the criteria for backports to the 22.x branch at this point. From my reading of the bug, it seems this can easily be worked around by renaming the produced binary? Is that not the case?

[mstorsjo]

I'm not sure how possible it is to work around it or not, but I'm quite sure the fix is very safe and very localized, so I would argue that there is no risk involved in backporting it. It's sad if it still falls outside of the backport criteria despite that...

[charles-zablit]

From my reading of the bug, it seems this can easily be worked around by renaming the produced binary? Is that not the case?

It can be worked around that way yes. Wouldn't this still count as a build failure when building lldb, even if there is a manual workaround for it?

[dyung]

From my reading of the bug, it seems this can easily be worked around by renaming the produced binary? Is that not the case?

It can be worked around that way yes. Wouldn't this still count as a build failure when building lldb, even if there is a manual workaround for it?

That's true. When did it start failing? Or has it always failed in this way?

[charles-zablit]

From my reading of the bug, it seems this can easily be worked around by renaming the produced binary? Is that not the case?

It can be worked around that way yes. Wouldn't this still count as a build failure when building lldb, even if there is a manual workaround for it?

That's true. When did it start failing? Or has it always failed in this way?

I assume it's been this way for a while. The lldb build bots use MSVC instead of clang, just like the release builds. This could have only been caught by a maintainer trying to build with clang, or an end user trying to build from source using clang (which is how we found this issue in the first place).

[mstorsjo]

I assume it's been this way for a while. The lldb build bots use MSVC instead of clang, just like the release builds. This could have only been caught by a maintainer trying to build with clang, or an end user trying to build from source using clang (which is how we found this issue in the first place).

Also, if building with clang-cl, this isn't an issue. It's only an issue if building with the clang driver, with the GNU like command line interface, in an MSVC environment. That is a much less tested configuration.

This configuration is tricky because it doesn't set the MSVC variable in cmake (as the compiler doesn't use the MSVC cl.exe style options), while it still expects other MSVC style aspects in the environment.

[c-rhodes]

I spoke to Doug, it's a small low-risk localized fix so we're happy, I've landed it.