[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Aleksandr Urakov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0891366571b2: [Windows] Introduce a switch for the 
`lldb-server` mode on Windows (authored by aleksandr.urakov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68258

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp


Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -81,13 +81,24 @@
   return ProcessSP(new ProcessWindows(target_sp, listener_sp));
 }
 
-void ProcessWindows::Initialize() {
-  static llvm::once_flag g_once_flag;
+static bool ShouldUseLLDBServer() {
+  llvm::StringRef use_lldb_server = ::getenv("LLDB_USE_LLDB_SERVER");
+  return use_lldb_server.equals_lower("on") ||
+ use_lldb_server.equals_lower("yes") ||
+ use_lldb_server.equals_lower("1") ||
+ use_lldb_server.equals_lower("true");
+}
 
-  llvm::call_once(g_once_flag, []() {
-PluginManager::RegisterPlugin(GetPluginNameStatic(),
-  GetPluginDescriptionStatic(), 
CreateInstance);
-  });
+void ProcessWindows::Initialize() {
+  if (!ShouldUseLLDBServer()) {
+static llvm::once_flag g_once_flag;
+
+llvm::call_once(g_once_flag, []() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(),
+CreateInstance);
+});
+  }
 }
 
 void ProcessWindows::Terminate() {}


Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -81,13 +81,24 @@
   return ProcessSP(new ProcessWindows(target_sp, listener_sp));
 }
 
-void ProcessWindows::Initialize() {
-  static llvm::once_flag g_once_flag;
+static bool ShouldUseLLDBServer() {
+  llvm::StringRef use_lldb_server = ::getenv("LLDB_USE_LLDB_SERVER");
+  return use_lldb_server.equals_lower("on") ||
+ use_lldb_server.equals_lower("yes") ||
+ use_lldb_server.equals_lower("1") ||
+ use_lldb_server.equals_lower("true");
+}
 
-  llvm::call_once(g_once_flag, []() {
-PluginManager::RegisterPlugin(GetPluginNameStatic(),
-  GetPluginDescriptionStatic(), CreateInstance);
-  });
+void ProcessWindows::Initialize() {
+  if (!ShouldUseLLDBServer()) {
+static llvm::once_flag g_once_flag;
+
+llvm::call_once(g_once_flag, []() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(),
+CreateInstance);
+});
+  }
 }
 
 void ProcessWindows::Terminate() {}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Hello! I am sorry for a delay with reply, I was OOO. Thanks all for the review!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth accepted this revision.
amccarth added a comment.

Given Pavel's comment, this LGTM.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68258#1691485 , @amccarth wrote:

> The environment variable for using the native PDB was always intended to be 
> temporary, since the native PDB reader would eventually be the only PDB 
> reader.


I don't know whether that's good or bad, but I think this variable is precisely 
the same kind of "temporary" as the other one. :)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

In D68258#1690757 , @aleksandr.urakov 
wrote:

> In D68258#1690756 , 
> @aleksandr.urakov wrote:
>
> > I've made it in the way similar to Zachary have made for the 
> > `SymbolFileNativePDB` plugin. An environment variable could be more 
> > convenient e.g. to run a bunch of tests using the `lldb-test` option.
>


The environment variable for using the native PDB was always intended to be 
temporary, since the native PDB reader would eventually be the only PDB reader.

In general, I find environment variables harder to manage (especially on 
Windows) than command line options.  They're invisible global variables that 
compound all the environmental issues that make building and testing so flaky 
and hard to diagnose.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

In D68258#1690756 , @aleksandr.urakov 
wrote:

> I've made it in the way similar to Zachary have made for the 
> `SymbolFileNativePDB` plugin. An environment variable could be more 
> convenient e.g. to run a bunch of tests using the `lldb-test` option.


I'm sorry, sure I mean the `lldb-server` option.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

I've made it in the way similar to Zachary have made for the 
`SymbolFileNativePDB` plugin. An environment variable could be more convenient 
e.g. to run a bunch of tests using the `lldb-test` option.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

Why an environment variable rather than a command line option?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Sounds reasonable. Thanks for doing this.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: labath, amccarth, asmith, stella.stamenova.
aleksandr.urakov added a project: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.

This patch introduces a switch, based on the environment variable 
`LLDB_USE_LLDB_SERVER`, to determine whether to use the `ProcessWindows` plugin 
(the old way) or the `lldb-server` way for debugging on Windows.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68258

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp


Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -81,13 +81,24 @@
   return ProcessSP(new ProcessWindows(target_sp, listener_sp));
 }
 
-void ProcessWindows::Initialize() {
-  static llvm::once_flag g_once_flag;
+static bool ShouldUseLLDBServer() {
+  llvm::StringRef use_lldb_server = ::getenv("LLDB_USE_LLDB_SERVER");
+  return use_lldb_server.equals_lower("on") ||
+ use_lldb_server.equals_lower("yes") ||
+ use_lldb_server.equals_lower("1") ||
+ use_lldb_server.equals_lower("true");
+}
 
-  llvm::call_once(g_once_flag, []() {
-PluginManager::RegisterPlugin(GetPluginNameStatic(),
-  GetPluginDescriptionStatic(), 
CreateInstance);
-  });
+void ProcessWindows::Initialize() {
+  if (!ShouldUseLLDBServer()) {
+static llvm::once_flag g_once_flag;
+
+llvm::call_once(g_once_flag, []() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(),
+CreateInstance);
+});
+  }
 }
 
 void ProcessWindows::Terminate() {}


Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -81,13 +81,24 @@
   return ProcessSP(new ProcessWindows(target_sp, listener_sp));
 }
 
-void ProcessWindows::Initialize() {
-  static llvm::once_flag g_once_flag;
+static bool ShouldUseLLDBServer() {
+  llvm::StringRef use_lldb_server = ::getenv("LLDB_USE_LLDB_SERVER");
+  return use_lldb_server.equals_lower("on") ||
+ use_lldb_server.equals_lower("yes") ||
+ use_lldb_server.equals_lower("1") ||
+ use_lldb_server.equals_lower("true");
+}
 
-  llvm::call_once(g_once_flag, []() {
-PluginManager::RegisterPlugin(GetPluginNameStatic(),
-  GetPluginDescriptionStatic(), CreateInstance);
-  });
+void ProcessWindows::Initialize() {
+  if (!ShouldUseLLDBServer()) {
+static llvm::once_flag g_once_flag;
+
+llvm::call_once(g_once_flag, []() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(),
+CreateInstance);
+});
+  }
 }
 
 void ProcessWindows::Terminate() {}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits