[cpullvm] Fix Windows LLDB Python plugin distribution - #663
Conversation
151548d to
cd85a11
Compare
Jonathon Penix (jonathonpenix)
left a comment
There was a problem hiding this comment.
Thanks for catching/fixing this.
I'm ok if this goes in as-is, but if builds haven't finished yet it might be worth resolving WIN32 vs CMAKE_HOST_WIN32 just to avoid any possible confusion in the future. Maybe just putting the new change in the check to set LLDB_ENABLE_DYNAMIC_SCRIPTINTERPRETERS would be easiest
| CACHE STRING "" | ||
| ) | ||
| if(NOT WIN32) | ||
| list(APPEND LLVM_DISTRIBUTION_COMPONENTS lldbPluginScriptInterpreterPython) |
There was a problem hiding this comment.
It might be worth moving this down into the same check as where we enable LLDB_ENABLE_DYNAMIC_SCRIPTINTERPRETERS since that is what controls whether we need this or not.
It'd also resolve the mix between WIN32 and CMAKE_HOST_WIN32--I'm not actually sure what the right one is here, but IIUC they aren't the same and we need the checks to both fire at the same time.
(In practice they might be essentially the same since we never cross compile the toolchains, but I'm not sure if there are other 'gotchas' I'm missing)
|
Just as an example of what I mean, see https://github.com/qualcomm/cpullvm-toolchain/pull/665/changes I don't mind too much which way we go for the branch (I think they should be the same for what we're doing), just need to pick one. |
…LDB_ENABLE_DYNAMIC_SCRIPTINTERPRETERS We don't set LLDB_ENABLE_DYNAMIC_SCRIPTINTERPRETERS on Windows, which causes build failures. Signed-off-by: syalamar <syalamar@qti.qualcomm.com>
cd85a11 to
a630221
Compare
17fae84
into
qualcomm:release/qualcomm-software/23.x
Summary
Exclude
lldbPluginScriptInterpreterPythonfrom Windows distribution components because the plugin is not built when dynamic Python script interpreters are disabled on Windows.