[Lldb-commits] [lldb] [lldb-dap] Added "port" property to vscode "attach" command. (PR #91570)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Mon Jun 10 00:08:39 PDT 2024


================
@@ -0,0 +1,202 @@
+"""
+Test lldb-dap "port" configuration to "attach" request
+"""
+
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbplatformutil
+import lldbdap_testcase
+import os
+import shutil
+import subprocess
+import tempfile
+import threading
+import sys
+import socket
+import select
+
+
+# A class representing a pipe for communicating with debug server.
+# This class includes menthods to open the pipe and read the port number from it.
+class Pipe(object):
+    def __init__(self, prefix):
+        self.name = os.path.join(prefix, "stub_port_number")
+        os.mkfifo(self.name)
+        self._fd = os.open(self.name, os.O_RDONLY | os.O_NONBLOCK)
+
+    def finish_connection(self, timeout):
+        pass
+
+    def read(self, size, timeout):
+        (readers, _, _) = select.select([self._fd], [], [], timeout)
+        if self._fd not in readers:
+            raise TimeoutError
+        return os.read(self._fd, size)
+
+    def close(self):
+        os.close(self._fd)
----------------
labath wrote:

I'm not sure I follow you're reasoning (please elaborate), but I'm pretty sure I don't agree with your conclusion.

It is true that the Pipe class is defined differently on different (host) platforms. It's also true that it's interface is more complicated than if it was designed to be used exclusively on (e.g.) posix system. However, I don't think any of this is a reason to not make it common code. However, I don't think any of this is a reason to *not* make it common code. If anything, I would say it's the opposite. Abstracting platform-specific behavior behind common interfaces is the best thing we can do in these circumstances. Sometimes that means the common interface will be a bit more complicated but that's just the reality we live in.

In this case, somebody (disclaimer: it was me, several years ago) already created a common interface and implemented it on both platforms. I don't see a reason to not make use of that. There is no fundamental reason why this test of yours cannot work on windows. If someday, someone cared enough to try to make it work, he would have to (re)implement the pipe abstraction to make it run. Why make him do that if we can make use of an abstraction that's already available?

I'm sorry, but I just don't understand what kind of confusion you're referring to. I don't know if it helps, but I can certainly imagine changing the class name to something less generic (e.g. `HostPipe` ?)

https://github.com/llvm/llvm-project/pull/91570


More information about the lldb-commits mailing list