Re-merge A few updates around "transcript" (#92843) (#94067)

Problematic PR: #92843
Reverted by: #94088

The first PR added a test which fails in Linux builds (see the last few
comments there).
This PR contains all the changes in the first PR, plus the fix to the
said test.

---------

Co-authored-by: Roy Shi <[email protected]>

Author: royitaqi

lldb/include/lldb/API/SBCommandInterpreter.h

@@ -320,10 +320,17 @@ class SBCommandInterpreter {
 
   /// Returns a list of handled commands, output and error. Each element in
   /// the list is a dictionary with the following keys/values:
-  /// - "command" (string): The command that was executed.
+  /// - "command" (string): The command that was given by the user.
+  /// - "commandName" (string): The name of the executed command.
+  /// - "commandArguments" (string): The arguments of the executed command.
   /// - "output" (string): The output of the command. Empty ("") if no output.
   /// - "error" (string): The error of the command. Empty ("") if no error.
-  /// - "seconds" (float): The time it took to execute the command.
+  /// - "durationInSeconds" (float): The time it took to execute the command.
+  /// - "timestampInEpochSeconds" (int): The timestamp when the command is
+  ///   executed.
+  ///
+  /// Turn on settings `interpreter.save-transcript` for LLDB to populate
+  /// this list. Otherwise this list is empty.
   SBStructuredData GetTranscript();
 
 protected:

lldb/include/lldb/Interpreter/CommandInterpreter.h

@@ -776,10 +776,14 @@ class CommandInterpreter : public Broadcaster,
 
   /// Contains a list of handled commands and their details. Each element in
   /// the list is a dictionary with the following keys/values:
-  /// - "command" (string): The command that was executed.
+  /// - "command" (string): The command that was given by the user.
+  /// - "commandName" (string): The name of the executed command.
+  /// - "commandArguments" (string): The arguments of the executed command.
   /// - "output" (string): The output of the command. Empty ("") if no output.
   /// - "error" (string): The error of the command. Empty ("") if no error.
-  /// - "seconds" (float): The time it took to execute the command.
+  /// - "durationInSeconds" (float): The time it took to execute the command.
+  /// - "timestampInEpochSeconds" (int): The timestamp when the command is
+  ///   executed.
   ///
   /// Turn on settings `interpreter.save-transcript` for LLDB to populate
   /// this list. Otherwise this list is empty.

lldb/include/lldb/Target/Statistics.h

@@ -133,6 +133,7 @@ struct ConstStringStats {
 struct StatisticsOptions {
   bool summary_only = false;
   bool load_all_debug_info = false;
+  bool include_transcript = false;
 };
 
 /// A class that represents statistics for a since lldb_private::Target.

lldb/source/Commands/CommandObjectStats.cpp

@@ -81,6 +81,9 @@ class CommandObjectStatsDump : public CommandObjectParsed {
       case 'f':
         m_stats_options.load_all_debug_info = true;
         break;
+      case 't':
+        m_stats_options.include_transcript = true;
+        break;
       default:
         llvm_unreachable("Unimplemented option");
       }

lldb/source/Commands/Options.td

@@ -1425,4 +1425,8 @@ let Command = "statistics dump" in {
     Desc<"Dump the total possible debug info statistics. "
     "Force loading all the debug information if not yet loaded, and collect "
     "statistics with those.">;
+  def statistics_dump_transcript: Option<"transcript", "t">, Group<1>,
+    Desc<"If the setting interpreter.save-transcript is enabled and this "
+    "option is specified, include a JSON array with all commands the user and/"
+    "or scripts executed during a debug session.">;
 }

lldb/source/Interpreter/CommandInterpreter.cpp

@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <chrono>
 #include <cstdlib>
 #include <limits>
 #include <memory>
@@ -1909,6 +1910,11 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
 
     transcript_item = std::make_shared<StructuredData::Dictionary>();
     transcript_item->AddStringItem("command", command_line);
+    transcript_item->AddIntegerItem(
+        "timestampInEpochSeconds",
+        std::chrono::duration_cast<std::chrono::seconds>(
+            std::chrono::system_clock::now().time_since_epoch())
+            .count());
     m_transcript.AddItem(transcript_item);
   }
 
@@ -2056,6 +2062,14 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
         log, "HandleCommand, command line after removing command name(s): '%s'",
         remainder.c_str());
 
+    // To test whether or not transcript should be saved, `transcript_item` is
+    // used instead of `GetSaveTrasncript()`. This is because the latter will
+    // fail when the command is "settings set interpreter.save-transcript true".
+    if (transcript_item) {
+      transcript_item->AddStringItem("commandName", cmd_obj->GetCommandName());
+      transcript_item->AddStringItem("commandArguments", remainder);
+    }
+
     ElapsedTime elapsed(execute_time);
     cmd_obj->Execute(remainder.c_str(), result);
   }
