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.

16 files changed, 78 insertions, 47 deletions

diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py
index 233a587244..bddd8ac077 100644
--- a/lib/spack/spack/binary_distribution.py
+++ b/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)
 
 
diff --git a/lib/spack/spack/hooks/__init__.py b/lib/spack/spack/hooks/__init__.py
index 8254648060..a1c8152d06 100644
--- a/lib/spack/spack/hooks/__init__.py
+++ b/lib/spack/spack/hooks/__init__.py
@@ -12,7 +12,7 @@ Hooks are not executed in any particular order.
 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)
diff --git a/lib/spack/spack/hooks/absolutify_elf_sonames.py b/lib/spack/spack/hooks/absolutify_elf_sonames.py
index 91377706e3..c0194a3266 100644
--- a/lib/spack/spack/hooks/absolutify_elf_sonames.py
+++ b/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
diff --git a/lib/spack/spack/hooks/licensing.py b/lib/spack/spack/hooks/licensing.py
index 1957205694..27628776f2 100644
--- a/lib/spack/spack/hooks/licensing.py
+++ b/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.
     """
diff --git a/lib/spack/spack/hooks/module_file_generation.py b/lib/spack/spack/hooks/module_file_generation.py
index 17a345dccb..e5e83cc9c8 100644
--- a/lib/spack/spack/hooks/module_file_generation.py
+++ b/lib/spack/spack/hooks/module_file_generation.py
@@ -10,7 +10,7 @@ import spack.modules
 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):
diff --git a/lib/spack/spack/hooks/permissions_setters.py b/lib/spack/spack/hooks/permissions_setters.py
index 13416e4ad5..fbd8079783 100644
--- a/lib/spack/spack/hooks/permissions_setters.py
+++ b/lib/spack/spack/hooks/permissions_setters.py
@@ -8,7 +8,7 @@ import os
 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)
 
diff --git a/lib/spack/spack/hooks/sbang.py b/lib/spack/spack/hooks/sbang.py
index 984202997e..7a6334dcc2 100644
--- a/lib/spack/spack/hooks/sbang.py
+++ b/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.
diff --git a/lib/spack/spack/hooks/write_install_manifest.py b/lib/spack/spack/hooks/write_install_manifest.py
index eb676b5843..fc796fb4b4 100644
--- a/lib/spack/spack/hooks/write_install_manifest.py
+++ b/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)
diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py
index 39ad78aa18..045e543e8c 100644
--- a/lib/spack/spack/installer.py
+++ b/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 @@ class PackageInstaller(object):
         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 @@ class BuildProcessInstaller(object):
         # 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 @@ class BuildProcessInstaller(object):
                 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)
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 7329e05455..3995233dba 100644
--- a/lib/spack/spack/modules/common.py
+++ b/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 @@ class BaseConfiguration(object):
         # 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 installed_implicitly
+        excluded_as_implicit = exclude_implicits and not self.explicit
 
         def debug_info(line_header, match_list):
             if match_list:
@@ -788,7 +792,8 @@ class BaseContext(tengine.Context):
     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 @@ class BaseContext(tengine.Context):
 
 
 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 @@ class BaseModuleFileWriter(object):
         m = self.module
 
         # Create the triplet of configuration/layout/context
-        self.conf = m.make_configuration(spec, module_set_name)
-        self.layout = m.make_layout(spec, module_set_name)
-        self.context = m.make_context(spec, module_set_name)
+        self.conf = m.make_configuration(spec, module_set_name, explicit)
+        self.layout = m.make_layout(spec, module_set_name, explicit)
+        self.context = m.make_context(spec, module_set_name, explicit)
 
         # Check if a default template has been defined,
         # throw if not found
diff --git a/lib/spack/spack/modules/lmod.py b/lib/spack/spack/modules/lmod.py
index d0f93d0ad3..81c3e74c42 100644
--- a/lib/spack/spack/modules/lmod.py
+++ b/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 @@ class LmodContext(BaseContext):
     @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 @@ class LmodContext(BaseContext):
         """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)
diff --git a/lib/spack/spack/modules/tcl.py b/lib/spack/spack/modules/tcl.py
index 765e7f7b2f..634426c357 100644
--- a/lib/spack/spack/modules/tcl.py
+++ b/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)
 
 
diff --git a/lib/spack/spack/rewiring.py b/lib/spack/spack/rewiring.py
index 359a36fbf9..cffdab52ba 100644
--- a/lib/spack/spack/rewiring.py
+++ b/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):
diff --git a/lib/spack/spack/test/modules/common.py b/lib/spack/spack/test/modules/common.py
index 3f1ed9ec6e..14b6b051a6 100644
--- a/lib/spack/spack/test/modules/common.py
+++ b/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")
diff --git a/lib/spack/spack/test/modules/conftest.py b/lib/spack/spack/test/modules/conftest.py
index 6f93348529..e6dfc024e2 100644
--- a/lib/spack/spack/test/modules/conftest.py
+++ b/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
 
diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py
index 5b60fba785..4345d1f04f 100644
--- a/lib/spack/spack/test/modules/tcl.py
+++ b/lib/spack/spack/test/modules/tcl.py
@@ -356,9 +356,7 @@ class TestTcl(object):
     @pytest.mark.regression("4400")
     @pytest.mark.db
     @pytest.mark.parametrize("config_name", ["exclude_implicits", "blacklist_implicits"])
-    def test_exclude_implicits(
-        self, modulefile_content, module_configuration, database, config_name
-    ):
+    def test_exclude_implicits(self, module_configuration, database, config_name):
         module_configuration(config_name)
 
         # mpileaks has been installed explicitly when setting up
@@ -375,6 +373,23 @@ class TestTcl(object):
             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):