From 6fac0ae687664b2662bea57f709db53b6d08bb3f Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Wed, 24 Jul 2019 17:25:24 +0200 Subject: Fixed a name clash in the 'from_environment_diff' function (#12116) * Added a unit test reproducing the failure in 12085 * Fixed name clash in the 'from_environment_diff' function The bug reported in #12085 stemmed from a name clash among variables, introduced during the refactor in #10753 and not caught by unit tests and reviews. --- lib/spack/spack/test/environment_modifications.py | 8 +++++++- lib/spack/spack/util/environment.py | 22 +++++++++++----------- 2 files changed, 18 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/test/environment_modifications.py b/lib/spack/spack/test/environment_modifications.py index b8bc7d3c34..a57ac74364 100644 --- a/lib/spack/spack/test/environment_modifications.py +++ b/lib/spack/spack/test/environment_modifications.py @@ -399,6 +399,7 @@ def test_sanitize_regex(env, blacklist, whitelist, expected, deleted): assert all(x not in after for x in deleted) +@pytest.mark.regression('12085') @pytest.mark.parametrize('before,after,search_list', [ # Set environment variables ({}, {'FOO': 'foo'}, [environment.SetEnv('FOO', 'foo')]), @@ -420,7 +421,12 @@ def test_sanitize_regex(env, blacklist, whitelist, expected, deleted): ({'FOO_PATH': '/a/path:/b/path'}, {'FOO_PATH': '/c/path:/a/path'}, [ environment.RemovePath('FOO_PATH', '/b/path'), environment.PrependPath('FOO_PATH', '/c/path') - ]) + ]), + # Modify two variables in the same environment + ({'FOO': 'foo', 'BAR': 'bar'}, {'FOO': 'baz', 'BAR': 'baz'}, [ + environment.SetEnv('FOO', 'baz'), + environment.SetEnv('BAR', 'baz'), + ]), ]) def test_from_environment_diff(before, after, search_list): diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index 7ad80a7eab..33f1379c4f 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -607,12 +607,12 @@ class EnvironmentModifications(object): env.unset(x) for x in modified_variables: - before = before[x] - after = after[x] - sep = return_separator_if_any(before, after) + value_before = before[x] + value_after = after[x] + sep = return_separator_if_any(value_before, value_after) if sep: - before_list = before.split(sep) - after_list = after.split(sep) + before_list = value_before.split(sep) + after_list = value_after.split(sep) # Filter out empty strings before_list = list(filter(None, before_list)) @@ -623,8 +623,8 @@ class EnvironmentModifications(object): before_list = list(dedupe(before_list)) after_list = list(dedupe(after_list)) # The reassembled cleaned entries - before = sep.join(before_list) - after = sep.join(after_list) + value_before = sep.join(before_list) + value_after = sep.join(after_list) # Paths that have been removed remove_list = [ @@ -638,12 +638,12 @@ class EnvironmentModifications(object): end = after_list.index(remaining_list[-1]) search = sep.join(after_list[start:end + 1]) except IndexError: - env.prepend_path(x, after) + env.prepend_path(x, value_after) continue - if search not in before: + if search not in value_before: # We just need to set the variable to the new value - env.prepend_path(x, after) + env.prepend_path(x, value_after) else: try: prepend_list = after_list[:start] @@ -663,7 +663,7 @@ class EnvironmentModifications(object): env.prepend_path(x, item) else: # We just need to set the variable to the new value - env.set(x, after) + env.set(x, value_after) return env -- cgit v1.2.3-70-g09d2