@@ -2072,7 +2086,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
 
     transcript_item->AddStringItem("output", result.GetOutputData());
     transcript_item->AddStringItem("error", result.GetErrorData());
-    transcript_item->AddFloatItem("seconds", execute_time.get().count());
+    transcript_item->AddFloatItem("durationInSeconds",
+                                  execute_time.get().count());
   }
 
   return result.Succeeded();

lldb/source/Target/Statistics.cpp

@@ -16,6 +16,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/UnixSignals.h"
+#include "lldb/Utility/StructuredData.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -225,6 +226,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 
   const bool summary_only = options.summary_only;
   const bool load_all_debug_info = options.load_all_debug_info;
+  const bool include_transcript = options.include_transcript;
 
   json::Array json_targets;
   json::Array json_modules;
@@ -364,5 +366,36 @@ llvm::json::Value DebuggerStats::ReportStatistics(
     global_stats.try_emplace("commands", std::move(cmd_stats));
   }
 
+  if (include_transcript) {
+    // When transcript is available, add it to the to-be-returned statistics.
+    //
+    // NOTE:
+    // When the statistics is polled by an LLDB command:
+    // - The transcript in the returned statistics *will NOT* contain the
+    //   returned statistics itself (otherwise infinite recursion).
+    // - The returned statistics *will* be written to the internal transcript
+    //   buffer. It *will* appear in the next statistcs or transcript poll.
+    //
+    // For example, let's say the following commands are run in order:
+    // - "version"
+    // - "statistics dump"  <- call it "A"
+    // - "statistics dump"  <- call it "B"
+    // The output of "A" will contain the transcript of "version" and
+    // "statistics dump" (A), with the latter having empty output. The output
+    // of B will contain the trascnript of "version", "statistics dump" (A),
+    // "statistics dump" (B), with A's output populated and B's output empty.
+    const StructuredData::Array &transcript =
+        debugger.GetCommandInterpreter().GetTranscript();
+    if (transcript.GetSize() != 0) {
+      std::string buffer;
+      llvm::raw_string_ostream ss(buffer);
+      json::OStream json_os(ss);
+      transcript.Serialize(json_os);
+      if (auto json_transcript = llvm::json::parse(ss.str()))
+        global_stats.try_emplace("transcript",
+                                 std::move(json_transcript.get()));
+    }
+  }
+
   return std::move(global_stats);
 }

lldb/test/API/commands/statistics/basic/TestStats.py

@@ -623,3 +623,50 @@ def test_had_frame_variable_errors(self):
         # Verify that the top level statistic that aggregates the number of
         # modules with debugInfoHadVariableErrors is greater than zero
         self.assertGreater(stats["totalModuleCountWithVariableErrors"], 0)
