1182271 - Allow to append env variables from various config files + avoid env variables from host

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Armen [:armenzg]]

Attachment: env.diff

It makes my life so much easier.

Comment 1

[Armen [:armenzg]]

Hi jlund,
If you could review this in the morning it would be great so I can land it before EOD.

This is a release type of job, can I assume that I should land both on mozharness (which would be used) and gecko (to keep in sync)?

Comment 2

[Jordan Lund (:jlund)]

Comment on attachment 8631792 [details] [diff] [review]
env.diff

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

::: mozharness/base/config.py
@@ +485,5 @@
> +                # We only append values from various configs for the 'env' entry
> +                # For everything else we follow the standard behaviour
> +                for i, (c_file, c_dict) in enumerate(self.all_cfg_files_and_dicts):
> +                    for v in c_dict.keys():
> +                        if v == 'env' and v in config:

oh, I didn't realize you wanted to just merge env.

my concern is that it may not be obvious this is a special snowflake. I wonder if we should have a condition that says whether we should merge all nested config items (including env) or not at all (like it is currently)

::: mozharness/base/script.py
@@ +894,5 @@
>                  partial_env = {}
>              if set_self_env is None:
>                  set_self_env = True
> +
> +        env = {'PATH': os.environ['PATH']} if avoid_host_env else os.environ.copy()

neat! I bet this will cause issues on certain platforms (windows likely) that require more than just that env. That is unless you specify all of those in the config

Comment 3

[Jordan Lund (:jlund)]

> This is a release type of job, can I assume that I should land both on
> mozharness (which would be used) and gecko (to keep in sync)?

what branches does this need to be patched for immediately? I would still prefer to land patches only on hgmo/build/mozharness as that will make syncing easier. That is until we work out all the kinks with gecko mozharness

Comment 4

[Armen [:armenzg]]

Comment on attachment 8631792 [details] [diff] [review]
env.diff

r+ from IRC.

Comment 5

[Armen [:armenzg]]

It did not get backed out and its on production.