1173875 - Allow having a minimal environment

Status: RESOLVED WONTFIX

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Armen [:armenzg]]

Attachment: Allow having a minimal environment

I generally don't like see all the environment variables of my host printed out when running any steps that we use query_venv().

In this patch, we use minimal_env to only use PATH and whatever is explicitely set by the user in the mozharness configs.

This is opt-in.

Some of this patch will land in bug 1173871.

Comment 1

[Aki Sasaki (not active)]

Have you tested on Windows? You may need to set os.environ['SystemRoot'].
https://mail.python.org/pipermail/python-dev/2011-December/114740.html
http://bugs.python.org/issue13524

Comment 2

[Jordan Lund (:jlund)]

Comment on attachment 8621096 [details] [diff] [review]
Allow having a minimal environment

Review of attachment 8621096 [details] [diff] [review]:
-----------------------------------------------------------------

neato!

though, sounds like Aki makes a good point in that this will likely cause spurious results for local runs particularly on windows.

iiuc, your issue from comment 0 is the fact that so many env vars are logged each time. If that's the case, maybe we should instead add some logic in run_command/get_output_from_command to suppress all env vars except for a subset we define in:
self.config['env_log_subset'] = ['PATH', 'foo', 'bar']

r- to clear review. I can change to r+ if we discuss it first and you feel strongly

::: mozharness/base/script.py
@@ +622,5 @@
>  
>          If you specify partial_env, partial_env will be used instead of
>          self.config['env'], and we don't save self.env as it's a one-off.
>  
> +        If you set minimal_env to True, it will only use PATH from the host's

my concern here is that minimal_env will override certain params (partial_env) while others can still override minimal_env (replace_dict, purge_env).

@@ +629,3 @@
>          """
> +        if minimal_env:
> +            partial_env = {}

why do we want to overwrite partial_env here? iiuc, if we pass partial_env and minimal_env, we will use partial_env + os.environ['PATH'] which seems like what we would expect to happen if we passed both no? If you do not wish so, we should probably log when both are passed with a self.warning and also mention the mutually exclusive params for this method

Comment 3

[Armen [:armenzg]]

Thanks to both of you for looking into it.

My main reason is because the logs are noisy and I like your thinking of muting them.

Nevertheless, I believe we're adding another feature to remove an annoyance yet I think it is more useful to be able to ensure the same environment variables for a step regardless of the host environments' env variables.

I'm closing as WONTFIX since it does not bother me enough and we can re-open it if there is someone else interested on fixing this.