+
+    def test_transcript_happy_path(self):
+        """
+        Test "statistics dump" and the transcript information.
+        """
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        target = self.createTestTarget(file_path=exe)
+        self.runCmd("settings set interpreter.save-transcript true")
+        self.runCmd("version")
+
+        # Verify the output of a first "statistics dump"
+        debug_stats = self.get_stats("--transcript")
+        self.assertIn("transcript", debug_stats)
+        transcript = debug_stats["transcript"]
+        self.assertEqual(len(transcript), 2)
+        self.assertEqual(transcript[0]["commandName"], "version")
+        self.assertEqual(transcript[1]["commandName"], "statistics dump")
+        # The first "statistics dump" in the transcript should have no output
+        self.assertNotIn("output", transcript[1])
+
+        # Verify the output of a second "statistics dump"
+        debug_stats = self.get_stats("--transcript")
+        self.assertIn("transcript", debug_stats)
+        transcript = debug_stats["transcript"]
+        self.assertEqual(len(transcript), 3)
+        self.assertEqual(transcript[0]["commandName"], "version")
+        self.assertEqual(transcript[1]["commandName"], "statistics dump")
+        # The first "statistics dump" in the transcript should have output now
+        self.assertIn("output", transcript[1])
+        self.assertEqual(transcript[2]["commandName"], "statistics dump")
+        # The second "statistics dump" in the transcript should have no output
+        self.assertNotIn("output", transcript[2])
+
+    def test_transcript_should_not_exist_when_not_asked_for(self):
+        """
+        Test "statistics dump" and the transcript information.
+        """
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        target = self.createTestTarget(file_path=exe)
+        self.runCmd("settings set interpreter.save-transcript true")
+        self.runCmd("version")
+
+        # Verify the output of a first "statistics dump"
+        debug_stats = self.get_stats()  # Not with "--transcript"
+        self.assertNotIn("transcript", debug_stats)

lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py

@@ -104,9 +104,10 @@ def getTranscriptAsPythonObject(self, ci):
 
         return json.loads(stream.GetData())
 
-    def test_structured_transcript(self):
+    def test_get_transcript(self):
         """Test structured transcript generation and retrieval."""
         ci = self.buildAndCreateTarget()
+        self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
 
         # Make sure the "save-transcript" setting is on
         self.runCmd("settings set interpreter.save-transcript true")
@@ -118,8 +119,7 @@ def test_structured_transcript(self):
         res = lldb.SBCommandReturnObject()
         ci.HandleCommand("version", res)
         ci.HandleCommand("an-unknown-command", res)
-        ci.HandleCommand("breakpoint set -f main.c -l %d" % self.line, res)
-        ci.HandleCommand("r", res)
+        ci.HandleCommand("br s -f main.c -l %d" % self.line, res)
         ci.HandleCommand("p a", res)
         ci.HandleCommand("statistics dump", res)
         total_number_of_commands = 6
@@ -130,56 +130,66 @@ def test_structured_transcript(self):
         # All commands should have expected fields.
         for command in transcript:
             self.assertIn("command", command)
+            # Unresolved commands don't have "commandName"/"commandArguments".
+            # We will validate these fields below, instead of here.
             self.assertIn("output", command)
             self.assertIn("error", command)
-            self.assertIn("seconds", command)
+            self.assertIn("durationInSeconds", command)
+            self.assertIn("timestampInEpochSeconds", command)
 
         # The following validates individual commands in the transcript.
         #
         # Notes:
         # 1. Some of the asserts rely on the exact output format of the
         #    commands. Hopefully we are not changing them any time soon.
-        # 2. We are removing the "seconds" field from each command, so that
-        #    some of the validations below can be easier / more readable.
+        # 2. We are removing the time-related fields from each command, so
+        #    that some of the validations below can be easier / more readable.
         for command in transcript:
-            del(command["seconds"])
+            del command["durationInSeconds"]
+            del command["timestampInEpochSeconds"]
 
         # (lldb) version
         self.assertEqual(transcript[0]["command"], "version")
+        self.assertEqual(transcript[0]["commandName"], "version")
+        self.assertEqual(transcript[0]["commandArguments"], "")
         self.assertIn("lldb version", transcript[0]["output"])
         self.assertEqual(transcript[0]["error"], "")
 
         # (lldb) an-unknown-command
         self.assertEqual(transcript[1],
             {
                 "command": "an-unknown-command",
+                # Unresolved commands don't have "commandName"/"commandArguments"
                 "output": "",
                 "error": "error: 'an-unknown-command' is not a valid command.\n",
             })
 
