Build logic fix: reorder definition of package module variables (#46992)

#44327 made sure to always run `set_package_py_globals` on all
packages before running `setup_dependent_package` for any package,
so that packages implementing the latter could depend on variables
like `spack_cc` being defined.

This ran into an undocumented dependency: `std_cmake_args` is set in
`set_package_py_globals` and makes use of `cmake_prefix_paths` (if it
is defined in the package); `py-torch`es implementation of
`cmake_prefix_paths` depends on a variable set by
`setup_dependent_package` (`python_platlib`).

This generally restores #44327, and corrects the resulting issue by
moving assignment of `std_cmake_args` to after both actions have been
run.

2 files changed, 41 insertions, 4 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 3737caf35e..ed62e517ef 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -617,14 +617,12 @@ def set_package_py_globals(pkg, context: Context = Context.BUILD):
     """
     module = ModuleChangePropagator(pkg)
 
+    jobs = spack.config.determine_number_of_jobs(parallel=pkg.parallel)
+    module.make_jobs = jobs
     if context == Context.BUILD:
-        module.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
         module.std_meson_args = spack.build_systems.meson.MesonBuilder.std_args(pkg)
         module.std_pip_args = spack.build_systems.python.PythonPipBuilder.std_args(pkg)
 
-    jobs = spack.config.determine_number_of_jobs(parallel=pkg.parallel)
-    module.make_jobs = jobs
-
     # TODO: make these build deps that can be installed if not found.
     module.make = MakeExecutable("make", jobs)
     module.gmake = MakeExecutable("gmake", jobs)
@@ -1048,6 +1046,12 @@ class SetupContext:
                     # This includes runtime dependencies, also runtime deps of direct build deps.
                     set_package_py_globals(pkg, context=Context.RUN)
 
+        # Looping over the set of packages a second time
+        # ensures all globals are loaded into the module space prior to
+        # any package setup. This guarantees package setup methods have
+        # access to expected module level definitions such as "spack_cc"
+        for dspec, flag in chain(self.external, self.nonexternal):
+            pkg = dspec.package
             for spec in dspec.dependents():
                 # Note: some specs have dependents that are unreachable from the root, so avoid
                 # setting globals for those.
@@ -1057,6 +1061,15 @@ class SetupContext:
                 pkg.setup_dependent_package(dependent_module, spec)
                 dependent_module.propagate_changes_to_mro()
 
+        pkg = self.specs[0].package
+        if self.context == Context.BUILD:
+            module = ModuleChangePropagator(pkg)
+            # std_cmake_args is not sufficiently static to be defined
+            # in set_package_py_globals and is deprecated so its handled
+            # here as a special case
+            module.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
+            module.propagate_changes_to_mro()
+
     def get_env_modifications(self) -> EnvironmentModifications:
         """Returns the environment variable modifications for the given input specs and context.
         Environment modifications include:
diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py
index f435a893a1..281d79f856 100644
--- a/lib/spack/spack/test/build_environment.py
+++ b/lib/spack/spack/test/build_environment.py
@@ -516,6 +516,30 @@ def test_setting_dtags_based_on_config(config_setting, expected_flag, config, mo
         assert dtags_to_add.value == expected_flag
 
 
+def test_module_globals_available_at_setup_dependent_time(
+    monkeypatch, mutable_config, mock_packages, working_env
+):
+    """Spack built package externaltest depends on an external package
+    externaltool. Externaltool's setup_dependent_package needs to be able to
+    access globals on the dependent"""
+
+    def setup_dependent_package(module, dependent_spec):
+        # Make sure set_package_py_globals was already called on
+        # dependents
+        # ninja is always set by the setup context and is not None
+        dependent_module = dependent_spec.package.module
+        assert hasattr(dependent_module, "ninja")
+        assert dependent_module.ninja is not None
+        dependent_spec.package.test_attr = True
+
+    externaltool = spack.spec.Spec("externaltest").concretized()
+    monkeypatch.setattr(
+        externaltool["externaltool"].package, "setup_dependent_package", setup_dependent_package
+    )
+    spack.build_environment.setup_package(externaltool.package, False)
+    assert externaltool.package.test_attr
+
+
 def test_build_jobs_sequential_is_sequential():
     assert (
         spack.config.determine_number_of_jobs(