Deprecate blacklist/whitelist in favor of include/exclude (#31569)

For a long time the module configuration has had a few settings that use
`blacklist`/`whitelist` terminology. We've been asked by some of our users to replace
this with more inclusive language. In addition to being non-inclusive, `blacklist` and
`whitelist` are inconsistent with the rest of Spack, which uses `include` and `exclude`
for the same concepts.

- [x] Deprecate `blacklist`, `whitelist`, `blacklist_implicits` and `environment_blacklist`
      in favor of `exclude`, `include`, `exclude_implicits` and `exclude_env_vars` in module
      configuration, to be removed in Spack v0.20.
- [x] Print deprecation warnings if any of the deprecated names are in module config.
- [x] Update tests to test old and new names.
- [x] Update docs.
- [x] Update `spack config update` to fix this automatically, and include a note in the error
      that you can use this command.

lib/spack/docs/module_file_support.rst

@@ -308,7 +308,7 @@ the variable ``FOOBAR`` will be unset.
      spec constraints are instead evaluated top to bottom.
 
 """"""""""""""""""""""""""""""""""""""""""""
-Blacklist or whitelist specific module files
+Exclude or include specific module files
 """"""""""""""""""""""""""""""""""""""""""""
 
 You can use anonymous specs also to prevent module files from being written or
@@ -322,8 +322,8 @@ your system. If you write a configuration file like:
    modules:
      default:
        tcl:
-         whitelist: ['gcc', 'llvm']  # Whitelist will have precedence over blacklist
+         include: ['gcc', 'llvm']  # include will have precedence over exclude
-         blacklist: ['%gcc@4.4.7']   # Assuming gcc@4.4.7 is the system compiler
+         exclude: ['%gcc@4.4.7']   # Assuming gcc@4.4.7 is the system compiler
 
 you will prevent the generation of module files for any package that
 is compiled with ``gcc@4.4.7``, with the only exception of any ``gcc``
@@ -490,7 +490,7 @@ satisfies a default, Spack will generate the module file in the
 appropriate path, and will generate a default symlink to the module
 file as well.
 
-.. warning:: 
+.. warning::
   If Spack is configured to generate multiple default packages in the
   same directory, the last modulefile to be generated will be the
   default module.
@@ -589,7 +589,7 @@ Filter out environment modifications
 Modifications to certain environment variables in module files are there by
 default, for instance because they are generated by prefix inspections.
 If you want to prevent modifications to some environment variables, you can
-do so by using the environment blacklist:
+do so by using the ``exclude_env_vars``:
 
 .. code-block:: yaml
 
@@ -599,7 +599,7 @@ do so by using the environment blacklist:
          all:
            filter:
              # Exclude changes to any of these variables
-             environment_blacklist: ['CPATH', 'LIBRARY_PATH']
+             exclude_env_vars: ['CPATH', 'LIBRARY_PATH']
 
 The configuration above will generate module files that will not contain
 modifications to either ``CPATH`` or ``LIBRARY_PATH``.

lib/spack/spack/binary_distribution.py

@@ -618,7 +618,7 @@ def get_buildfile_manifest(spec):
     Return a data structure with information about a build, including
     text_to_relocate, binary_to_relocate, binary_to_relocate_fullpath
     link_to_relocate, and other, which means it doesn't fit any of previous
-    checks (and should not be relocated). We blacklist docs (man) and
+    checks (and should not be relocated). We exclude docs (man) and
     metadata (.spack). This can be used to find a particular kind of file
     in spack, or to generate the build metadata.
     """
@@ -626,12 +626,12 @@ def get_buildfile_manifest(spec):
             "link_to_relocate": [], "other": [],
             "binary_to_relocate_fullpath": []}
 
-    blacklist = (".spack", "man")
+    exclude_list = (".spack", "man")
 
     # Do this at during tarball creation to save time when tarball unpacked.
     # Used by make_package_relative to determine binaries to change.
     for root, dirs, files in os.walk(spec.prefix, topdown=True):
-        dirs[:] = [d for d in dirs if d not in blacklist]
+        dirs[:] = [d for d in dirs if d not in exclude_list]
 
         # Directories may need to be relocated too.
         for directory in dirs:

lib/spack/spack/cmd/edit.py

@@ -104,9 +104,9 @@ def edit(parser, args):
             path = os.path.join(path, name)
             if not os.path.exists(path):
                 files = glob.glob(path + '*')
-                blacklist = ['.pyc', '~']  # blacklist binaries and backups
+                exclude_list = ['.pyc', '~']  # exclude binaries and backups
                 files = list(filter(
-                    lambda x: all(s not in x for s in blacklist), files))
+                    lambda x: all(s not in x for s in exclude_list), files))
                 if len(files) > 1:
                     m = 'Multiple files exist with the name {0}.'.format(name)
                     m += ' Please specify a suffix. Files are:\n\n'

lib/spack/spack/cmd/modules/__init__.py

@@ -131,7 +131,7 @@ def check_module_set_name(name):
 
 _missing_modules_warning = (
     "Modules have been omitted for one or more specs, either"
-    " because they were blacklisted or because the spec is"
+    " because they were excluded or because the spec is"
     " associated with a package that is installed upstream and"
     " that installation has not generated a module file. Rerun"
     " this command with debug output enabled for more details.")
@@ -180,7 +180,7 @@ def loads(module_type, specs, args, out=None):
     for spec, mod in modules:
         if not mod:
             module_output_for_spec = (
-                '## blacklisted or missing from upstream: {0}'.format(
+                '## excluded or missing from upstream: {0}'.format(
                     spec.format()))
         else:
             d['exclude'] = '## ' if spec.name in exclude_set else ''
@@ -293,8 +293,8 @@ def refresh(module_type, specs, args):
         cls(spec, args.module_set_name) for spec in specs
         if spack.repo.path.exists(spec.name)]
 
-    # Filter blacklisted packages early
+    # Filter excluded packages early
-    writers = [x for x in writers if not x.conf.blacklisted]
+    writers = [x for x in writers if not x.conf.excluded]
 
     # Detect name clashes in module files
     file2writer = collections.defaultdict(list)

lib/spack/spack/modules/common.py

@@ -54,6 +54,34 @@
 import spack.util.spack_yaml as syaml
 
 
+def get_deprecated(dictionary, name, old_name, default):
+    """Get a deprecated property from a ``dict``.
+
+    Arguments:
+        dictionary (dict): dictionary to get a value from.
+        name (str): New name for the property. If present, supersedes ``old_name``.
+        old_name (str): Deprecated name for the property. If present, a warning
+            is printed.
+        default (object): value to return if neither name is found.
+    """
+    value = default
+
+    # always warn if old name is present
+    if old_name in dictionary:
+        value = dictionary.get(old_name, value)
+        main_msg = "`{}:` is deprecated in module config and will be removed in v0.20."
+        details = (
+            "Use `{}:` instead. You can run `spack config update` to translate your "
+            "configuration files automatically."
+        )
+        tty.warn(main_msg.format(old_name), details.format(name))
+
+    # name overrides old name if present
+    value = dictionary.get(name, value)
+
+    return value
+
+
 #: config section for this file
 def configuration(module_set_name):
     config_path = 'modules:%s' % module_set_name
@@ -351,14 +379,14 @@ def get_module(
 
     Retrieve the module file for the given spec if it is available. If the
     module is not available, this will raise an exception unless the module
-    is blacklisted or if the spec is installed upstream.
+    is excluded or if the spec is installed upstream.
 
     Args:
         module_type: the type of module we want to retrieve (e.g. lmod)
         spec: refers to the installed package that we want to retrieve a module
             for
-        required: if the module is required but blacklisted, this function will
+        required: if the module is required but excluded, this function will
-            print a debug message. If a module is missing but not blacklisted,
+            print a debug message. If a module is missing but not excluded,
             then an exception is raised (regardless of whether it is required)
         get_full_path: if ``True``, this returns the full path to the module.
             Otherwise, this returns the module name.
@@ -386,13 +414,13 @@ def get_module(
     else:
         writer = spack.modules.module_types[module_type](spec, module_set_name)
         if not os.path.isfile(writer.layout.filename):
-            if not writer.conf.blacklisted:
+            if not writer.conf.excluded:
                 err_msg = "No module available for package {0} at {1}".format(
                     spec, writer.layout.filename
                 )
                 raise ModuleNotFoundError(err_msg)
             elif required:
-                tty.debug("The module configuration has blacklisted {0}: "
+                tty.debug("The module configuration has excluded {0}: "
                           "omitting it".format(spec))
             else:
                 return None
@@ -483,26 +511,30 @@ def hash(self):
         return None
 
     @property
-    def blacklisted(self):
+    def excluded(self):
-        """Returns True if the module has been blacklisted,
+        """Returns True if the module has been excluded, False otherwise."""
-        False otherwise.
+
-        """
         # A few variables for convenience of writing the method
         spec = self.spec
         conf = self.module.configuration(self.name)
 
-        # Compute the list of whitelist rules that match
+        # Compute the list of include rules that match
-        wlrules = conf.get('whitelist', [])
+        # DEPRECATED: remove 'whitelist' in v0.20
-        whitelist_matches = [x for x in wlrules if spec.satisfies(x)]
+        include_rules = get_deprecated(conf, "include", "whitelist", [])
+        include_matches = [x for x in include_rules if spec.satisfies(x)]
 
-        # Compute the list of blacklist rules that match
+        # Compute the list of exclude rules that match
-        blrules = conf.get('blacklist', [])
+        # DEPRECATED: remove 'blacklist' in v0.20
-        blacklist_matches = [x for x in blrules if spec.satisfies(x)]
+        exclude_rules = get_deprecated(conf, "exclude", "blacklist", [])
+        exclude_matches = [x for x in exclude_rules if spec.satisfies(x)]
 
-        # Should I blacklist the module because it's implicit?
+        # Should I exclude the module because it's implicit?
-        blacklist_implicits = conf.get('blacklist_implicits')
+        # DEPRECATED: remove 'blacklist_implicits' in v0.20
+        exclude_implicits = get_deprecated(
+            conf, "exclude_implicits", "blacklist_implicits", None
+        )
         installed_implicitly = not spec._installed_explicitly()
-        blacklisted_as_implicit = blacklist_implicits and installed_implicitly
+        excluded_as_implicit = exclude_implicits and installed_implicitly
 
         def debug_info(line_header, match_list):
             if match_list:
@@ -511,15 +543,15 @@ def debug_info(line_header, match_list):
                 for rule in match_list:
                     tty.debug('\t\tmatches rule: {0}'.format(rule))
 
-        debug_info('WHITELIST', whitelist_matches)
+        debug_info('INCLUDE', include_matches)
-        debug_info('BLACKLIST', blacklist_matches)
+        debug_info('EXCLUDE', exclude_matches)
 
-        if blacklisted_as_implicit:
+        if excluded_as_implicit:
-            msg = '\tBLACKLISTED_AS_IMPLICIT : {0}'.format(spec.cshort_spec)
+            msg = '\tEXCLUDED_AS_IMPLICIT : {0}'.format(spec.cshort_spec)
             tty.debug(msg)
 
-        is_blacklisted = blacklist_matches or blacklisted_as_implicit
+        is_excluded = exclude_matches or excluded_as_implicit
-        if not whitelist_matches and is_blacklisted:
+        if not include_matches and is_excluded:
             return True
 
         return False
@@ -544,17 +576,22 @@ def specs_to_prereq(self):
         return self._create_list_for('prerequisites')
 
     @property
-    def environment_blacklist(self):
+    def exclude_env_vars(self):
         """List of variables that should be left unmodified."""
-        return self.conf.get('filter', {}).get('environment_blacklist', {})
+        filter = self.conf.get('filter', {})
+
+        # DEPRECATED: remove in v0.20
+        return get_deprecated(
+            filter, "exclude_env_vars", "environment_blacklist", {}
+        )
 
     def _create_list_for(self, what):
-        whitelist = []
+        include = []
         for item in self.conf[what]:
             conf = type(self)(item, self.name)
-            if not conf.blacklisted:
+            if not conf.excluded:
-                whitelist.append(item)
+                include.append(item)
-        return whitelist
+        return include
 
     @property
     def verbose(self):
@@ -733,8 +770,8 @@ def environment_modifications(self):
         # Modifications required from modules.yaml
         env.extend(self.conf.env)
 
-        # List of variables that are blacklisted in modules.yaml
+        # List of variables that are excluded in modules.yaml
-        blacklist = self.conf.environment_blacklist
+        exclude = self.conf.exclude_env_vars
 
         # We may have tokens to substitute in environment commands
 
@@ -758,7 +795,7 @@ def environment_modifications(self):
                 pass
             x.name = str(x.name).replace('-', '_')
 
-        return [(type(x).__name__, x) for x in env if x.name not in blacklist]
+        return [(type(x).__name__, x) for x in env if x.name not in exclude]
 
     @tengine.context_property
     def autoload(self):
@@ -831,9 +868,9 @@ def write(self, overwrite=False):
                 existing file. If False the operation is skipped an we print
                 a warning to the user.
         """
-        # Return immediately if the module is blacklisted
+        # Return immediately if the module is excluded
-        if self.conf.blacklisted:
+        if self.conf.excluded:
-            msg = '\tNOT WRITING: {0} [BLACKLISTED]'
+            msg = '\tNOT WRITING: {0} [EXCLUDED]'
             tty.debug(msg.format(self.spec.cshort_spec))
             return
 

lib/spack/spack/schema/modules.py

@@ -18,9 +18,13 @@
 #:
 #: THIS NEEDS TO BE UPDATED FOR EVERY NEW KEYWORD THAT
 #: IS ADDED IMMEDIATELY BELOW THE MODULE TYPE ATTRIBUTE
-spec_regex = r'(?!hierarchy|core_specs|verbose|hash_length|whitelist|' \
+spec_regex = (
-             r'blacklist|projections|naming_scheme|core_compilers|all|' \
+    r'(?!hierarchy|core_specs|verbose|hash_length|defaults|'
-             r'defaults)(^\w[\w-]*)'
+    r'whitelist|blacklist|'  # DEPRECATED: remove in 0.20.
+    r'include|exclude|'      # use these more inclusive/consistent options
+    r'projections|naming_scheme|core_compilers|all)(^\w[\w-]*)'
+
+)
 
 #: Matches a valid name for a module set
 valid_module_set_name = r'^(?!arch_folder$|lmod$|roots$|enable$|prefix_inspections$|'\
@@ -50,12 +54,21 @@
             'default': {},
             'additionalProperties': False,
             'properties': {
+                # DEPRECATED: remove in 0.20.
                 'environment_blacklist': {
                     'type': 'array',
                     'default': [],
                     'items': {
                         'type': 'string'
                     }
+                },
+                # use exclude_env_vars instead
+                'exclude_env_vars': {
+                    'type': 'array',
+                    'default': [],
+                    'items': {
+                        'type': 'string'
+                    }
                 }
             }
         },
