[Lldb-commits] [PATCH] D86235: Fix swig scripts install target name

Jonas Devlieghere via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Aug 19 13:42:32 PDT 2020


JDevlieghere added a comment.

In D86235#2227052 <https://reviews.llvm.org/D86235#2227052>, @aadsm wrote:

> @JDevlieghere thanks for the quick review, but on the name I mean the actual `finish_swig_python_scripts`, this sounds like a step name and not a component distributed by llvm like `liblldb` ot `lldb-server`. That was the reason at the time I named it `lldb-python-scripts` because it was very clear what was being installed.
> Would you be fine with me changing `swig_scripts_target` back to `lldb-python-scripts`?

No objections at all. IIUC that would mean changing the first argument in `CMakeLists.txt:89` to:

  finish_swig_python("lldb-python" "${lldb_python_bindings_dir}" "${lldb_python_target_dir}")

and then in `CMakeLists.txt:166` something like

  set(swig_scripts_target "${swig_target}-scripts")
  set(swig_scripts_install_target "install-${swig_scripts_target}")

Sounds good to me, I didn't know anybody as explicitly specifying them anywhere, I wouldn't have broken that if I had known. My apologies.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86235/new/

https://reviews.llvm.org/D86235



More information about the lldb-commits mailing list