[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-24 Thread walter erquinigo via Phabricator via lldb-commits
wallace abandoned this revision. wallace added a comment. With https://reviews.llvm.org/D76470, targets created by lldb-vscode by default are inheriting the debugger's environment. I don't need this change anymore. We can work on providing a flag that disables that default behavior, but that

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D74636#1938283 , @clayborg wrote: > I would either add the option to SBLaunchInfo so it can be specified, or > execute the command. If the target is created, it is setting a target > specific setting. If had to pick I would

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-23 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. I would either add the option to SBLaunchInfo so it can be specified, or execute the command. If the target is created, it is setting a target specific setting. If had to pick I would add the API to SBLaunchInfo. Whenever I see something that can't be done through the

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-23 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment. TBH I'd prefer to have consistency across all these options regardless of how the target is launched. That means modifying the defaults of the target settings that can be specified via launch.json arguments. What do you think, @clayborg? Repository: rG LLVM Github

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D74636#1934466 , @wallace wrote: > I added some tests cases to show why I used "settings set target.inherit-env". > > There are currently two ways to launch a process. Either with the plain > "program" argument, > or with the

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-20 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251821. wallace added a comment. fix test failing with python3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files:

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-20 Thread Phabricator via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG4ec6ebabfc3e: [lldb-vscode] Add inheritEnvironment option (authored by Hector Diaz hd...@fb.com, committed by Walter

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-20 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251766. wallace added a comment. further simplify code. Sorry for the noise Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files:

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-20 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251762. wallace added a comment. simplify code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files:

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-20 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251760. wallace added a comment. test pass now Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files:

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-20 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251754. wallace added a comment. I added some tests cases to show why I used "settings set target.inherit-env". There are currently two ways to launch a process. Either with the plain "program" argument, or with the "launchCommands" argument. The latter is

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-20 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp:8 + +int main(int argc, char const *argv[], char const *envp[]) { + char **env_var_pointer = environ; I guess you don't need the `envp` argument if you're

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-19 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251527. wallace added a comment. Now using the latest SBEnvironment API Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files:

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-19 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1414 +auto kv = llvm::StringRef(name_and_value).split("="); +env.Set(kv.first.str().c_str(), kv.second.str().c_str(), true); + } We want to have a function in

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-19 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251479. wallace added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files:

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-19 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks for updating this patch. This seems ok to me (modulo comments here and the other patch), but I think it would be better to move all of the SB changes into that other patch. (the reason I requested this update was to see whether the other patch has all that's

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251235. wallace added a comment. Using the new SBEnvironment class Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files: lldb/bindings/interface/SBLaunchInfo.i

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251236. wallace added a comment. improve a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files: lldb/bindings/interface/SBLaunchInfo.i

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-12 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment. The original intention of this is to have any working environment, as quite often complex programs require many environment variables that are common to most processes. Having the user specify each of those is a bit too much to ask for, and they are complaining because

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D74636#1918110 , @clayborg wrote: > In D74636#1916356 , @labath wrote: > > > There's a `target.inherit-env` setting in lldb (which I believe also works > > correctly for remote

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-11 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment. Regarding implementation: The target.inherit-env setting is only effectively used by CommandObjectProcess when launching the target from the command line. lldb-vscode is not following the same codepath and not using that property. What about exposing the Platform's

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-11 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D74636#1916356 , @labath wrote: > There's a `target.inherit-env` setting in lldb (which I believe also works > correctly for remote launches). Could you use that instead of reimplementing > the feature in vscode? The real

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-11 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. There's a `target.inherit-env` setting in lldb (which I believe also works correctly for remote launches). Could you use that instead of reimplementing the feature in vscode? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-10 Thread Héctor Luis Díaz Aceves via Phabricator via lldb-commits
diazhector98 updated this revision to Diff 249536. diazhector98 added a comment. Adding remote process check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files:

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-10 Thread walter erquinigo via Phabricator via lldb-commits
wallace requested changes to this revision. wallace added inline comments. This revision now requires changes to proceed. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1380 auto envs = GetStrings(arguments, "env"); + if (launchWithDebuggerEnvironment) { +std::vector

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-10 Thread Héctor Luis Díaz Aceves via Phabricator via lldb-commits
diazhector98 updated this revision to Diff 249527. diazhector98 added a comment. Adding support for windows systems Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74636/new/ https://reviews.llvm.org/D74636 Files:

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-02-19 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close, just need to check for the "host" platform and possibly add windows support, or return an error on windows, if LLVM doesn't have a OS abstracted wrapper that allows