[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 22 13:09:29 PST 2025
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/123837
>From bc7bb2471e44ca7441f592611fd293f2b8c90435 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 21 Jan 2025 14:34:49 -0800
Subject: [PATCH 1/5] Have Progress message updates include their messages when
sent over DAP
---
lldb/tools/lldb-dap/ProgressEvent.cpp | 15 ++++++++++-----
lldb/tools/lldb-dap/ProgressEvent.h | 3 ++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/lldb/tools/lldb-dap/ProgressEvent.cpp b/lldb/tools/lldb-dap/ProgressEvent.cpp
index 0dcc2ee81001d5..7807ecfb6c34d0 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.cpp
+++ b/lldb/tools/lldb-dap/ProgressEvent.cpp
@@ -117,6 +117,10 @@ json::Value ProgressEvent::ToJSON() const {
body.try_emplace("cancellable", false);
}
+ if (m_event_type == progressUpdate) {
+ EmplaceSafeString(body, "message", m_message);
+ }
+
std::string timestamp(llvm::formatv("{0:f9}", m_creation_time.count()));
EmplaceSafeString(body, "timestamp", timestamp);
@@ -164,10 +168,11 @@ const ProgressEvent &ProgressEventManager::GetMostRecentEvent() const {
return m_last_update_event ? *m_last_update_event : m_start_event;
}
-void ProgressEventManager::Update(uint64_t progress_id, uint64_t completed,
- uint64_t total) {
- if (std::optional<ProgressEvent> event = ProgressEvent::Create(
- progress_id, std::nullopt, completed, total, &GetMostRecentEvent())) {
+void ProgressEventManager::Update(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total) {
+ if (std::optional<ProgressEvent> event =
+ ProgressEvent::Create(progress_id, StringRef(message), completed,
+ total, &GetMostRecentEvent())) {
if (event->GetEventType() == progressEnd)
m_finished = true;
@@ -227,7 +232,7 @@ void ProgressEventReporter::Push(uint64_t progress_id, const char *message,
m_unreported_start_events.push(event_manager);
}
} else {
- it->second->Update(progress_id, completed, total);
+ it->second->Update(progress_id, message, completed, total);
if (it->second->Finished())
m_event_managers.erase(it);
}
diff --git a/lldb/tools/lldb-dap/ProgressEvent.h b/lldb/tools/lldb-dap/ProgressEvent.h
index 72317b879c803a..8494221890a1c8 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.h
+++ b/lldb/tools/lldb-dap/ProgressEvent.h
@@ -99,7 +99,8 @@ class ProgressEventManager {
/// Receive a new progress event for the start event and try to report it if
/// appropriate.
- void Update(uint64_t progress_id, uint64_t completed, uint64_t total);
+ void Update(uint64_t progress_id, const char *message, uint64_t completed,
+ uint64_t total);
/// \return
/// \b true if a \a progressEnd event has been notified. There's no
>From faefdb6487eb97cca56755a20597f3d6a7348c6c Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 21 Jan 2025 15:16:46 -0800
Subject: [PATCH 2/5] PR feedback, pass string ref instead of char *
---
lldb/tools/lldb-dap/ProgressEvent.cpp | 12 +++++-------
lldb/tools/lldb-dap/ProgressEvent.h | 2 +-
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/lldb/tools/lldb-dap/ProgressEvent.cpp b/lldb/tools/lldb-dap/ProgressEvent.cpp
index 7807ecfb6c34d0..6a4978c055e516 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.cpp
+++ b/lldb/tools/lldb-dap/ProgressEvent.cpp
@@ -117,9 +117,8 @@ json::Value ProgressEvent::ToJSON() const {
body.try_emplace("cancellable", false);
}
- if (m_event_type == progressUpdate) {
+ if (m_event_type == progressUpdate)
EmplaceSafeString(body, "message", m_message);
- }
std::string timestamp(llvm::formatv("{0:f9}", m_creation_time.count()));
EmplaceSafeString(body, "timestamp", timestamp);
@@ -168,11 +167,10 @@ const ProgressEvent &ProgressEventManager::GetMostRecentEvent() const {
return m_last_update_event ? *m_last_update_event : m_start_event;
}
-void ProgressEventManager::Update(uint64_t progress_id, const char *message,
+void ProgressEventManager::Update(uint64_t progress_id, llvm::StringRef message,
uint64_t completed, uint64_t total) {
- if (std::optional<ProgressEvent> event =
- ProgressEvent::Create(progress_id, StringRef(message), completed,
- total, &GetMostRecentEvent())) {
+ if (std::optional<ProgressEvent> event = ProgressEvent::Create(
+ progress_id, message, completed, total, &GetMostRecentEvent())) {
if (event->GetEventType() == progressEnd)
m_finished = true;
@@ -232,7 +230,7 @@ void ProgressEventReporter::Push(uint64_t progress_id, const char *message,
m_unreported_start_events.push(event_manager);
}
} else {
- it->second->Update(progress_id, message, completed, total);
+ it->second->Update(progress_id, StringRef(message), completed, total);
if (it->second->Finished())
m_event_managers.erase(it);
}
diff --git a/lldb/tools/lldb-dap/ProgressEvent.h b/lldb/tools/lldb-dap/ProgressEvent.h
index 8494221890a1c8..d1b9b9dd887cd8 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.h
+++ b/lldb/tools/lldb-dap/ProgressEvent.h
@@ -99,7 +99,7 @@ class ProgressEventManager {
/// Receive a new progress event for the start event and try to report it if
/// appropriate.
- void Update(uint64_t progress_id, const char *message, uint64_t completed,
+ void Update(uint64_t progress_id, llvm::StringRef message, uint64_t completed,
uint64_t total);
/// \return
>From 56846b2fb5f491311af5ebfe98750c3159259e66 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 22 Jan 2025 11:56:15 -0800
Subject: [PATCH 3/5] Add new test for progress
---
.../test/API/tools/lldb-dap/progress/Makefile | 3 +
.../lldb-dap/progress/Progress_emitter.py | 86 +++++++++++++++++++
.../lldb-dap/progress/TestDAP_Progress.py | 49 +++++++++++
.../test/API/tools/lldb-dap/progress/main.cpp | 5 ++
4 files changed, 143 insertions(+)
create mode 100644 lldb/test/API/tools/lldb-dap/progress/Makefile
create mode 100644 lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
create mode 100755 lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
create mode 100644 lldb/test/API/tools/lldb-dap/progress/main.cpp
diff --git a/lldb/test/API/tools/lldb-dap/progress/Makefile b/lldb/test/API/tools/lldb-dap/progress/Makefile
new file mode 100644
index 00000000000000..99998b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/progress/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
new file mode 100644
index 00000000000000..8f2a504f8320f1
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -0,0 +1,86 @@
+import inspect
+import optparse
+import shlex
+import sys
+import time
+
+import lldb
+
+
+class ProgressTesterCommand:
+ program = "test-progress"
+
+ @classmethod
+ def register_lldb_command(cls, debugger, module_name):
+ parser = cls.create_options()
+ cls.__doc__ = parser.format_help()
+ # Add any commands contained in this module to LLDB
+ command = "command script add -c %s.%s %s" % (
+ module_name,
+ cls.__name__,
+ cls.program,
+ )
+ debugger.HandleCommand(command)
+ print(
+ 'The "{0}" command has been installed, type "help {0}" or "{0} '
+ '--help" for detailed help.'.format(cls.program)
+ )
+
+ @classmethod
+ def create_options(cls):
+ usage = "usage: %prog [options]"
+ description = "Jacob Lalonde's sbprogress testing tool"
+ # Opt parse is deprecated, but leaving this the way it is because it allows help formating
+ # Additionally all our commands use optparse right now, ideally we migrate them all in one go.
+ parser = optparse.OptionParser(
+ description=description, prog=cls.program, usage=usage
+ )
+
+ parser.add_option(
+ "--total", dest="total", help="Total to count up.", type="int"
+ )
+
+ parser.add_option(
+ "--seconds",
+ dest="seconds",
+ help="Total number of seconds to wait between increments",
+ type="int",
+ )
+
+ return parser
+
+ def get_short_help(self):
+ return "Progress Tester"
+
+ def get_long_help(self):
+ return self.help_string
+
+ def __init__(self, debugger, unused):
+ self.parser = self.create_options()
+ self.help_string = self.parser.format_help()
+
+ def __call__(self, debugger, command, exe_ctx, result):
+
+ command_args = shlex.split(command)
+ try:
+ (cmd_options, args) = self.parser.parse_args(command_args)
+ except:
+ result.SetError("option parsing failed")
+ return
+
+ total = cmd_options.total
+ progress = lldb.SBProgress("Progress tester", "Detail", total, debugger)
+
+ # This actually should start at 1 but it's 6:30 on a Friday...
+ for i in range(1, total):
+ progress.Increment(1, f"Step {i}")
+ time.sleep(cmd_options.seconds)
+
+
+def __lldb_init_module(debugger, dict):
+ # Register all classes that have a register_lldb_command method
+ for _name, cls in inspect.getmembers(sys.modules[__name__]):
+ if inspect.isclass(cls) and callable(
+ getattr(cls, "register_lldb_command", None)
+ ):
+ cls.register_lldb_command(debugger, __name__)
diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
new file mode 100755
index 00000000000000..ad166578e224cd
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -0,0 +1,49 @@
+"""
+Test lldb-dap output events
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import os
+import time
+
+import lldbdap_testcase
+
+
+class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase):
+ @skipIfWindows
+ def test_output(self):
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
+ print(f"Progress emitter path: {progress_emitter}")
+ source = "main.cpp"
+ # Set breakpoint in the thread function so we can step the threads
+ breakpoint_ids = self.set_source_breakpoints(
+ source, [line_number(source, "// break here")]
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+ self.dap_server.request_evaluate(
+ f"`command script import {progress_emitter}", context="repl"
+ )
+ self.dap_server.request_evaluate(
+ "`test-progress --total 3 --seconds 1", context="repl"
+ )
+
+ self.dap_server.wait_for_event("progressEnd", 15)
+ # Expect at least a start, an update, and end event
+ # However because the progress is an RAII object and we can't guaruntee
+ # it's deterministic destruction in the python API, we verify just start and update
+ # otherwise this test could be flakey.
+ self.assertTrue(len(self.dap_server.progress_events) > 0)
+ start_found = False
+ update_found = False
+ for event in self.dap_server.progress_events:
+ event_type = event["event"]
+ if "progressStart" in event_type:
+ start_found = True
+ if "progressUpdate" in event_type:
+ update_found = True
+
+ self.assertTrue(start_found)
+ self.assertTrue(update_found)
diff --git a/lldb/test/API/tools/lldb-dap/progress/main.cpp b/lldb/test/API/tools/lldb-dap/progress/main.cpp
new file mode 100644
index 00000000000000..3bac5d0fd6db1a
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/progress/main.cpp
@@ -0,0 +1,5 @@
+int main() {
+ char *ptr = "unused";
+ // break here
+ return 0;
+}
>From f6d8d81fcabec54b8d03755a12dde708e6cfde96 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 22 Jan 2025 12:14:43 -0800
Subject: [PATCH 4/5] Appease darker
---
lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
index 8f2a504f8320f1..f2ac77dce8f3a6 100644
--- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -60,7 +60,6 @@ def __init__(self, debugger, unused):
self.help_string = self.parser.format_help()
def __call__(self, debugger, command, exe_ctx, result):
-
command_args = shlex.split(command)
try:
(cmd_options, args) = self.parser.parse_args(command_args)
>From e7bc9023fa4a4267dad486223f11618ca547991f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 22 Jan 2025 13:08:59 -0800
Subject: [PATCH 5/5] Clean up code when copying personal tool, explain the
raii comment better.
---
lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py | 3 +--
lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
index f2ac77dce8f3a6..7f4055cab9ddda 100644
--- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -29,7 +29,7 @@ def register_lldb_command(cls, debugger, module_name):
@classmethod
def create_options(cls):
usage = "usage: %prog [options]"
- description = "Jacob Lalonde's sbprogress testing tool"
+ description = "SBProgress testing tool"
# Opt parse is deprecated, but leaving this the way it is because it allows help formating
# Additionally all our commands use optparse right now, ideally we migrate them all in one go.
parser = optparse.OptionParser(
@@ -70,7 +70,6 @@ def __call__(self, debugger, command, exe_ctx, result):
total = cmd_options.total
progress = lldb.SBProgress("Progress tester", "Detail", total, debugger)
- # This actually should start at 1 but it's 6:30 on a Friday...
for i in range(1, total):
progress.Increment(1, f"Step {i}")
time.sleep(cmd_options.seconds)
diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
index ad166578e224cd..36c0cef9c47143 100755
--- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -32,7 +32,7 @@ def test_output(self):
self.dap_server.wait_for_event("progressEnd", 15)
# Expect at least a start, an update, and end event
- # However because the progress is an RAII object and we can't guaruntee
+ # However because the underlying Progress instance is an RAII object and we can't guaruntee
# it's deterministic destruction in the python API, we verify just start and update
# otherwise this test could be flakey.
self.assertTrue(len(self.dap_server.progress_events) > 0)
More information about the lldb-commits
mailing list