From 9ec289857cd488c2de6f17537b7778052c2ab947 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Fri, 14 Apr 2023 10:59:12 +0200
Subject: [PATCH] netcdf: fix bugs introduced with multiple build systems split
 (#36825)

Fixes #36689

- The "base" builder class should be last in the MRO
- `filter_compiler_wrappers` needs to be moved to builders
- Decorating a function from a mixin class require using
   the correct metaclass for the mixin
---
 lib/spack/spack/builder.py                      |  3 ++-
 lib/spack/spack/mixins.py                       | 17 +++++++++--------
 .../repos/builtin/packages/netcdf-c/package.py  | 14 +++++++-------
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/lib/spack/spack/builder.py b/lib/spack/spack/builder.py
index 44dac364e93..db6ca7aa4b0 100644
--- a/lib/spack/spack/builder.py
+++ b/lib/spack/spack/builder.py
@@ -244,7 +244,8 @@ def __new__(mcs, name, bases, attr_dict):
                 callbacks_from_base = getattr(base, temporary_stage.attribute_name, None)
                 if callbacks_from_base:
                     break
-            callbacks_from_base = callbacks_from_base or []
+            else:
+                callbacks_from_base = []
 
             # Set the callbacks in this class and flush the temporary stage
             attr_dict[temporary_stage.attribute_name] = staged_callbacks[:] + callbacks_from_base
diff --git a/lib/spack/spack/mixins.py b/lib/spack/spack/mixins.py
index 541a2ba1bd8..189c3947efc 100644
--- a/lib/spack/spack/mixins.py
+++ b/lib/spack/spack/mixins.py
@@ -59,9 +59,10 @@ def filter_compiler_wrappers(*files, **kwargs):
 
     find_kwargs = {"recursive": kwargs.get("recursive", False)}
 
-    def _filter_compiler_wrappers_impl(self):
+    def _filter_compiler_wrappers_impl(pkg_or_builder):
+        pkg = getattr(pkg_or_builder, "pkg", pkg_or_builder)
         # Compute the absolute path of the search root
-        root = os.path.join(self.prefix, relative_root) if relative_root else self.prefix
+        root = os.path.join(pkg.prefix, relative_root) if relative_root else pkg.prefix
 
         # Compute the absolute path of the files to be filtered and
         # remove links from the list.
@@ -71,10 +72,10 @@ def _filter_compiler_wrappers_impl(self):
         x = llnl.util.filesystem.FileFilter(*abs_files)
 
         compiler_vars = [
-            ("CC", self.compiler.cc),
-            ("CXX", self.compiler.cxx),
-            ("F77", self.compiler.f77),
-            ("FC", self.compiler.fc),
+            ("CC", pkg.compiler.cc),
+            ("CXX", pkg.compiler.cxx),
+            ("F77", pkg.compiler.f77),
+            ("FC", pkg.compiler.fc),
         ]
 
         # Some paths to the compiler wrappers might be substrings of the others.
@@ -103,11 +104,11 @@ def _filter_compiler_wrappers_impl(self):
             x.filter(wrapper_path, compiler_path, **filter_kwargs)
 
         # Remove this linking flag if present (it turns RPATH into RUNPATH)
-        x.filter("{0}--enable-new-dtags".format(self.compiler.linker_arg), "", **filter_kwargs)
+        x.filter("{0}--enable-new-dtags".format(pkg.compiler.linker_arg), "", **filter_kwargs)
 
         # NAG compiler is usually mixed with GCC, which has a different
         # prefix for linker arguments.
-        if self.compiler.name == "nag":
+        if pkg.compiler.name == "nag":
             x.filter("-Wl,--enable-new-dtags", "", **filter_kwargs)
 
     spack.builder.run_after(after)(_filter_compiler_wrappers_impl)
diff --git a/var/spack/repos/builtin/packages/netcdf-c/package.py b/var/spack/repos/builtin/packages/netcdf-c/package.py
index f7024353d08..25d93a52cfd 100644
--- a/var/spack/repos/builtin/packages/netcdf-c/package.py
+++ b/var/spack/repos/builtin/packages/netcdf-c/package.py
@@ -6,8 +6,8 @@
 import os
 import sys
 
-from spack.build_systems.autotools import AutotoolsBuilder
-from spack.build_systems.cmake import CMakeBuilder
+import spack.builder
+from spack.build_systems import autotools, cmake
 from spack.package import *
 
 
@@ -151,8 +151,6 @@ class NetcdfC(CMakePackage, AutotoolsPackage):
     conflicts("+parallel-netcdf", when="@:4.0")
     conflicts("+hdf4", when="@:4.0")
 
-    filter_compiler_wrappers("nc-config", relative_root="bin")
-
     default_build_system = "cmake" if sys.platform == "win32" else "autotools"
 
     build_system("cmake", "autotools", default=default_build_system)
@@ -178,7 +176,7 @@ def setup_dependent_build_environment(self, env, dependent_spec):
             env.prepend_path("PATH", self._nc_config_backup_dir)
 
 
-class BackupStep:
+class BackupStep(metaclass=spack.builder.PhaseCallbacksMeta):
     @property
     def _nc_config_backup_dir(self):
         return join_path(self.pkg.metadata_dir, "spack-nc-config")
@@ -191,8 +189,10 @@ def backup_nc_config(self):
             mkdirp(self._nc_config_backup_dir)
             install(nc_config_file, self._nc_config_backup_dir)
 
+    filter_compiler_wrappers("nc-config", relative_root="bin")
 
-class CMakeBuilder(CMakeBuilder, BackupStep, Setup):
+
+class CMakeBuilder(BackupStep, Setup, cmake.CMakeBuilder):
     def cmake_args(self):
         base_cmake_args = [
             self.define_from_variant("BUILD_SHARED_LIBS", "shared"),
@@ -212,7 +212,7 @@ def cmake_args(self):
         return base_cmake_args
 
 
-class AutotoolsBuilder(AutotoolsBuilder, BackupStep, Setup):
+class AutotoolsBuilder(BackupStep, Setup, autotools.AutotoolsBuilder):
     @property
     def force_autoreconf(self):
         # The patch for 4.7.0 touches configure.ac.