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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D74636#1918110 <https://reviews.llvm.org/D74636#1918110>, @clayborg wrote:

> In D74636#1916356 <https://reviews.llvm.org/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 question is if we want launching to fail when the user tries to enable this for remote targets? If we don't care if it fails, then we can use "target.inherit-env", but I kind of like that it does fail as it clearly conveys to the user that their environment won't be passed along instead of just ignoring the request. Thoughts?


The real question for me is what do we expect this setting to do. If the intention is to pass the host environment, then that doesn't generally make sense for remote targets, and failing would be good. But if we just want to ensure it inherits "a suitable" environment, then there's no reason to fail. I believe this is what `target.inherit-env` will do for remote launches, where it will take the environment from the remote platform, and "inherit" that. I think it would make sense if this setting and `target.inherit-env` behaved the same way...

In D74636#1918682 <https://reviews.llvm.org/D74636#1918682>, @wallace wrote:

> 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 environment in the API and merging SBPlatform->GetEnvironment() with the launch.json's environment if inheritEnvironment is True? That would be very similar to what is doing CommandObjectProcess


SBPlatform::GetEnvironment sounds perfectly reasonable to me.

I am also wondering if D76009 <https://reviews.llvm.org/D76009> is relevant in any way here...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636