Windows: fix library loading and enable Clingo bootstrapping (#33400)

Changes to improve locating shared libraries on Windows, which in
turn enables the use of Clingo. This PR attempts to establish a
proper distinction between linking on Windows vs. Linux/Mac: on
Windows, linking is always done with .lib files (never .dll files).
This somewhat complicates the model since the Spec.lib method could
return libraries that were used for both linking and loading, but
since these are not always the same on Windows, it was decided to
treat Spec.libs as being for link-time libraries. Additional functions
are added to help dependents locate run-time libraries.

* Clingo is now the default concretizer on Windows
* Clingo is now the concretizer used for unit tests on Windows
* Fix a permissions issue that can occur while moving Git files during
  fetching/staging
* Packages can now implement "win_add_library_dependent" to register
  files/directories that include libraries that would need to link
  to dependency dlls
* Packages can now implement "win_add_rpath" to register the locations
  of dlls that dependents would want to load
* "Spec.libs" on Windows is updated to return link-time libraries
  (i.e. .lib files, rather than .dll files)
* PackageBase.rpath on Windows is now updated to return the most-likely
  locations where .dlls will be found (which is generally in the bin/
  directory)

8 files changed, 115 insertions, 73 deletions

diff --git a/.github/workflows/windows_python.yml b/.github/workflows/windows_python.yml
index f1b828ceaa..7e6caa0189 100644
--- a/.github/workflows/windows_python.yml
+++ b/.github/workflows/windows_python.yml
@@ -23,7 +23,7 @@ jobs:
         python-version: 3.9
     - name: Install Python packages
       run: |
-          python -m pip install --upgrade pip six pywin32 setuptools codecov pytest-cov
+          python -m pip install --upgrade pip six pywin32 setuptools codecov pytest-cov clingo
     - name: Create local develop
       run: |
         .\spack\.github\workflows\setup_git.ps1
@@ -49,7 +49,7 @@ jobs:
         python-version: 3.9
     - name: Install Python packages
       run: |
-          python -m pip install --upgrade pip six pywin32 setuptools codecov coverage pytest-cov
+          python -m pip install --upgrade pip six pywin32 setuptools codecov coverage pytest-cov clingo
     - name: Create local develop
       run: |
         .\spack\.github\workflows\setup_git.ps1
@@ -81,7 +81,7 @@ jobs:
         echo F|xcopy .\spack\share\spack\qa\configuration\windows_config.yaml $env:USERPROFILE\.spack\windows\config.yaml
         spack external find cmake
         spack external find ninja
-        spack install abseil-cpp
+        spack -d install abseil-cpp
   make-installer:
     runs-on: windows-latest
     steps:
diff --git a/etc/spack/defaults/windows/config.yaml b/etc/spack/defaults/windows/config.yaml
index 956bc97c20..367bf831cf 100644
--- a/etc/spack/defaults/windows/config.yaml
+++ b/etc/spack/defaults/windows/config.yaml
@@ -1,5 +1,5 @@
 config:
   locks: false
-  concretizer: original
+  concretizer: clingo
   build_stage::
     - '$spack/.staging'
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index 511f7c4f59..fa1188c133 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -1083,7 +1083,11 @@ def temp_cwd():
         with working_dir(tmp_dir):
             yield tmp_dir
     finally:
-        shutil.rmtree(tmp_dir)
+        kwargs = {}
+        if is_windows:
+            kwargs["ignore_errors"] = False
+            kwargs["onerror"] = readonly_file_handler(ignore_errors=True)
+        shutil.rmtree(tmp_dir, **kwargs)
 
 
 @contextmanager
@@ -2095,7 +2099,7 @@ def find_system_libraries(libraries, shared=True):
     return libraries_found
 
 
-def find_libraries(libraries, root, shared=True, recursive=False):
+def find_libraries(libraries, root, shared=True, recursive=False, runtime=True):
     """Returns an iterable of full paths to libraries found in a root dir.
 
     Accepts any glob characters accepted by fnmatch:
@@ -2116,6 +2120,10 @@ def find_libraries(libraries, root, shared=True, recursive=False):
             otherwise for static. Defaults to True.
         recursive (bool): if False search only root folder,
             if True descends top-down from the root. Defaults to False.
+        runtime (bool): Windows only option, no-op elsewhere. If true,
+            search for runtime shared libs (.DLL), otherwise, search
+            for .Lib files. If shared is false, this has no meaning.
+            Defaults to True.
 
     Returns:
         LibraryList: The libraries that have been found
@@ -2130,7 +2138,9 @@ def find_libraries(libraries, root, shared=True, recursive=False):
 
     if is_windows:
         static_ext = "lib"
-        shared_ext = "dll"
+        # For linking (runtime=False) you need the .lib files regardless of
+        # whether you are doing a shared or static link
+        shared_ext = "dll" if runtime else "lib"
     else:
         # Used on both Linux and macOS
         static_ext = "a"
@@ -2174,13 +2184,13 @@ def find_libraries(libraries, root, shared=True, recursive=False):
     return LibraryList(found_libs)
 
 
-def find_all_shared_libraries(root, recursive=False):
+def find_all_shared_libraries(root, recursive=False, runtime=True):
     """Convenience function that returns the list of all shared libraries found
     in the directory passed as argument.
 
     See documentation for `llnl.util.filesystem.find_libraries` for more information
     """
-    return find_libraries("*", root=root, shared=True, recursive=recursive)
+    return find_libraries("*", root=root, shared=True, recursive=recursive, runtime=runtime)
 
 
 def find_all_static_libraries(root, recursive=False):
@@ -2226,48 +2236,36 @@ class WindowsSimulatedRPath(object):
         self.pkg = package
         self._addl_rpaths = set()
         self.link_install_prefix = link_install_prefix
-        self._internal_links = set()
+        self._additional_library_dependents = set()
 
     @property
-    def link_dest(self):
+    def library_dependents(self):
         """
         Set of directories where package binaries/libraries are located.
         """
-        if hasattr(self.pkg, "libs") and self.pkg.libs:
-            pkg_libs = set(self.pkg.libs.directories)
-        else:
-            pkg_libs = set((self.pkg.prefix.lib, self.pkg.prefix.lib64))
-
-        return pkg_libs | set([self.pkg.prefix.bin]) | self.internal_links
-
-    @property
-    def internal_links(self):
-        """
-        linking that would need to be established within the package itself. Useful for links
-        against extension modules/build time executables/internal linkage
-        """
-        return self._internal_links
+        return set([self.pkg.prefix.bin]) | self._additional_library_dependents
 
-    def add_internal_links(self, *dest):
+    def add_library_dependent(self, *dest):
         """
-        Incorporate additional paths into the rpath (sym)linking scheme.
-
-        Paths provided to this method are linked against by a package's libraries
-        and libraries found at these paths are linked against a package's binaries.
-        (i.e. /site-packages -> /bin and /bin -> /site-packages)
+        Add paths to directories or libraries/binaries to set of
+        common paths that need to link against other libraries
 
-        Specified paths should be outside of a package's lib, lib64, and bin
+        Specified paths should fall outside of a package's common
+        link paths, i.e. the bin
         directories.
         """
-        self._internal_links = self._internal_links | set(*dest)
+        for pth in dest:
+            if os.path.isfile(pth):
+                self._additional_library_dependents.add(os.path.dirname)
+            else:
+                self._additional_library_dependents.add(pth)
 
     @property
-    def link_targets(self):
+    def rpaths(self):
         """
         Set of libraries this package needs to link against during runtime
         These packages will each be symlinked into the packages lib and binary dir
         """
-
         dependent_libs = []
         for path in self.pkg.rpath:
             dependent_libs.extend(list(find_all_shared_libraries(path, recursive=True)))
@@ -2275,18 +2273,43 @@ class WindowsSimulatedRPath(object):
             dependent_libs.extend(list(find_all_shared_libraries(extra_path, recursive=True)))
         return set(dependent_libs)
 
-    def include_additional_link_paths(self, *paths):
+    def add_rpath(self, *paths):
         """
         Add libraries found at the root of provided paths to runtime linking
 
         These are libraries found outside of the typical scope of rpath linking
-        that require manual inclusion in a runtime linking scheme
+        that require manual inclusion in a runtime linking scheme.
+        These links are unidirectional, and are only
+        intended to bring outside dependencies into this package
 
         Args:
             *paths (str): arbitrary number of paths to be added to runtime linking
         """
         self._addl_rpaths = self._addl_rpaths | set(paths)
 
+    def _link(self, path, dest):
+        file_name = os.path.basename(path)
+        dest_file = os.path.join(dest, file_name)
+        if os.path.exists(dest):
+            try:
+                symlink(path, dest_file)
+            # For py2 compatibility, we have to catch the specific Windows error code
+            # associate with trying to create a file that already exists (winerror 183)
+            except OSError as e:
+                if e.winerror == 183:
+                    # We have either already symlinked or we are encoutering a naming clash
+                    # either way, we don't want to overwrite existing libraries
+                    already_linked = islink(dest_file)
+                    tty.debug(
+                        "Linking library %s to %s failed, " % (path, dest_file) + "already linked."
+                        if already_linked
+                        else "library with name %s already exists at location %s."
+                        % (file_name, dest)
+                    )
+                    pass
+                else:
+                    raise e
+
     def establish_link(self):
         """
         (sym)link packages to runtime dependencies based on RPath configuration for
@@ -2298,29 +2321,8 @@ class WindowsSimulatedRPath(object):
 
         # for each binary install dir in self.pkg (i.e. pkg.prefix.bin, pkg.prefix.lib)
         # install a symlink to each dependent library
-        for library, lib_dir in itertools.product(self.link_targets, self.link_dest):
-            if not path_contains_subdirectory(library, lib_dir):
-                file_name = os.path.basename(library)
-                dest_file = os.path.join(lib_dir, file_name)
-                if os.path.exists(lib_dir):
-                    try:
-                        symlink(library, dest_file)
-                    # For py2 compatibility, we have to catch the specific Windows error code
-                    # associate with trying to create a file that already exists (winerror 183)
-                    except OSError as e:
-                        if e.winerror == 183:
-                            # We have either already symlinked or we are encoutering a naming clash
-                            # either way, we don't want to overwrite existing libraries
-                            already_linked = islink(dest_file)
-                            tty.debug(
-                                "Linking library %s to %s failed, " % (library, dest_file)
-                                + "already linked."
-                                if already_linked
-                                else "library with name %s already exists." % file_name
-                            )
-                            pass
-                        else:
-                            raise e
+        for library, lib_dir in itertools.product(self.rpaths, self.library_dependents):
+            self._link(library, lib_dir)
 
 
 @system_path_filter
diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index ea85c6a682..df993978af 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -865,7 +865,12 @@ class GitFetchStrategy(VCSFetchStrategy):
                 repo_name = get_single_file(".")
                 if self.stage:
                     self.stage.srcdir = repo_name
-                shutil.move(repo_name, dest)
+                shutil.copytree(repo_name, dest, symlinks=True)
+                shutil.rmtree(
+                    repo_name,
+                    ignore_errors=False,
+                    onerror=fs.readonly_file_handler(ignore_errors=True),
+                )
 
             with working_dir(dest):
                 checkout_args = ["checkout", commit]
diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py
index 3c3bcaa84d..cf96beca58 100644
--- a/lib/spack/spack/package_base.py
+++ b/lib/spack/spack/package_base.py
@@ -140,11 +140,30 @@ class WindowsRPathMeta(object):
     they would a genuine RPATH, i.e. adding directories that contain
     runtime library dependencies"""
 
-    def add_search_paths(self, *path):
-        """Add additional rpaths that are not implicitly included in the search
-        scheme
+    def win_add_library_dependent(self):
+        """Return extra set of directories that require linking for package
+
+        This method should be overridden by packages that produce
+        binaries/libraries/python extension modules/etc that are installed into
+        directories outside a package's `bin`, `lib`, and `lib64` directories,
+        but still require linking against one of the packages dependencies, or
+        other components of the package itself. No-op otherwise.
+
+        Returns:
+            List of additional directories that require linking
+        """
+        return []
+
+    def win_add_rpath(self):
+        """Return extra set of rpaths for package
+
+        This method should be overridden by packages needing to
+        include additional paths to be searched by rpath. No-op otherwise
+
+        Returns:
+            List of additional rpaths
         """
-        self.win_rpath.include_additional_link_paths(*path)
+        return []
 
     def windows_establish_runtime_linkage(self):
         """Establish RPATH on Windows
@@ -152,6 +171,8 @@ class WindowsRPathMeta(object):
         Performs symlinking to incorporate rpath dependencies to Windows runtime search paths
         """
         if is_windows:
+            self.win_rpath.add_library_dependent(*self.win_add_library_dependent())
+            self.win_rpath.add_rpath(*self.win_add_rpath())
             self.win_rpath.establish_link()
 
 
@@ -2571,12 +2592,17 @@ class PackageBase(six.with_metaclass(PackageMeta, WindowsRPathMeta, PackageViewM
     @property
     def rpath(self):
         """Get the rpath this package links with, as a list of paths."""
-        rpaths = [self.prefix.lib, self.prefix.lib64]
         deps = self.spec.dependencies(deptype="link")
-        rpaths.extend(d.prefix.lib for d in deps if os.path.isdir(d.prefix.lib))
-        rpaths.extend(d.prefix.lib64 for d in deps if os.path.isdir(d.prefix.lib64))
+
+        # on Windows, libraries of runtime interest are typically
+        # stored in the bin directory
         if is_windows:
+            rpaths = [self.prefix.bin]
             rpaths.extend(d.prefix.bin for d in deps if os.path.isdir(d.prefix.bin))
+        else:
+            rpaths = [self.prefix.lib, self.prefix.lib64]
+            rpaths.extend(d.prefix.lib for d in deps if os.path.isdir(d.prefix.lib))
+            rpaths.extend(d.prefix.lib64 for d in deps if os.path.isdir(d.prefix.lib64))
         return rpaths
 
     @property
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 2f4120a5f8..a05451b7b4 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -1030,7 +1030,9 @@ def _libs_default_handler(descriptor, spec, cls):
     )
 
     for shared in search_shared:
-        libs = fs.find_libraries(name, home, shared=shared, recursive=True)
+        # Since we are searching for link libraries, on Windows search only for
+        # ".Lib" extensions by default as those represent import libraries for implict links.
+        libs = fs.find_libraries(name, home, shared=shared, recursive=True, runtime=False)
         if libs:
             return libs
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index ccdc19a174..ffc05a7fc0 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -2,7 +2,7 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-import os
+import posixpath
 import sys
 
 import jinja2
@@ -459,7 +459,7 @@ class TestConcretize(object):
     def test_external_package(self):
         spec = Spec("externaltool%gcc")
         spec.concretize()
-        assert spec["externaltool"].external_path == os.path.sep + os.path.join(
+        assert spec["externaltool"].external_path == posixpath.sep + posixpath.join(
             "path", "to", "external_tool"
         )
         assert "externalprereq" not in spec
@@ -490,10 +490,10 @@ class TestConcretize(object):
     def test_external_and_virtual(self):
         spec = Spec("externaltest")
         spec.concretize()
-        assert spec["externaltool"].external_path == os.path.sep + os.path.join(
+        assert spec["externaltool"].external_path == posixpath.sep + posixpath.join(
             "path", "to", "external_tool"
         )
-        assert spec["stuff"].external_path == os.path.sep + os.path.join(
+        assert spec["stuff"].external_path == posixpath.sep + posixpath.join(
             "path", "to", "external_virtual_gcc"
         )
         assert spec["externaltool"].compiler.satisfies("gcc")
diff --git a/var/spack/repos/builtin/packages/clingo/package.py b/var/spack/repos/builtin/packages/clingo/package.py
index a3d39d8e62..a24ffb0956 100644
--- a/var/spack/repos/builtin/packages/clingo/package.py
+++ b/var/spack/repos/builtin/packages/clingo/package.py
@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import os
 
 from spack.compiler import UnsupportedCompilerFlag
 from spack.package import *
@@ -120,3 +121,9 @@ class Clingo(CMakePackage):
             args += ["-DCLINGO_BUILD_WITH_PYTHON=OFF"]
 
         return args
+
+    def win_add_library_dependent(self):
+        if "+python" in self.spec:
+            return [os.path.join(self.prefix, self.spec["python"].package.platlib)]
+        else:
+            return []