@@ -95,12 +108,20 @@
                 'minimum': 0,
                 'default': 7
             },
+            # DEPRECATED: remove in 0.20.
             'whitelist': array_of_strings,
             'blacklist': array_of_strings,
             'blacklist_implicits': {
                 'type': 'boolean',
                 'default': False
             },
+            # whitelist/blacklist have been replaced with include/exclude
+            'include': array_of_strings,
+            'exclude': array_of_strings,
+            'exclude_implicits': {
+                'type': 'boolean',
+                'default': False
+            },
             'defaults': array_of_strings,
             'naming_scheme': {
                 'type': 'string'  # Can we be more specific here?
@@ -224,14 +245,51 @@ def deprecation_msg_default_module_set(instance, props):
 }
 
 
-def update(data):
+# deprecated keys and their replacements
-    """Update the data in place to remove deprecated properties.
+exclude_include_translations = {
+    "whitelist": "include",
+    "blacklist": "exclude",
+    "blacklist_implicits": "exclude_implicits",
+    "environment_blacklist": "exclude_env_vars",
+}
 
-    Args:
-        data (dict): dictionary to be updated
 
-    Returns:
+def update_keys(data, key_translations):
-        True if data was changed, False otherwise
+    """Change blacklist/whitelist to exclude/include.
+
+    Arguments:
+        data (dict): data from a valid modules configuration.
+        key_translations (dict): A dictionary of keys to translate to
+            their respective values.
+
+    Return:
+        (bool) whether anything was changed in data
+    """
+    changed = False
+
+    if isinstance(data, dict):
+        keys = list(data.keys())
+        for key in keys:
+            value = data[key]
+
+            translation = key_translations.get(key)
+            if translation:
+                data[translation] = data.pop(key)
+                changed = True
+
+            changed |= update_keys(value, key_translations)
+
+    elif isinstance(data, list):
+        for elt in data:
+            changed |= update_keys(elt, key_translations)
+
+    return changed
+
+
+def update_default_module_set(data):
+    """Update module configuration to move top-level keys inside default module set.
+
+    This change was introduced in v0.18 (see 99083f1706 or #28659).
     """
     changed = False
 
@@ -258,3 +316,21 @@ def update(data):
         data['default'] = default
 
     return changed
+
+
+def update(data):
+    """Update the data in place to remove deprecated properties.
+
+    Args:
+        data (dict): dictionary to be updated
+
+    Returns:
+        True if data was changed, False otherwise
+    """
+    # deprecated top-level module config (everything in default module set)
+    changed = update_default_module_set(data)
+
+    # translate blacklist/whitelist to exclude/include
+    changed |= update_keys(data, exclude_include_translations)
+
+    return changed

lib/spack/spack/test/cmd/module.py

@@ -149,16 +149,20 @@ def test_find_recursive():
 
 
 @pytest.mark.db
-def test_find_recursive_blacklisted(database, module_configuration):
+# DEPRECATED: remove blacklist in v0.20
-    module_configuration('blacklist')
+@pytest.mark.parametrize("config_name", ["exclude", "blacklist"])
+def test_find_recursive_excluded(database, module_configuration, config_name):
+    module_configuration(config_name)
 
     module('lmod', 'refresh', '-y', '--delete-tree')
     module('lmod', 'find', '-r', 'mpileaks ^mpich')
 
 
 @pytest.mark.db
-def test_loads_recursive_blacklisted(database, module_configuration):
+# DEPRECATED: remove blacklist in v0.20
-    module_configuration('blacklist')
+@pytest.mark.parametrize("config_name", ["exclude", "blacklist"])
+def test_loads_recursive_excluded(database, module_configuration, config_name):
+    module_configuration(config_name)
 
     module('lmod', 'refresh', '-y', '--delete-tree')
     output = module('lmod', 'loads', '-r', 'mpileaks ^mpich')
@@ -166,7 +170,7 @@ def test_loads_recursive_blacklisted(database, module_configuration):
 
     assert any(re.match(r'[^#]*module load.*mpileaks', ln) for ln in lines)
     assert not any(re.match(r'[^#]module load.*callpath', ln) for ln in lines)
-    assert any(re.match(r'## blacklisted or missing.*callpath', ln)
+    assert any(re.match(r'## excluded or missing.*callpath', ln)
                for ln in lines)
 
     # TODO: currently there is no way to separate stdout and stderr when

lib/spack/spack/test/conftest.py

@@ -1003,7 +1003,7 @@ def __call__(self, filename):
 
 
 @pytest.fixture()
-def module_configuration(monkeypatch, request):
+def module_configuration(monkeypatch, request, mutable_config):
     """Reads the module configuration file from the mock ones prepared
     for tests and monkeypatches the right classes to hook it in.
     """
@@ -1018,6 +1018,8 @@ def module_configuration(monkeypatch, request):
         spack.paths.test_path, 'data', 'modules', writer_key
     )
 
+    # ConfigUpdate, when called, will modify configuration, so we need to use
+    # the mutable_config fixture
     return ConfigUpdate(root_for_conf, writer_mod, writer_key, monkeypatch)
 
 

lib/spack/spack/test/data/modules/lmod/alter_environment.yaml

@@ -10,7 +10,7 @@ lmod:
   all:
     autoload: none
     filter:
-      environment_blacklist:
+      exclude_env_vars:
         - CMAKE_PREFIX_PATH
     environment:
       set:

lib/spack/spack/test/data/modules/lmod/blacklist.yaml

@@ -1,3 +1,5 @@
+# DEPRECATED: remove this in v0.20
+# See `exclude.yaml` for the new syntax
 enable:
   - lmod
 lmod:

lib/spack/spack/test/data/modules/lmod/blacklist_environment.yaml

@@ -0,0 +1,30 @@
+# DEPRECATED: remove this in v0.20
+# See `alter_environment.yaml` for the new syntax
+enable:
+  - lmod
+lmod:
+  core_compilers:
+    - 'clang@3.3'
+
+  hierarchy:
+    - mpi
+
+  all:
+    autoload: none
+    filter:
+      environment_blacklist:
+        - CMAKE_PREFIX_PATH
+    environment:
+      set:
+        '{name}_ROOT': '{prefix}'
+
+  'platform=test target=x86_64':
+    environment:
+      set:
+        FOO: 'foo'
+      unset:
+        - BAR
+
+  'platform=test target=core2':
+    load:
+      - 'foo/bar'

lib/spack/spack/test/data/modules/lmod/exclude.yaml

@@ -0,0 +1,12 @@
+enable:
+  - lmod
+lmod:
+  core_compilers:
+    - 'clang@3.3'
+  hierarchy:
+    - mpi
+  exclude:
+    - callpath
+
+  all:
+    autoload: direct

lib/spack/spack/test/data/modules/tcl/alter_environment.yaml

@@ -4,7 +4,7 @@ tcl:
   all:
     autoload: none
     filter:
-      environment_blacklist:
+      exclude_env_vars:
         - CMAKE_PREFIX_PATH
     environment:
       set:

lib/spack/spack/test/data/modules/tcl/blacklist.yaml

@@ -1,3 +1,5 @@
+# DEPRECATED: remove this in v0.20
+# See `exclude.yaml` for the new syntax
 enable:
   - tcl
 tcl:

lib/spack/spack/test/data/modules/tcl/blacklist_environment.yaml

@@ -0,0 +1,25 @@
+# DEPRECATED: remove this in v0.20
+# See `alter_environment.yaml` for the new syntax
+enable:
+  - tcl
+tcl:
+  all:
+    autoload: none
+    filter:
+      environment_blacklist:
+        - CMAKE_PREFIX_PATH
+    environment:
+      set:
+        '{name}_ROOT': '{prefix}'
+
+  'platform=test target=x86_64':
+    environment:
+      set:
+        FOO: 'foo'
+        OMPI_MCA_mpi_leave_pinned: '1'
+      unset:
+        - BAR
+
+  'platform=test target=core2':
+    load:
+      - 'foo/bar'

lib/spack/spack/test/data/modules/tcl/blacklist_implicits.yaml

@@ -1,3 +1,5 @@
+# DEPRECATED: remove this in v0.20
+# See `exclude_implicits.yaml` for the new syntax
 enable:
   - tcl
 tcl:

lib/spack/spack/test/data/modules/tcl/exclude.yaml

@@ -0,0 +1,10 @@
+enable:
+  - tcl
+tcl:
+  include:
+    - zmpi
+  exclude:
+    - callpath
+    - mpi
+  all:
+    autoload: direct

lib/spack/spack/test/data/modules/tcl/exclude_implicits.yaml

@@ -0,0 +1,6 @@
+enable:
+  - tcl
+tcl:
+  exclude_implicits: true
+  all:
+    autoload: direct

lib/spack/spack/test/environment_modifications.py

@@ -379,40 +379,40 @@ def test_clear(env):
     assert len(env) == 0
 
 
-@pytest.mark.parametrize('env,blacklist,whitelist', [
+@pytest.mark.parametrize('env,exclude,include', [
-    # Check we can blacklist a literal
+    # Check we can exclude a literal
     ({'SHLVL': '1'}, ['SHLVL'], []),
-    # Check whitelist takes precedence
+    # Check include takes precedence
     ({'SHLVL': '1'}, ['SHLVL'], ['SHLVL']),
 ])
-def test_sanitize_literals(env, blacklist, whitelist):
+def test_sanitize_literals(env, exclude, include):
 
-    after = environment.sanitize(env, blacklist, whitelist)
+    after = environment.sanitize(env, exclude, include)
 
-    # Check that all the whitelisted variables are there
+    # Check that all the included variables are there
-    assert all(x in after for x in whitelist)
+    assert all(x in after for x in include)
 
-    # Check that the blacklisted variables that are not
+    # Check that the excluded variables that are not
-    # whitelisted are there
+    # included are there
-    blacklist = list(set(blacklist) - set(whitelist))
+    exclude = list(set(exclude) - set(include))
-    assert all(x not in after for x in blacklist)
+    assert all(x not in after for x in exclude)
 
 
-@pytest.mark.parametrize('env,blacklist,whitelist,expected,deleted', [
+@pytest.mark.parametrize('env,exclude,include,expected,deleted', [
-    # Check we can blacklist using a regex
+    # Check we can exclude using a regex
     ({'SHLVL': '1'}, ['SH.*'], [], [], ['SHLVL']),
-    # Check we can whitelist using a regex
+    # Check we can include using a regex
     ({'SHLVL': '1'}, ['SH.*'], ['SH.*'], ['SHLVL'], []),
-    # Check regex to blacklist Modules v4 related vars
+    # Check regex to exclude Modules v4 related vars
     ({'MODULES_LMALTNAME': '1', 'MODULES_LMCONFLICT': '2'},
      ['MODULES_(.*)'], [], [], ['MODULES_LMALTNAME', 'MODULES_LMCONFLICT']),
     ({'A_modquar': '1', 'b_modquar': '2', 'C_modshare': '3'},
      [r'(\w*)_mod(quar|share)'], [], [],
      ['A_modquar', 'b_modquar', 'C_modshare']),
 ])
-def test_sanitize_regex(env, blacklist, whitelist, expected, deleted):
+def test_sanitize_regex(env, exclude, include, expected, deleted):
 
-    after = environment.sanitize(env, blacklist, whitelist)
+    after = environment.sanitize(env, exclude, include)
 
     assert all(x in after for x in expected)
     assert all(x not in after for x in deleted)
@@ -460,7 +460,7 @@ def test_from_environment_diff(before, after, search_list):
 @pytest.mark.skipif(sys.platform == 'win32',
                     reason="LMod not supported on Windows")
 @pytest.mark.regression('15775')
-def test_blacklist_lmod_variables():
+def test_exclude_lmod_variables():
     # Construct the list of environment modifications
     file = os.path.join(datadir, 'sourceme_lmod.sh')
     env = EnvironmentModifications.from_sourcing_file(file)

lib/spack/spack/test/modules/common.py

@@ -11,7 +11,9 @@
 import spack.error
 import spack.modules.tcl
 import spack.package_base
+import spack.schema.modules
 import spack.spec
+import spack.util.spack_yaml as syaml
 from spack.modules.common import UpstreamModuleIndex
 from spack.spec import Spec
 
@@ -226,3 +228,34 @@ def find_nothing(*args):
     assert module_path
 
     spack.package_base.PackageBase.uninstall_by_spec(spec)
+
+
+# DEPRECATED: remove blacklist in v0.20
+@pytest.mark.parametrize("module_type, old_config,new_config", [
+    ("tcl",  "blacklist.yaml",             "exclude.yaml"),
+    ("tcl",  "blacklist_implicits.yaml",   "exclude_implicits.yaml"),
+    ("tcl",  "blacklist_environment.yaml", "alter_environment.yaml"),
+    ("lmod", "blacklist.yaml",             "exclude.yaml"),
+    ("lmod", "blacklist_environment.yaml", "alter_environment.yaml"),
+])
+def test_exclude_include_update(module_type, old_config, new_config):
+    module_test_data_root = os.path.join(
+        spack.paths.test_path, 'data', 'modules', module_type
+    )
+    with open(os.path.join(module_test_data_root, old_config)) as f:
+        old_yaml = syaml.load(f)
+    with open(os.path.join(module_test_data_root, new_config)) as f:
+        new_yaml = syaml.load(f)
+
+    # ensure file that needs updating is translated to the right thing.
+    assert spack.schema.modules.update_keys(
+        old_yaml, spack.schema.modules.exclude_include_translations
+    )
+    assert new_yaml == old_yaml
+
+    # ensure a file that doesn't need updates doesn't get updated
+    original_new_yaml = new_yaml.copy()
+    assert not spack.schema.modules.update_keys(
+        new_yaml, spack.schema.modules.exclude_include_translations
+    )
+    original_new_yaml == new_yaml

lib/spack/spack/test/modules/lmod.py

@@ -110,10 +110,16 @@ def test_autoload_all(self, modulefile_content, module_configuration):
 
         assert len([x for x in content if 'depends_on(' in x]) == 5
 
-    def test_alter_environment(self, modulefile_content, module_configuration):
+    # DEPRECATED: remove blacklist in v0.20
+    @pytest.mark.parametrize(
+        "config_name", ["alter_environment", "blacklist_environment"]
+    )
+    def test_alter_environment(
+            self, modulefile_content, module_configuration, config_name
+    ):
         """Tests modifications to run-time environment."""
 
-        module_configuration('alter_environment')
+        module_configuration(config_name)
         content = modulefile_content('mpileaks platform=test target=x86_64')
 
         assert len(
@@ -145,10 +151,11 @@ def test_prepend_path_separator(self, modulefile_content,
             elif re.match(r'[a-z]+_path\("SEMICOLON"', line):
                 assert line.endswith('"bar", ";")')
 
-    def test_blacklist(self, modulefile_content, module_configuration):
+    @pytest.mark.parametrize("config_name", ["exclude", "blacklist"])
-        """Tests blacklisting the generation of selected modules."""
+    def test_exclude(self, modulefile_content, module_configuration, config_name):
+        """Tests excluding the generation of selected modules."""
 
-        module_configuration('blacklist')
+        module_configuration(config_name)
         content = modulefile_content(mpileaks_spec_string)
 
         assert len([x for x in content if 'depends_on(' in x]) == 1

lib/spack/spack/test/modules/tcl.py

@@ -97,10 +97,16 @@ def test_prerequisites_all(self, modulefile_content, module_configuration):
 
         assert len([x for x in content if 'prereq' in x]) == 5
 
-    def test_alter_environment(self, modulefile_content, module_configuration):
+    # DEPRECATED: remove blacklist in v0.20
+    @pytest.mark.parametrize(
+        "config_name", ["alter_environment", "blacklist_environment"]
+    )
+    def test_alter_environment(
+            self, modulefile_content, module_configuration, config_name
+    ):
         """Tests modifications to run-time environment."""
 
-        module_configuration('alter_environment')
+        module_configuration(config_name)
         content = modulefile_content('mpileaks platform=test target=x86_64')
 
         assert len([x for x in content
@@ -129,10 +135,11 @@ def test_alter_environment(self, modulefile_content, module_configuration):
         assert len([x for x in content if 'module load foo/bar' in x]) == 1
         assert len([x for x in content if 'setenv LIBDWARF_ROOT' in x]) == 1
 
-    def test_blacklist(self, modulefile_content, module_configuration):
+    @pytest.mark.parametrize("config_name", ["exclude", "blacklist"])
-        """Tests blacklisting the generation of selected modules."""
+    def test_exclude(self, modulefile_content, module_configuration, config_name):
+        """Tests excluding the generation of selected modules."""
 
-        module_configuration('blacklist')
+        module_configuration(config_name)
         content = modulefile_content('mpileaks ^zmpi')
 
         assert len([x for x in content if 'is-loaded' in x]) == 1
@@ -359,24 +366,27 @@ def test_extend_context(
 
     @pytest.mark.regression('4400')
     @pytest.mark.db
-    def test_blacklist_implicits(
+    @pytest.mark.parametrize(
-            self, modulefile_content, module_configuration, database
+        "config_name", ["exclude_implicits", "blacklist_implicits"]
+    )
+    def test_exclude_implicits(
+            self, modulefile_content, module_configuration, database, config_name
     ):
-        module_configuration('blacklist_implicits')
+        module_configuration(config_name)
 
         # mpileaks has been installed explicitly when setting up
         # the tests database
         mpileaks_specs = database.query('mpileaks')
         for item in mpileaks_specs:
             writer = writer_cls(item, 'default')
-            assert not writer.conf.blacklisted
+            assert not writer.conf.excluded
 
         # callpath is a dependency of mpileaks, and has been pulled
         # in implicitly
         callpath_specs = database.query('callpath')
         for item in callpath_specs:
             writer = writer_cls(item, 'default')
-            assert writer.conf.blacklisted
+            assert writer.conf.excluded
 
     @pytest.mark.regression('9624')
     @pytest.mark.db

lib/spack/spack/util/environment.py

@@ -673,10 +673,10 @@ def from_sourcing_file(filename, *arguments, **kwargs):
                 (default: ``&> /dev/null``)
             concatenate_on_success (str): operator used to execute a command
                 only when the previous command succeeds (default: ``&&``)
-            blacklist ([str or re]): ignore any modifications of these
+            exclude ([str or re]): ignore any modifications of these
                 variables (default: [])
-            whitelist ([str or re]): always respect modifications of these
+            include ([str or re]): always respect modifications of these
-                variables (default: []). has precedence over blacklist.
+                variables (default: []). Supersedes any excluded variables.
             clean (bool): in addition to removing empty entries,
                 also remove duplicate entries (default: False).
         """
@@ -687,13 +687,13 @@ def from_sourcing_file(filename, *arguments, **kwargs):
             msg = 'Trying to source non-existing file: {0}'.format(filename)
             raise RuntimeError(msg)
 
-        # Prepare a whitelist and a blacklist of environment variable names
+        # Prepare include and exclude lists of environment variable names
-        blacklist = kwargs.get('blacklist', [])
+        exclude = kwargs.get('exclude', [])
-        whitelist = kwargs.get('whitelist', [])
+        include = kwargs.get('include', [])
         clean = kwargs.get('clean', False)
 
         # Other variables unrelated to sourcing a file
-        blacklist.extend([
+        exclude.extend([
             # Bash internals
             'SHLVL', '_', 'PWD', 'OLDPWD', 'PS1', 'PS2', 'ENV',
             # Environment modules v4
@@ -706,12 +706,12 @@ def from_sourcing_file(filename, *arguments, **kwargs):
         # Compute the environments before and after sourcing
         before = sanitize(
             environment_after_sourcing_files(os.devnull, **kwargs),
-            blacklist=blacklist, whitelist=whitelist
+            exclude=exclude, include=include
         )
         file_and_args = (filename,) + arguments
         after = sanitize(
             environment_after_sourcing_files(file_and_args, **kwargs),
-            blacklist=blacklist, whitelist=whitelist
+            exclude=exclude, include=include
         )
 
         # Delegate to the other factory
@@ -881,22 +881,6 @@ def validate(env, errstream):
         set_or_unset_not_first(variable, list_of_changes, errstream)
 
 
-def filter_environment_blacklist(env, variables):
-    """Generator that filters out any change to environment variables present in
-    the input list.
-
-    Args:
-        env: list of environment modifications
-        variables: list of variable names to be filtered
-
-    Returns:
-        items in env if they are not in variables
-    """
-    for item in env:
-        if item.name not in variables:
-            yield item
-
-
 def inspect_path(root, inspections, exclude=None):
     """Inspects ``root`` to search for the subdirectories in ``inspections``.
     Adds every path found to a list of prepend-path commands and returns it.
@@ -1060,17 +1044,15 @@ def _source_single_file(file_and_args, environment):
     return current_environment
 
 
-def sanitize(environment, blacklist, whitelist):
+def sanitize(environment, exclude, include):
     """Returns a copy of the input dictionary where all the keys that
-    match a blacklist pattern and don't match a whitelist pattern are
+    match an excluded pattern and don't match an included pattern are
     removed.
 
     Args:
         environment (dict): input dictionary
-        blacklist (list): literals or regex patterns to be
+        exclude (list): literals or regex patterns to be excluded
-            blacklisted
+        include (list): literals or regex patterns to be included
-        whitelist (list): literals or regex patterns to be
-            whitelisted
     """
 
     def set_intersection(fullset, *args):
@@ -1088,9 +1070,9 @@ def set_intersection(fullset, *args):
     # Don't modify input, make a copy instead
     environment = sjson.decode_json_dict(dict(environment))
 
-    # Retain (whitelist) has priority over prune (blacklist)
+    # include supersedes any excluded items
-    prune = set_intersection(set(environment), *blacklist)
+    prune = set_intersection(set(environment), *exclude)
-    prune -= set_intersection(prune, *whitelist)
+    prune -= set_intersection(prune, *include)
     for k in prune:
         environment.pop(k, None)