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



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

Reply via email to