env modifications : added a validation rule

2 files changed, 33 insertions, 3 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index c51fa58477..59b234624c 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -34,8 +34,9 @@ import shutil
 import sys
 
 import spack
+import llnl.util.tty as tty
 from llnl.util.filesystem import *
-from spack.environment import EnvironmentModifications, concatenate_paths
+from spack.environment import EnvironmentModifications, concatenate_paths, validate
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -288,8 +289,7 @@ def setup_package(pkg):
     # Allow dependencies to set up environment as well
     for dependency_spec in pkg.spec.traverse(root=False):
         dependency_spec.package.setup_dependent_environment(env, pkg.spec)
-    # TODO : implement validation
-    #validate(env)
+    validate(env, tty.warn)
     env.apply_modifications()
 
 
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 6d214595a3..74aef57fe8 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -202,3 +202,33 @@ def concatenate_paths(paths):
         string
     """
     return ':'.join(str(item) for item in paths)
+
+
+def set_or_unset_not_first(variable, changes, errstream):
+    """
+    Check if we are going to set or unset something after other modifications have already been requested
+    """
+    indexes = [ii for ii, item in enumerate(changes) if ii != 0 and type(item) in [SetEnv, UnsetEnv]]
+    if indexes:
+        good = '\t    \t{context} at {filename}:{lineno}'
+        nogood = '\t--->\t{context} at {filename}:{lineno}'
+        errstream('Suspicious requests to set or unset the variable \'{var}\' found'.format(var=variable))
+        for ii, item in enumerate(changes):
+            print_format = nogood if ii in indexes else good
+            errstream(print_format.format(**item.args))
+
+
+def validate(env, errstream):
+    """
+    Validates the environment modifications to check for the presence of suspicious patterns. Prompts a warning for
+    everything that was found
+
+    Current checks:
+    - set or unset variables after other changes on the same variable
+
+    Args:
+        env: list of environment modifications
+    """
+    modifications = env.group_by_name()
+    for variable, list_of_changes in sorted(modifications.items()):
+        set_or_unset_not_first(variable, list_of_changes, errstream)