[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

In D76105#1920633 <https://reviews.llvm.org/D76105#1920633>, @jingham wrote:

> If I'm following the logic correctly, if you run a target with inherit-env off, and then do:
>
>   env VAR_IN_ENVIRONMENT=NOT_THE_ENVIRONMENTS_VALUE
>  
>
>
> then turn inherit-env on, we will preserve the value you set it to, not the environment value, because you pass in false for can_replace.  That seems right to me.  It would be good to test that case explicitly, however.
>
> But if you have inherit-env on and then run the command above, and then turn inherit-env off, your changed value will just get deleted.  That seems unexpected.  I think you have to compare the values in the else branch and only delete the key if the value is the same as the environment value.  Does that sound right?


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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76105