-        # (lldb) breakpoint set -f main.c -l <line>
-        self.assertEqual(transcript[2]["command"], "breakpoint set -f main.c -l %d" % self.line)
+        # (lldb) br s -f main.c -l <line>
+        self.assertEqual(transcript[2]["command"], "br s -f main.c -l %d" % self.line)
+        self.assertEqual(transcript[2]["commandName"], "breakpoint set")
+        self.assertEqual(
+            transcript[2]["commandArguments"], "-f main.c -l %d" % self.line
+        )
         # Breakpoint 1: where = a.out`main + 29 at main.c:5:3, address = 0x0000000100000f7d
         self.assertIn("Breakpoint 1: where = a.out`main ", transcript[2]["output"])
         self.assertEqual(transcript[2]["error"], "")
 
-        # (lldb) r
-        self.assertEqual(transcript[3]["command"], "r")
-        # Process 25494 launched: '<path>/TestCommandInterpreterAPI.test_structured_transcript/a.out' (x86_64)
-        self.assertIn("Process", transcript[3]["output"])
-        self.assertIn("launched", transcript[3]["output"])
-        self.assertEqual(transcript[3]["error"], "")
-
         # (lldb) p a
-        self.assertEqual(transcript[4],
+        self.assertEqual(transcript[3],
             {
                 "command": "p a",
-                "output": "(int) 123\n",
-                "error": "",
+                "commandName": "dwim-print",
+                "commandArguments": "-- a",
+                "output": "",
+                "error": "error: <user expression 0>:1:1: use of undeclared identifier 'a'\n    1 | a\n      | ^\n",
             })
 
         # (lldb) statistics dump
-        statistics_dump = json.loads(transcript[5]["output"])
+        self.assertEqual(transcript[4]["command"], "statistics dump")
+        self.assertEqual(transcript[4]["commandName"], "statistics dump")
+        self.assertEqual(transcript[4]["commandArguments"], "")
+        self.assertEqual(transcript[4]["error"], "")
+        statistics_dump = json.loads(transcript[4]["output"])
         # Dump result should be valid JSON
         self.assertTrue(statistics_dump is not json.JSONDecodeError)
         # Dump result should contain expected fields
@@ -189,15 +199,15 @@ def test_structured_transcript(self):
         self.assertIn("targets", statistics_dump)
 
     def test_save_transcript_setting_default(self):
-        ci = self.buildAndCreateTarget()
-        res = lldb.SBCommandReturnObject()
+        ci = self.dbg.GetCommandInterpreter()
+        self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
 
         # The setting's default value should be "false"
         self.runCmd("settings show interpreter.save-transcript", "interpreter.save-transcript (boolean) = false\n")
-        # self.assertEqual(res.GetOutput(), )
 
     def test_save_transcript_setting_off(self):
-        ci = self.buildAndCreateTarget()
+        ci = self.dbg.GetCommandInterpreter()
+        self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
 
         # Make sure the setting is off
         self.runCmd("settings set interpreter.save-transcript false")
@@ -208,8 +218,8 @@ def test_save_transcript_setting_off(self):
         self.assertEqual(transcript, [])
 
     def test_save_transcript_setting_on(self):
-        ci = self.buildAndCreateTarget()
-        res = lldb.SBCommandReturnObject()
+        ci = self.dbg.GetCommandInterpreter()
+        self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
 
         # Make sure the setting is on
         self.runCmd("settings set interpreter.save-transcript true")
@@ -220,7 +230,7 @@ def test_save_transcript_setting_on(self):
         self.assertEqual(len(transcript), 1)
         self.assertEqual(transcript[0]["command"], "version")
 
-    def test_save_transcript_returns_copy(self):
+    def test_get_transcript_returns_copy(self):
         """
         Test that the returned structured data is *at least* a shallow copy.
 
@@ -229,7 +239,8 @@ def test_save_transcript_returns_copy(self):
         because there is no logic in the command interpreter to modify a
         transcript item (representing a command) after it has been returned.
         """
-        ci = self.buildAndCreateTarget()
+        ci = self.dbg.GetCommandInterpreter()
+        self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
 
         # Make sure the setting is on
         self.runCmd("settings set interpreter.save-transcript true")