labath added a comment.

In D76105#1920753 <https://reviews.llvm.org/D76105#1920753>, @friss wrote:

> Yeah, this shortcoming was pretty obvious while writing the patch. I don't 
> like it very much, it seems like the inherit behavior should be handled 
> closer to the point we launch, Or at least the inherited environment should 
> be stored separately from the one you set manually. Comparing values would 
> solve one class of issues, but what about explicitly setting one variable to 
> the same value it has in your environment. Then you would delete it when 
> changing the inherit-env property. Also not very intuitive.


Yeah, I've been thinking about that too. Is it now too late to do anything 
about that?

In my ideal world the "env-vars" setting would only hold the values that the 
user has set, and nothing more. And there would be a separate way to get the 
effective enviroment used for launches and what not. Maybe something like:

- `platform environment` => get the current platform's environment -- this is 
the thing that would get inherited
- `process environment` => get the effective environment -- if a process is 
running it's the environment the process was launched with, if it's not yet 
started, it's the (merged) environment it would be launched with. Or maybe 
these should be separate commands?

We wouldn't have to add all of these commands, if we don't think they are 
useful. My main point is about avoiding merging user-provided and inherited 
variables so early in the process..

One gotcha with this flow is that it becomes hard to unset a specific 
environment variable but inherit the rest. But maybe that doesn't matter and 
setting the variable to an empty string is enough? Or we can have a "target 
populate-environment" command which would fill in the host environment into the 
"target.env-vars" setting and set "inherit-env=false" (I've seen this kind of 
flow when launching processes from visual studio (through a gui), and I kind of 
liked it)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76105



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

Reply via email to