[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

aeubanks added a comment. Herald added a project: All. Hi, sorry to ping a very old patch but `TestVSCode_runInTerminal.py` is still failing on my linux machine (and a coworker said the same thing). Searching my email for the test name, some 12.0.0 and 13.0.0 release email threads also mention

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

stella.stamenova added inline comments. Comment at: lldb/tools/lldb-vscode/FifoFiles.cpp:9 + +#if !defined(WIN32) +#include mstorsjo wrote: > stella.stamenova wrote: > > Also, this is no good. It works if you are targeting windows on windows, > > but not if

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

mstorsjo added inline comments. Comment at: lldb/tools/lldb-vscode/FifoFiles.cpp:9 + +#if !defined(WIN32) +#include stella.stamenova wrote: > Also, this is no good. It works if you are targeting windows on windows, but > not if you are targeting something else

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace added a comment. @stella.stamenova , I've just pushed a fix. Let me know if it actually works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951 ___

Re: [Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

Good to know. I'll work on that right now. Thanks! Il giorno mer 27 gen 2021 alle ore 11:41 Stella Stamenova via Phabricator < revi...@reviews.llvm.org> ha scritto: > stella.stamenova added inline comments. > > > > Comment at: lldb/tools/lldb-vscode/FifoFiles.cpp:9 > + > +#if

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

stella.stamenova added inline comments. Comment at: lldb/tools/lldb-vscode/FifoFiles.cpp:9 + +#if !defined(WIN32) +#include Also, this is no good. It works if you are targeting windows on windows, but not if you are targeting something else and building on

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace added a comment. I'll send a diff tomorrow morning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

stella.stamenova added a comment. In D93951#2523886 , @wallace wrote: > @stella.stamenova , is that build a Debug or Release build? I think that will > help me determine why it's failing Release. It passed in Debug. Repository: rG LLVM Github

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace added a comment. @stella.stamenova , is that build a Debug or Release build? I think that will help me determine why it's failing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

stella.stamenova added a comment. I am seeing this consistently fail on our internal Ubuntu bot: ERROR: test_missingArgInRunInTerminalLauncher (TestVSCode_runInTerminal.TestVSCode_runInTerminal) -- Traceback (most recent

Re: [Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

I've already submitted a fix, let's see if the buildbot gets fixed Il giorno lun 25 gen 2021 alle ore 14:18 Stella Stamenova via Phabricator < revi...@reviews.llvm.org> ha scritto: > stella.stamenova added a comment. > > It looks like this broke the windows lldb bot: > > > Repository: > rG

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

stella.stamenova added a comment. It looks like this broke the windows lldb bot: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0f0462cacf34: [vscode] Improve runInTerminal and support linux (authored by wallace). Changed prior to commit:

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Just one rename of an instance variable and this is good to go! Much cleaner without the directory and multiple files. Comment at: lldb/tools/lldb-vscode/FifoFiles.h:26

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace updated this revision to Diff 315655. wallace added a comment. improve tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951 Files:

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace updated this revision to Diff 315652. wallace added a comment. Addressed all comments: - Now using a single file for all the communication. Did indeed simplified a lot of code - Now adding timeouts to Send operations. This will help prevent the adaptor from undefinitely waiting for the

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

clayborg added a comment. Can we still try to use just one file? Isn't this file both read and write? The flow would be: - create 1 file using mkfifo(...) and pass it down as argument - launcher lldb-vscode will write pid or error JSON back into the stream, and will start to wait for some data

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace updated this revision to Diff 315527. wallace added a comment. Followed all the suggestions: - This is POSIX-only, but making it work for windows should only require deleting a few #ifdef blocks and using the CreateNamedPipe API instead of mkfifo. I'll do it later in another diff. -

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace marked 5 inline comments as done. wallace added inline comments. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2994 + // signal to prevent being paused forever. + const char *timeout_env_var = getenv("LLDB_VSCODE_RIT_TIMEOUT_IN_MS"); + int timeout_in_ms =

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace marked an inline comment as done. wallace added a comment. I'll think about just writing this as tcp sockets. That would for sure be cross platform Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace added a comment. I'll follow your recommendations. Regarding Windows, mkfifo can be replaced with CreateNamedPipe and should lead to the same behavior. Sadly, I don't have a Windows machine where I could even compile it... So this should be left for someone else or for when I can get

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

clayborg added a comment. And a test for current working directory Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

clayborg added a comment. We should also add a test that makes sure that env vars set in the launch config are received in the program that runs in the terminal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So I like that we are using files in the file system for things, but we are using 3 of them and that seems excessive. See inlined comments and see if we can get down to using

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace updated this revision to Diff 315015. wallace marked 10 inline comments as done. wallace added a comment. Herald added a subscriber: mgorny. Address all comments. - Moved all the communication logic between the debug adaptor and the launcher to classes, simplifying the code in

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Great stuff! See inline comments. A few new tests need to be added: - test running lldb-vscode with invalid "--launch-target" executable path so things fail to exec and verify

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace updated this revision to Diff 314151. wallace added a comment. Herald added a subscriber: JDevlieghere. nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93951/new/ https://reviews.llvm.org/D93951 Files:

[Lldb-commits] [PATCH] D93951: [vscode] Improve runInTerminal and support linux

wallace created this revision. wallace added reviewers: clayborg, kusmour, aadsm. Herald added a subscriber: dang. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Depends on D93874 . runInTerminal