Recipe engine bug: unexpected failure - Percentage in environment fails in prod but not in testing |
|||||
Issue descriptionhttps://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/467070 --------------------------------------------- Traceback (most recent call last): File "/b/rr/tmpp0nlQx/rw/checkout/scripts/slave/.recipe_deps/recipe_engine/recipe_engine/step_runner.py", line 181, in open_step step_env = _merge_envs(os.environ, (rendered_step.config.env or {})) File "/b/rr/tmpp0nlQx/rw/checkout/scripts/slave/.recipe_deps/recipe_engine/recipe_engine/step_runner.py", line 670, in _merge_envs result[str(k)] = str(v) % original TypeError: float argument required, not instance --------------------------------------------- This was caused by https://chromium-review.googlesource.com/c/517847 which did a; ninja_env['NINJA_STATUS'] = ( '%e [%s/%t %p - %r running, current %c p/s, overall %o p/s] ') This passed the training / testing - but failed on production. This is caused by _merge_env in step_runner.py; --------------------------------------------- def _merge_envs(original, override): """Merges two environments. Returns a new environment dict with entries from |override| overwriting corresponding entries in |original|. Keys whose value is None will completely remove the environment variable. Values can contain %(KEY)s strings, which will be substituted with the values from the original (useful for amending, as opposed to overwriting, variables like PATH). """ result = original.copy() if not override: return result for k, v in override.items(): if v is None: if k in result: del result[k] else: result[str(k)] = str(v) % original return result ---------------------------------------------
,
May 31 2017
,
May 31 2017
As mentioned in chat, but for posterity: _merge_env is use to merge the recipe's concept of the environment with the real OS's environment. It %-formats the env value with the current environ to allow things like `%(PATH)s` to expand inside the environment values, as documented here: https://github.com/luci/recipes-py/blob/master/recipe_modules/context/api.py#L79 We could add some checks to the context module to ensure that the value is %-formattable in this way (maybe by passing in some fake dictionary-like object that is able to supply empty values for any conceivable key). That said, I'd like to get rid of this %-format hack. The only thing it's really used for is manipulating $PATH and $PYTHONPATH; they would be better served by having explicit support for path-like environment variables.
,
May 31 2017
,
May 31 2017
I added some error checks and tests and such: https://codereview.chromium.org/2913203002
,
Jun 1 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by tansell@chromium.org
, May 30 2017