This is an archive of the discontinued LLVM Phabricator instance.

[CMake][LLDB] Resolve install conflict when `LLDB_BUILD_FRAMEWORK=ON`
ClosedPublic

Authored by paperchalice on Jan 7 2022, 9:58 PM.

Diff Detail

Event Timeline

paperchalice created this revision.Jan 7 2022, 9:58 PM
paperchalice requested review of this revision.Jan 7 2022, 9:58 PM
JDevlieghere requested changes to this revision.Jan 9 2022, 9:32 PM

Hey LJC. I'm the person maintaining the LLDB framework build, so please include me in any future patches that change that. I'm also on the majority of the blamelist for this file, which is always a useful way to find good reviewers. With that out of the way, I do have a few comments about this patch:

  • It doesn't look like the RPATH change is really related to the install target so please create a separate patch for that.
  • Note how the function this is (finish_swig_python) takes lldb_python_target_dir as an input argument. Downstream we create more than one directory, and this patch is going to break that.

You say that LLDB.framework/Resources/Python is installed by liblldb, but I can't immediately see where that's happening in API/CMakeLists.txt.

This revision now requires changes to proceed.Jan 9 2022, 9:32 PM
paperchalice edited the summary of this revision. (Show Details)

Install python scripts into canonical resource path.

JDevlieghere accepted this revision.Jan 13 2022, 6:47 PM

Looks reasonable. LGTM.

This revision is now accepted and ready to land.Jan 13 2022, 6:47 PM

Could you help me commit this change? Because I don't have permission. Thanks!