[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

Mon Oct 4 05:19:16 PDT 2021

mgorny marked an inline comment as done.
mgorny added inline comments.

================
Comment at: lldb/unittests/Host/CMakeLists.txt:27
+if (LLDB_ENABLE_TERMIOS AND
+    CMAKE_SYSTEM_NAME MATCHES "Darwin|FreeBSD|Linux|NetBSD|OpenBSD")
+  list(APPEND FILES
----------------
labath wrote:
> mgorny wrote:
> > labath wrote:
> > > mgorny wrote:
> > > > FTR, I've tested it on FreeBSD and Linux so far. NetBSD in progress. Darwin and OpenBSD support is based on what I've found in gnulib but explicit testing would be helpful.
> > > At this point, I'd just go with `NOT windows`
> > If I'm using `PseudoTerminal`, then I guess I don't need the platform check at all (and 'not Windows' is implied by `LLDB_ENABLE_TERMIOS`).
> Yep.
> 
> Although, I have a feeling that this will actually not work (as in, it will trigger the llvm unrecognized-source-file cmake alarm) when termios is disabled. IIRC, the llvm solution is to pass this file in some "optionally compiled sources" argument. The lldb solution would be to put the file in a subdirectory (posix, I guess).
> I'd probably go with the second one, since we already have a linux subfolder.
Sure, done that.

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

https://reviews.llvm.org/D110962