[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a subscriber: stella.stamenova.
sgraenitz added a comment.

In D146058#4193470 <https://reviews.llvm.org/D146058#4193470>, @DavidSpickett wrote:

> Myself and Omair both work on Linaro's bots. We run lldb checks on linux arm/aarch64 and Windows on Arm. We could look at installing it.

Right now, my focus is x86 Windows and Linux, but thanks for the offer! So, this would be https://lab.llvm.org/buildbot/#/builders/219 (Windows AArch64) and https://lab.llvm.org/buildbot/#/builders/17 (Linux AArch32)? Let me discuss this internally and come back to you soon.

> Pavel runs the x86 debian bot and I think the x86 Windows bot may have been retired, or at least is offline at the moment (https://lab.llvm.org/buildbot/#/builders/83)

Yes, that would be cool. @labath What do you think?

In D146058#4193471 <https://reviews.llvm.org/D146058#4193471>, @DavidSpickett wrote:

> The Windows x86 bot has in fact been removed: https://github.com/llvm/llvm-zorg/commit/acb6223670a41a7fe5d9846075e6b92bd157d78f

Oh I still saw that one running a few days back! @stella.stamenova in the review you mention a second windows lldb bot, is it x86 as well? I don't see it in https://lab.llvm.org/buildbot/#/console



================
Comment at: lldb/test/CMakeLists.txt:39
+if (LLDB_TEST_OBJC_GNUSTEP_DIR)
+  message(STATUS "Found GNUstep ObjC runtime: ${LLDB_TEST_OBJC_GNUSTEP_DIR}/${gnustep_info}")
+endif()
----------------
theraven wrote:
> DavidSpickett wrote:
> > sgraenitz wrote:
> > > Might be worth a `FindGNUstep.cmake` at some point? (Or is there one somewhere?)
> > > 
> > > Windows default install dir:
> > > ```
> > > -- Found GNUstep ObjC runtime: C:/Program Files (x86)/libobjc/lib/objc.dll
> > > ```
> > > 
> > > Linux default install dir:
> > > ```
> > > -- Found GNUstep ObjC runtime: /usr/local/lib/libobjc.so
> > > ```
> > I think we generally use UPPERCASE names for cmake variables.
> This is somewhat complicated.  If `gnustep-config` exists, the runtime supports installing itself into a location defined by the gnustep-make install, which supports NeXT, Apple, and FHS-style layouts.  It's probably not worth trying to autodetect the general case.
> This is somewhat complicated.
> It's probably not worth trying to autodetect the general case.

Configuring with cmake and running the install target gives me these paths. Do you mean I should remove them and just let the user set `LLDB_TEST_OBJC_GNUSTEP_DIR`?

> If gnustep-config exists, the runtime supports installing itself into a location defined by the gnustep-make install, which supports NeXT, Apple, and FHS-style layouts.

Right now, I am assuming to find the shared library in the `lib` subdirectory and headers in `include`. Is that different in the other layouts? Do we need to support that?

> I think we generally use UPPERCASE names for cmake variables.

I  think the convention is: UPPERCASE for global variables, lowercase for local variables


================
Comment at: lldb/test/Shell/helper/build.py:687
+        args.extend(['-o', obj])
+        args.append(source)
+
----------------
DavidSpickett wrote:
> Why did these 2 need to move?
The flags wouldn't apply for previous source files otherwise. I opted to move them both in order to keep them close in the resulting command.


================
Comment at: lldb/test/Shell/lit.cfg.py:27
 # by individual lit.local.cfg files in the test subdirectories.
-config.suffixes = ['.test', '.cpp', '.s']
+config.suffixes = ['.test', '.cpp', '.s', '.m']
 
----------------
DavidSpickett wrote:
> There are a couple of .m files in the Shell dir already, I assume the related tests still pass for those?
Interesting, it's exactly one .m and one .mm outside `Input` directories right? Let me double-check that.
```
SymbolFile/DWARF/clang-ast-from-dwarf-objc-property.m
SymbolFile/DWARF/x86/module-ownership.mm
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146058