[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

[Haibo Huang via Phabricator via lldb-commits]

hhb marked an inline comment as done.
hhb added a comment.

In D69589#1726812 <https://reviews.llvm.org/D69589#1726812>, @labath wrote:

> This looks reasonable to me. I'm just wondering, now that these are separate targets, I guess that means they can be run in random order, right? Will that cause any problems, for instance when creating a package and its subpackage (formatters and formatters/cpp maybe)?


I try to avoid that problem by creating directory every time. Other than that I don't think there's any dependencies between two subpackages...

But in general, yes, that's something we should be careful of...



================
Comment at: lldb/CMakeLists.txt:133
+  add_copy_file_target(lldb_python_init
+    FILES          "${lldb_scripts_dir}/lldb.py"
+    DEST_DIR       "${lldb_python_build_path}"
----------------
labath wrote:
> Would it be possible to just make the swig step place this file into the correct place directly?
The difficulty is that, there is a config time path dependency like this:

swig_wrapper -> liblldb -> finish_swig (renamed to lldb_python_packages in this change)

Where liblldb uses the path of LLDBWrapPython.cpp in swig_wrapper. And here we reference the path of liblldb, to calculate the right path for python packages.

That means when swig_wrapper is created, we don't have a good way to figure out the right path for python packages and put the file there directly.

The best way to solve this is to hard code liblldb path for framework build. I.e. replace line 92 here:

```
get_target_property(liblldb_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
```

With something more constant. This removes the liblldb -> finish_swig dependency. Then all the code here can be moved to scripts/CMakeLists.txt (I think they belong there better anyway). And swig_wrapper can get access to python package path.

The only question is whether we can / should "predict" liblldb path for framework... I'll look more into this. But it probably deserves a separate change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589