[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack added inline comments.


================
Comment at: lldb/source/Host/CMakeLists.txt:109-121
+
+    set(ANDROID_SOURCES
+      android/ZipFile.cpp
+      android/HostInfoAndroid.cpp
+      )
     if (CMAKE_SYSTEM_NAME MATCHES "Android")
+      list(APPEND ANDROID_SOURCES
----------------
bulbazord wrote:
> splhack wrote:
> > bulbazord wrote:
> > > I don't think this is correct to do. lldbHost is different than other libraries in that it's meant to provide functionality that lldb and lldb-server needs to work on the host system. Unconditionally adding a host subdirectory for android even when we're on Linux doesn't make sense to do.
> > I agree with that, however, I think this is pretty much only way to unblock writing and running unit tests for the Android host system, which has been no tests at all. The AndroidPlatformTest D152855 requires this to run the tests on Linux, and Android is basically Linux, so, hope it still makes sense for the unit testing capability. (only android/LibcGlue.cpp is not buildable for Linux target.)
> I see. You want to be able to run the android host tests but that's not easy to do right now. I think this is a reasonable thing to want to do (especially since so much of android support in lldb is not well tested AFAIK).
> 
> Instead of making this functionality specific to android hosts, why not make it possible to do on all platforms? This would do a few things:
> - It would make it easier to test on more than just Linux and Android machines.
> - It would open up the possibility of being able to use an apk on the host machine instead of needing to fetch it from the remote device via `adb shell dd`. An optimization for sure, but for large shared objects this may be able to improve performance.
> 
> What do you think?
Yeah, sounds great to me! will update move things around.
- include/lldb/Utility/ZipFile.h
- source/Utility/ZipFile.cpp
- `HostInfoAndroid::ResolveZipPath` -> `ZipFile::ResolveBionicZipPath` (this is bionic libc specific)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152759