labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
This looks great to me. Maybe wait a while to see if anyone else has any
thoughts on this?
This not your fault, but the thing that bothers me about this build process
(and which this patch makes visible) is how the locations of the python files
in source bear absolutely no resemblance to the locations the files will end up
installed. For instance, I would never have expected that something from the
"examples" folder ends up in an actual shippable artifact. I think it would be
great if we could rearrange the sources so that their source layout roughly
matches the way in which they are going to be used...
================
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}"
----------------
hhb wrote:
> 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.
Ok, that sounds fair. I think it would be great if this stuff could be moved to
the scripts folder.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69589/new/
https://reviews.llvm.org/D69589
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
