modules: correctly detect explicit installation (#36533)

When generating modulefile, correctly detect software installation asked
by user as explicit installation.

Explicit installation status were previously fetched from database
record of spec, which was only set after modulefile generation.

Code is updated to pass down the explicit status of software
installation to the object that generates modulefiles.

Fixes #34730.
Fixes #12105.

A value for the explicit argument has to be set when creating a new
installation, but for operations on existing installation, this value is
retrieved from database. Such operations are: module rm, module refresh,
module setdefaults or when get_module function is used.

Update on the way tests that mimics an installation, thus explicit
argument has to be set under such situation.

lib/spack/spack/binary_distribution.py

@@ -2026,7 +2026,7 @@ def install_root_node(spec, allow_root, unsigned=False, force=False, sha256=None
     with spack.util.path.filter_padding():
         tty.msg('Installing "{0}" from a buildcache'.format(spec.format()))
         extract_tarball(spec, download_result, allow_root, unsigned, force)
-        spack.hooks.post_install(spec)
+        spack.hooks.post_install(spec, False)
         spack.store.db.add(spec, spack.store.layout)
 
 

lib/spack/spack/hooks/__init__.py

@@ -12,7 +12,7 @@
 Currently the following hooks are supported:
 
     * pre_install(spec)
-    * post_install(spec)
+    * post_install(spec, explicit)
     * pre_uninstall(spec)
     * post_uninstall(spec)
     * on_install_start(spec)

lib/spack/spack/hooks/absolutify_elf_sonames.py

@@ -131,7 +131,7 @@ def find_and_patch_sonames(prefix, exclude_list, patchelf):
     return patch_sonames(patchelf, prefix, relative_paths)
 
 
-def post_install(spec):
+def post_install(spec, explicit=None):
     # Skip if disabled
     if not spack.config.get("config:shared_linking:bind", False):
         return

lib/spack/spack/hooks/licensing.py

@@ -169,7 +169,7 @@ def write_license_file(pkg, license_path):
         f.close()
 
 
-def post_install(spec):
+def post_install(spec, explicit=None):
     """This hook symlinks local licenses to the global license for
     licensed software.
     """

lib/spack/spack/hooks/module_file_generation.py

@@ -10,7 +10,7 @@
 import spack.modules.common
 
 
-def _for_each_enabled(spec, method_name):
+def _for_each_enabled(spec, method_name, explicit=None):
     """Calls a method for each enabled module"""
     set_names = set(spack.config.get("modules", {}).keys())
     # If we have old-style modules enabled, we put those in the default set
@@ -27,7 +27,7 @@ def _for_each_enabled(spec, method_name):
             continue
 
         for type in enabled:
-            generator = spack.modules.module_types[type](spec, name)
+            generator = spack.modules.module_types[type](spec, name, explicit)
             try:
                 getattr(generator, method_name)()
             except RuntimeError as e:
@@ -36,7 +36,7 @@ def _for_each_enabled(spec, method_name):
                 tty.warn(msg.format(method_name, str(e)))
 
 
-def post_install(spec):
+def post_install(spec, explicit):
     import spack.environment as ev  # break import cycle
 
     if ev.active_environment():
@@ -45,7 +45,7 @@ def post_install(spec):
         # can manage interactions between env views and modules
         return
 
-    _for_each_enabled(spec, "write")
+    _for_each_enabled(spec, "write", explicit)
 
 
 def post_uninstall(spec):

lib/spack/spack/hooks/permissions_setters.py

@@ -8,7 +8,7 @@
 import spack.util.file_permissions as fp
 
 
-def post_install(spec):
+def post_install(spec, explicit=None):
     if not spec.external:
         fp.set_permissions_by_spec(spec.prefix, spec)
 

lib/spack/spack/hooks/sbang.py

@@ -224,7 +224,7 @@ def install_sbang():
     os.rename(sbang_tmp_path, sbang_path)
 
 
-def post_install(spec):
+def post_install(spec, explicit=None):
     """This hook edits scripts so that they call /bin/bash
     $spack_prefix/bin/sbang instead of something longer than the
     shebang limit.

lib/spack/spack/hooks/write_install_manifest.py

@@ -6,6 +6,6 @@
 import spack.verify
 
 
-def post_install(spec):
+def post_install(spec, explicit=None):
     if not spec.external:
         spack.verify.write_manifest(spec)

lib/spack/spack/installer.py

@@ -315,7 +315,7 @@ def _install_from_cache(pkg, cache_only, explicit, unsigned=False):
     tty.debug("Successfully extracted {0} from binary cache".format(pkg_id))
     _print_timer(pre=_log_prefix(pkg.name), pkg_id=pkg_id, timer=t)
     _print_installed_pkg(pkg.spec.prefix)
-    spack.hooks.post_install(pkg.spec)
+    spack.hooks.post_install(pkg.spec, explicit)
     return True
 
 
@@ -353,7 +353,7 @@ def _process_external_package(pkg, explicit):
         # For external packages we just need to run
         # post-install hooks to generate module files.
         tty.debug("{0} generating module file".format(pre))
-        spack.hooks.post_install(spec)
+        spack.hooks.post_install(spec, explicit)
 
         # Add to the DB
         tty.debug("{0} registering into DB".format(pre))
@@ -1260,6 +1260,10 @@ def _install_task(self, task):
         if not pkg.unit_test_check():
             return
 
+        # Injecting information to know if this installation request is the root one
+        # to determine in BuildProcessInstaller whether installation is explicit or not
+        install_args["is_root"] = task.is_root
+
         try:
             self._setup_install_dir(pkg)
 
@@ -1879,6 +1883,9 @@ def __init__(self, pkg, install_args):
         # whether to enable echoing of build output initially or not
         self.verbose = install_args.get("verbose", False)
 
+        # whether installation was explicitly requested by the user
+        self.explicit = install_args.get("is_root", False) and install_args.get("explicit", True)
+
         # env before starting installation
         self.unmodified_env = install_args.get("unmodified_env", {})
 
@@ -1939,7 +1946,7 @@ def run(self):
                 self.timer.write_json(timelog)
 
             # Run post install hooks before build stage is removed.
-            spack.hooks.post_install(self.pkg.spec)
+            spack.hooks.post_install(self.pkg.spec, self.explicit)
 
         _print_timer(pre=self.pre, pkg_id=self.pkg_id, timer=self.timer)
         _print_installed_pkg(self.pkg.prefix)

lib/spack/spack/modules/common.py

@@ -428,12 +428,17 @@ class BaseConfiguration(object):
 
     default_projections = {"all": "{name}-{version}-{compiler.name}-{compiler.version}"}
 
-    def __init__(self, spec, module_set_name):
+    def __init__(self, spec, module_set_name, explicit=None):
         # Module where type(self) is defined
         self.module = inspect.getmodule(self)
         # Spec for which we want to generate a module file
         self.spec = spec
         self.name = module_set_name
+        # Software installation has been explicitly asked (get this information from
+        # db when querying an existing module, like during a refresh or rm operations)
+        if explicit is None:
+            explicit = spec._installed_explicitly()
+        self.explicit = explicit
         # Dictionary of configuration options that should be applied
         # to the spec
         self.conf = merge_config_rules(self.module.configuration(self.name), self.spec)
@@ -519,8 +524,7 @@ def excluded(self):
         # Should I exclude the module because it's implicit?
         # DEPRECATED: remove 'blacklist_implicits' in v0.20
         exclude_implicits = get_deprecated(conf, "exclude_implicits", "blacklist_implicits", None)
-        installed_implicitly = not spec._installed_explicitly()
+        excluded_as_implicit = exclude_implicits and not self.explicit
-        excluded_as_implicit = exclude_implicits and installed_implicitly
 
         def debug_info(line_header, match_list):
             if match_list:
@@ -788,7 +792,8 @@ def autoload(self):
     def _create_module_list_of(self, what):
         m = self.conf.module
         name = self.conf.name
-        return [m.make_layout(x, name).use_name for x in getattr(self.conf, what)]
+        explicit = self.conf.explicit
+        return [m.make_layout(x, name, explicit).use_name for x in getattr(self.conf, what)]
 
     @tengine.context_property
     def verbose(self):
@@ -797,7 +802,7 @@ def verbose(self):
 
 
 class BaseModuleFileWriter(object):
-    def __init__(self, spec, module_set_name):
+    def __init__(self, spec, module_set_name, explicit=None):
         self.spec = spec
 
         # This class is meant to be derived. Get the module of the
@@ -806,9 +811,9 @@ def __init__(self, spec, module_set_name):
         m = self.module
 
         # Create the triplet of configuration/layout/context
-        self.conf = m.make_configuration(spec, module_set_name)
+        self.conf = m.make_configuration(spec, module_set_name, explicit)
-        self.layout = m.make_layout(spec, module_set_name)
+        self.layout = m.make_layout(spec, module_set_name, explicit)
-        self.context = m.make_context(spec, module_set_name)
+        self.context = m.make_context(spec, module_set_name, explicit)
 
         # Check if a default template has been defined,
         # throw if not found

lib/spack/spack/modules/lmod.py

@@ -33,24 +33,26 @@ def configuration(module_set_name):
 configuration_registry: Dict[str, Any] = {}
 
 
-def make_configuration(spec, module_set_name):
+def make_configuration(spec, module_set_name, explicit):
     """Returns the lmod configuration for spec"""
-    key = (spec.dag_hash(), module_set_name)
+    key = (spec.dag_hash(), module_set_name, explicit)
     try:
         return configuration_registry[key]
     except KeyError:
-        return configuration_registry.setdefault(key, LmodConfiguration(spec, module_set_name))
+        return configuration_registry.setdefault(
+            key, LmodConfiguration(spec, module_set_name, explicit)
+        )
 
 
-def make_layout(spec, module_set_name):
+def make_layout(spec, module_set_name, explicit):
     """Returns the layout information for spec"""
-    conf = make_configuration(spec, module_set_name)
+    conf = make_configuration(spec, module_set_name, explicit)
     return LmodFileLayout(conf)
 
 
-def make_context(spec, module_set_name):
+def make_context(spec, module_set_name, explicit):
     """Returns the context information for spec"""
-    conf = make_configuration(spec, module_set_name)
+    conf = make_configuration(spec, module_set_name, explicit)
     return LmodContext(conf)
 
 
@@ -409,7 +411,7 @@ def missing(self):
     @tengine.context_property
     def unlocked_paths(self):
         """Returns the list of paths that are unlocked unconditionally."""
-        layout = make_layout(self.spec, self.conf.name)
+        layout = make_layout(self.spec, self.conf.name, self.conf.explicit)
         return [os.path.join(*parts) for parts in layout.unlocked_paths[None]]
 
     @tengine.context_property
@@ -417,7 +419,7 @@ def conditionally_unlocked_paths(self):
         """Returns the list of paths that are unlocked conditionally.
         Each item in the list is a tuple with the structure (condition, path).
         """
-        layout = make_layout(self.spec, self.conf.name)
+        layout = make_layout(self.spec, self.conf.name, self.conf.explicit)
         value = []
         conditional_paths = layout.unlocked_paths
         conditional_paths.pop(None)

lib/spack/spack/modules/tcl.py

@@ -30,24 +30,26 @@ def configuration(module_set_name):
 configuration_registry: Dict[str, Any] = {}
 
 
-def make_configuration(spec, module_set_name):
+def make_configuration(spec, module_set_name, explicit):
     """Returns the tcl configuration for spec"""
-    key = (spec.dag_hash(), module_set_name)
+    key = (spec.dag_hash(), module_set_name, explicit)
     try:
         return configuration_registry[key]
     except KeyError:
-        return configuration_registry.setdefault(key, TclConfiguration(spec, module_set_name))
+        return configuration_registry.setdefault(
+            key, TclConfiguration(spec, module_set_name, explicit)
+        )
 
 
-def make_layout(spec, module_set_name):
+def make_layout(spec, module_set_name, explicit):
     """Returns the layout information for spec"""
-    conf = make_configuration(spec, module_set_name)
+    conf = make_configuration(spec, module_set_name, explicit)
     return TclFileLayout(conf)
 
 
-def make_context(spec, module_set_name):
+def make_context(spec, module_set_name, explicit):
     """Returns the context information for spec"""
-    conf = make_configuration(spec, module_set_name)
+    conf = make_configuration(spec, module_set_name, explicit)
     return TclContext(conf)
 
 

lib/spack/spack/rewiring.py

@@ -119,7 +119,7 @@ def rewire_node(spec, explicit):
     spack.store.db.add(spec, spack.store.layout, explicit=explicit)
 
     # run post install hooks
-    spack.hooks.post_install(spec)
+    spack.hooks.post_install(spec, explicit)
 
 
 class RewireError(spack.error.SpackError):

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

@@ -67,7 +67,7 @@ def test_modules_default_symlink(
     module_type, mock_packages, mock_module_filename, mock_module_defaults, config
 ):
     spec = spack.spec.Spec("mpileaks@2.3").concretized()
-    mock_module_defaults(spec.format("{name}{@version}"))
+    mock_module_defaults(spec.format("{name}{@version}"), True)
 
     generator_cls = spack.modules.module_types[module_type]
     generator = generator_cls(spec, "default")

lib/spack/spack/test/modules/conftest.py

@@ -19,11 +19,11 @@ def modulefile_content(request):
 
     writer_cls = getattr(request.module, "writer_cls")
 
-    def _impl(spec_str, module_set_name="default"):
+    def _impl(spec_str, module_set_name="default", explicit=True):
         # Write the module file
         spec = spack.spec.Spec(spec_str)
         spec.concretize()
-        generator = writer_cls(spec, module_set_name)
+        generator = writer_cls(spec, module_set_name, explicit)
         generator.write(overwrite=True)
 
         # Get its filename
@@ -56,10 +56,10 @@ def factory(request):
     # Class of the module file writer
     writer_cls = getattr(request.module, "writer_cls")
 
-    def _mock(spec_string, module_set_name="default"):
+    def _mock(spec_string, module_set_name="default", explicit=True):
         spec = spack.spec.Spec(spec_string)
         spec.concretize()
-        return writer_cls(spec, module_set_name), spec
+        return writer_cls(spec, module_set_name, explicit), spec
 
     return _mock
 

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

@@ -356,9 +356,7 @@ def test_extend_context(self, modulefile_content, module_configuration):
     @pytest.mark.regression("4400")
     @pytest.mark.db
     @pytest.mark.parametrize("config_name", ["exclude_implicits", "blacklist_implicits"])
-    def test_exclude_implicits(
+    def test_exclude_implicits(self, module_configuration, database, config_name):
-        self, modulefile_content, module_configuration, database, config_name
-    ):
         module_configuration(config_name)
 
         # mpileaks has been installed explicitly when setting up
@@ -375,6 +373,23 @@ def test_exclude_implicits(
             writer = writer_cls(item, "default")
             assert writer.conf.excluded
 
+    @pytest.mark.regression("12105")
+    @pytest.mark.parametrize("config_name", ["exclude_implicits", "blacklist_implicits"])
+    def test_exclude_implicits_with_arg(self, module_configuration, config_name):
+        module_configuration(config_name)
+
+        # mpileaks is defined as explicit with explicit argument set on writer
+        mpileaks_spec = spack.spec.Spec("mpileaks")
+        mpileaks_spec.concretize()
+        writer = writer_cls(mpileaks_spec, "default", True)
+        assert not writer.conf.excluded
+
+        # callpath is defined as implicit with explicit argument set on writer
+        callpath_spec = spack.spec.Spec("callpath")
+        callpath_spec.concretize()
+        writer = writer_cls(callpath_spec, "default", False)
+        assert writer.conf.excluded
+
     @pytest.mark.regression("9624")
     @pytest.mark.db
     def test_autoload_with_constraints(self, modulefile_content, module_configuration, database):