[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory (RFC)

Tue Oct 11 08:46:33 PDT 2022

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

In D135631#3849295 <https://reviews.llvm.org/D135631#3849295>, @DavidSpickett wrote:

> So either way, if someone was running with no logs enabled we would have to ask them to enable them then re-run and get a new set of diagnostics (which is fine as is , not a criticism).

Yeah, the idea is that if we ask someone to enable logs (which is still very likely even with the diagnostics) we'd automatically include them in the diagnostics so they don't need to go look in multiple places.

In D135631#3849307 <https://reviews.llvm.org/D135631#3849307>, @DavidSpickett wrote:

> Also is the buffer option the same as "circular" here?
>
>   -h <log-handler> ( --log-handler <log-handler> )
>        Specify a log handler which determines where log messages are written.
>        Values: default | stream | circular | os

Yup

> So we could have diagnostics also save anything kept in "circular", on the off chance that writing to a file isn't possible.

Technically yes, but then we have two ad-hoc solutions that support half the log handlers. I think if we care about that we might as well go with the T-based log handler behind the scenes.

================
Comment at: lldb/test/Shell/Diagnostics/TestCopyLogs.test:3
+# RUN: mkdir -p %t
+# The "ll''db" below is not a typo but a way to prevent lit from substituting 'lldb'.
+# RUN: %lldb -o 'log enable ll''db commands -f %t/commands.log' -o 'diagnostics dump -d %t/diags'
----------------
labath wrote:
> That's cute, but I suspect windows will have a problem with that. `-s %s` (and putting the commands in this file) would be safer.
I thought about that, but I still need `%t` to be expanded. I guess I could put `log enable lldb commands -f ` in a file and append `%t/commands.log` to it. 

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

https://reviews.llvm.org/D135631