[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

In D76009#1919103 <https://reviews.llvm.org/D76009#1919103>, @labath wrote:

> Actually, hang on.
>
> > One existing test had to be modified, because the initialization of
> >  the environment properties now take place at the time the target is
> >  created, not at the first use of the environment (usually launch
> >  time).


Just to be clear, in that test the host environment was modified between the creation of the target and the launch which I think is pretty unlikely to matter in practice.

> Does this mean that the target-local value of the `target.inherit-env` setting no longer has any effect? Currently I can set it after creating a target (but before launching) to stop the process inheriting the default environment:
> 
>   (lldb) file /usr/bin/env
>   Current executable set to '/usr/bin/env' (x86_64).
>   (lldb) set set target.inherit-env false
>   (lldb) pr la
>   Process 26684 launched: '/usr/bin/env' (x86_64)
>   Process 26684 exited with status = 0 (0x00000000) 
> 
> 
> I take it that after this, that will no longer be possible? It would still be possible to do that by setting the global value of the setting before creating a target, but the target-local setting would be useless (?)

The inherited environment is collected the first time the property is accessed and running the callback triggers this. For some reason just doing `set show target.env-vars` doesn't trigger it though, I'd need to debug this to understand. So yes, with this change, the `target.inherit-env` setting will take effect at target creation time and not at whatever next point in time the lazy logic is triggered.

One way to fix this is to add another callback for the `target.inherit-env` property and remove the variables that are in the environment.

> I'm not really sure how these settings are supposed to work, but this does not seem ideal to me.

Jim would certainly be able to explain the intent here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76009