Windows: enable "spack install" tests (#34696)

* The module-level skip for tests in `cmd.install` on Windows is removed.
  A few classes of errors still persist:

  * Cdash tests are not working on Windows
  * Tests for failed installs are also not working (this will require
    investigating bugs in output redirection)
  * Environments are not yet supported on Windows

  overall though, this enables testing of most basic uses of "spack install"
* Git repositories cached for version lookups were using a layout that
  mimicked the URL as much as possible. This was useful for listing the
  cache directory and understanding what was present at a glance, but
  the paths were overly long on Windows. On all systems, the layout is
  now a single directory based on a hash of the Git URL and is shortened
  (which ensures a consistent and acceptable length, and also avoids
  special characters).
  * In particular, this removes util.url.parse_git_url and its associated
    test, which were used exclusively for generating the git cache layout
* Bootstrapping is now enabled for unit tests on Windows
This commit is contained in:
markus-ferrell 2023-08-14 16:15:40 -04:00 committed by GitHub
parent 4d945be955
commit d823037c40
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 145 additions and 212 deletions

View file

@ -209,7 +209,6 @@ def unit_test(parser, args, unknown_args):
# mock configuration used by unit tests
# Note: skip on windows here because for the moment,
# clingo is wholly unsupported from bootstrap
if sys.platform != "win32":
with spack.bootstrap.ensure_bootstrap_configuration():
spack.bootstrap.ensure_core_dependencies()
if pytest is None:

View file

@ -11,6 +11,7 @@
import shutil
import sys
from contextlib import contextmanager
from pathlib import Path
import llnl.util.filesystem as fs
import llnl.util.tty as tty
@ -104,7 +105,7 @@ def relative_path_for_spec(self, spec):
projection = spack.projections.get_projection(self.projections, spec)
path = spec.format(projection)
return path
return str(Path(path))
def write_spec(self, spec, path):
"""Write a spec out to a file."""

View file

@ -7,6 +7,7 @@
import filecmp
import itertools
import os
import pathlib
import re
import sys
import time
@ -38,8 +39,6 @@
buildcache = SpackCommand("buildcache")
find = SpackCommand("find")
pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="does not run on windows")
@pytest.fixture()
def noop_install(monkeypatch):
@ -204,7 +203,7 @@ def test_show_log_on_error(
assert isinstance(install.error, spack.build_environment.ChildError)
assert install.error.pkg.name == "build-error"
assert "==> Installing build-error" in out
assert "Installing build-error" in out
assert "See build log for details:" in out
@ -263,9 +262,9 @@ def test_install_commit(mock_git_version_info, install_mockery, mock_packages, m
"""
repo_path, filename, commits = mock_git_version_info
monkeypatch.setattr(
spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False
)
file_url = pathlib.Path(repo_path).as_uri()
monkeypatch.setattr(spack.package_base.PackageBase, "git", file_url, raising=False)
# Use the earliest commit in the respository
spec = Spec(f"git-test-commit@{commits[-1]}").concretized()
@ -548,6 +547,9 @@ def test_cdash_report_concretization_error(
assert any(x in content for x in expected_messages)
@pytest.mark.skipif(
sys.platform == "win32", reason="Windows log_output logs phase header out of order"
)
@pytest.mark.disable_clean_stage_check
def test_cdash_upload_build_error(tmpdir, mock_fetch, install_mockery, capfd):
# capfd interferes with Spack's capturing
@ -747,6 +749,10 @@ def test_install_deps_then_package(tmpdir, mock_fetch, install_mockery):
assert os.path.exists(root.prefix)
@pytest.mark.skipif(
sys.platform == "win32",
reason="Environment views not supported on windows. Revisit after #34701",
)
@pytest.mark.regression("12002")
def test_install_only_dependencies_in_env(
tmpdir, mock_fetch, install_mockery, mutable_mock_env_path
@ -896,7 +902,7 @@ def test_install_help_does_not_show_cdash_options(capsys):
assert "CDash URL" not in captured.out
def test_install_help_cdash(capsys):
def test_install_help_cdash():
"""Make sure `spack install --help-cdash` describes CDash arguments"""
install_cmd = SpackCommand("install")
out = install_cmd("--help-cdash")
@ -913,6 +919,9 @@ def test_cdash_auth_token(tmpdir, mock_fetch, install_mockery, capfd):
assert "Using CDash auth token from environment" in out
@pytest.mark.skipif(
sys.platform == "win32", reason="Windows log_output logs phase header out of order"
)
@pytest.mark.disable_clean_stage_check
def test_cdash_configure_warning(tmpdir, mock_fetch, install_mockery, capfd):
# capfd interferes with Spack's capturing of e.g., Build.xml output
@ -938,6 +947,9 @@ def test_cdash_configure_warning(tmpdir, mock_fetch, install_mockery, capfd):
assert "foo: No such file or directory" in content
@pytest.mark.skipif(
sys.platform == "win32", reason="ArchSpec gives test platform debian rather than windows"
)
def test_compiler_bootstrap(
install_mockery_mutable_config,
mock_packages,
@ -954,6 +966,7 @@ def test_compiler_bootstrap(
install("a%gcc@=12.0")
@pytest.mark.skipif(sys.platform == "win32", reason="Binary mirrors not supported on windows")
def test_compiler_bootstrap_from_binary_mirror(
install_mockery_mutable_config,
mock_packages,
@ -994,6 +1007,9 @@ def test_compiler_bootstrap_from_binary_mirror(
install("--no-cache", "--only", "package", "b%gcc@10.2.0")
@pytest.mark.skipif(
sys.platform == "win32", reason="ArchSpec gives test platform debian rather than windows"
)
@pytest.mark.regression("16221")
def test_compiler_bootstrap_already_installed(
install_mockery_mutable_config,
@ -1037,6 +1053,10 @@ def test_install_fails_no_args_suggests_env_activation(tmpdir):
assert "using the `spack.yaml` in this directory" in output
@pytest.mark.skipif(
sys.platform == "win32",
reason="Environment views not supported on windows. Revisit after #34701",
)
def test_install_env_with_tests_all(
tmpdir, mock_packages, mock_fetch, install_mockery, mutable_mock_env_path
):
@ -1048,6 +1068,10 @@ def test_install_env_with_tests_all(
assert os.path.exists(test_dep.prefix)
@pytest.mark.skipif(
sys.platform == "win32",
reason="Environment views not supported on windows. Revisit after #34701",
)
def test_install_env_with_tests_root(
tmpdir, mock_packages, mock_fetch, install_mockery, mutable_mock_env_path
):
@ -1059,6 +1083,10 @@ def test_install_env_with_tests_root(
assert not os.path.exists(test_dep.prefix)
@pytest.mark.skipif(
sys.platform == "win32",
reason="Environment views not supported on windows. Revisit after #34701",
)
def test_install_empty_env(
tmpdir, mock_packages, mock_fetch, install_mockery, mutable_mock_env_path
):
@ -1072,6 +1100,10 @@ def test_install_empty_env(
assert "no specs to install" in out
@pytest.mark.skipif(
sys.platform == "win32",
reason="Windows logger I/O operation on closed file when install fails",
)
@pytest.mark.disable_clean_stage_check
@pytest.mark.parametrize(
"name,method",
@ -1095,6 +1127,7 @@ def test_installation_fail_tests(install_mockery, mock_fetch, name, method):
assert "See test log for details" in output
@pytest.mark.skipif(sys.platform == "win32", reason="Buildcache not supported on windows")
def test_install_use_buildcache(
capsys,
mock_packages,
@ -1172,6 +1205,10 @@ def install_use_buildcache(opt):
install_use_buildcache(opt)
@pytest.mark.skipif(
sys.platform == "win32",
reason="Windows logger I/O operation on closed file when install fails",
)
@pytest.mark.regression("34006")
@pytest.mark.disable_clean_stage_check
def test_padded_install_runtests_root(install_mockery_mutable_config, mock_fetch):

View file

@ -8,6 +8,7 @@
"""
import os
import os.path
from pathlib import Path
import pytest
@ -29,9 +30,13 @@ def test_yaml_directory_layout_parameters(tmpdir, default_mock_concretization):
# Ensure default layout matches expected spec format
layout_default = DirectoryLayout(str(tmpdir))
path_default = layout_default.relative_path_for_spec(spec)
assert path_default == spec.format(
assert path_default == str(
Path(
spec.format(
"{architecture}/" "{compiler.name}-{compiler.version}/" "{name}-{version}-{hash}"
)
)
)
# Test hash_length parameter works correctly
layout_10 = DirectoryLayout(str(tmpdir), hash_length=10)
@ -43,7 +48,7 @@ def test_yaml_directory_layout_parameters(tmpdir, default_mock_concretization):
assert len(path_default) - len(path_7) == 25
# Test path_scheme
arch, compiler, package7 = path_7.split("/")
arch, compiler, package7 = path_7.split(os.sep)
projections_package7 = {"all": "{name}-{version}-{hash:7}"}
layout_package7 = DirectoryLayout(str(tmpdir), projections=projections_package7)
path_package7 = layout_package7.relative_path_for_spec(spec)
@ -61,10 +66,10 @@ def test_yaml_directory_layout_parameters(tmpdir, default_mock_concretization):
layout_arch_ns = DirectoryLayout(str(tmpdir), projections=arch_ns_scheme_projections)
arch_path_spec2 = layout_arch_ns.relative_path_for_spec(spec2)
assert arch_path_spec2 == spec2.format(arch_scheme)
assert arch_path_spec2 == str(Path(spec2.format(arch_scheme)))
ns_path_spec = layout_arch_ns.relative_path_for_spec(spec)
assert ns_path_spec == spec.format(ns_scheme)
assert ns_path_spec == str(Path(spec.format(ns_scheme)))
# Ensure conflicting parameters caught
with pytest.raises(InvalidDirectoryLayoutParametersError):

View file

@ -8,8 +8,6 @@
import os.path
import urllib.parse
import pytest
import spack.util.path
import spack.util.url as url_util
@ -198,89 +196,6 @@ def test_url_join_absolute_paths():
assert url_util.join(*args, resolve_href=False) == "http://example.com/path/resource"
@pytest.mark.parametrize(
"url,parts",
[
(
"ssh://user@host.xz:500/path/to/repo.git/",
("ssh", "user", "host.xz", 500, "/path/to/repo.git"),
),
(
"ssh://user@host.xz/path/to/repo.git/",
("ssh", "user", "host.xz", None, "/path/to/repo.git"),
),
(
"ssh://host.xz:500/path/to/repo.git/",
("ssh", None, "host.xz", 500, "/path/to/repo.git"),
),
("ssh://host.xz/path/to/repo.git/", ("ssh", None, "host.xz", None, "/path/to/repo.git")),
(
"ssh://user@host.xz/path/to/repo.git/",
("ssh", "user", "host.xz", None, "/path/to/repo.git"),
),
("ssh://host.xz/path/to/repo.git/", ("ssh", None, "host.xz", None, "/path/to/repo.git")),
(
"ssh://user@host.xz/~user/path/to/repo.git/",
("ssh", "user", "host.xz", None, "~user/path/to/repo.git"),
),
(
"ssh://host.xz/~user/path/to/repo.git/",
("ssh", None, "host.xz", None, "~user/path/to/repo.git"),
),
(
"ssh://user@host.xz/~/path/to/repo.git",
("ssh", "user", "host.xz", None, "~/path/to/repo.git"),
),
("ssh://host.xz/~/path/to/repo.git", ("ssh", None, "host.xz", None, "~/path/to/repo.git")),
("git@github.com:spack/spack.git", (None, "git", "github.com", None, "spack/spack.git")),
("user@host.xz:/path/to/repo.git/", (None, "user", "host.xz", None, "/path/to/repo.git")),
("host.xz:/path/to/repo.git/", (None, None, "host.xz", None, "/path/to/repo.git")),
(
"user@host.xz:~user/path/to/repo.git/",
(None, "user", "host.xz", None, "~user/path/to/repo.git"),
),
(
"host.xz:~user/path/to/repo.git/",
(None, None, "host.xz", None, "~user/path/to/repo.git"),
),
("user@host.xz:path/to/repo.git", (None, "user", "host.xz", None, "path/to/repo.git")),
("host.xz:path/to/repo.git", (None, None, "host.xz", None, "path/to/repo.git")),
(
"rsync://host.xz/path/to/repo.git/",
("rsync", None, "host.xz", None, "/path/to/repo.git"),
),
("git://host.xz/path/to/repo.git/", ("git", None, "host.xz", None, "/path/to/repo.git")),
(
"git://host.xz/~user/path/to/repo.git/",
("git", None, "host.xz", None, "~user/path/to/repo.git"),
),
("http://host.xz/path/to/repo.git/", ("http", None, "host.xz", None, "/path/to/repo.git")),
(
"https://host.xz/path/to/repo.git/",
("https", None, "host.xz", None, "/path/to/repo.git"),
),
("https://github.com/spack/spack", ("https", None, "github.com", None, "/spack/spack")),
("https://github.com/spack/spack/", ("https", None, "github.com", None, "/spack/spack")),
("file:///path/to/repo.git/", ("file", None, None, None, "/path/to/repo.git")),
("file://~/path/to/repo.git/", ("file", None, None, None, "~/path/to/repo.git")),
# bad ports should give us None
("ssh://host.xz:port/path/to/repo.git/", None),
# bad ports should give us None
("ssh://host-foo.xz:port/path/to/repo.git/", None),
# regular file paths should give us None
("/path/to/repo.git/", None),
("path/to/repo.git/", None),
("~/path/to/repo.git", None),
],
)
def test_git_url_parse(url, parts):
if parts is None:
with pytest.raises(ValueError):
url_util.parse_git_url(url)
else:
assert parts == url_util.parse_git_url(url)
def test_default_download_name():
url = "https://example.com:1234/path/to/file.txt;params?abc=def#file=blob.tar"
filename = url_util.default_download_filename(url)

View file

@ -8,6 +8,7 @@
where it makes sense.
"""
import os
import pathlib
import sys
import pytest
@ -598,11 +599,10 @@ def test_invalid_versions(version_str):
Version(version_str)
@pytest.mark.skipif(sys.platform == "win32", reason="Not supported on Windows (yet)")
def test_versions_from_git(git, mock_git_version_info, monkeypatch, mock_packages):
repo_path, filename, commits = mock_git_version_info
monkeypatch.setattr(
spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False
spack.package_base.PackageBase, "git", pathlib.Path(repo_path).as_uri(), raising=False
)
for commit in commits:
@ -619,7 +619,6 @@ def test_versions_from_git(git, mock_git_version_info, monkeypatch, mock_package
assert str(comparator) == expected
@pytest.mark.skipif(sys.platform == "win32", reason="Not supported on Windows (yet)")
@pytest.mark.parametrize(
"commit_idx,expected_satisfies,expected_not_satisfies",
[
@ -643,7 +642,7 @@ def test_git_hash_comparisons(
"""Check that hashes compare properly to versions"""
repo_path, filename, commits = mock_git_version_info
monkeypatch.setattr(
spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False
spack.package_base.PackageBase, "git", pathlib.Path(repo_path).as_uri(), raising=False
)
spec = spack.spec.Spec(f"git-test-commit@{commits[commit_idx]}").concretized()
@ -654,12 +653,11 @@ def test_git_hash_comparisons(
assert not spec.satisfies(item)
@pytest.mark.skipif(sys.platform == "win32", reason="Not supported on Windows (yet)")
def test_git_ref_comparisons(mock_git_version_info, install_mockery, mock_packages, monkeypatch):
"""Check that hashes compare properly to versions"""
repo_path, filename, commits = mock_git_version_info
monkeypatch.setattr(
spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False
spack.package_base.PackageBase, "git", pathlib.Path(repo_path).as_uri(), raising=False
)
# Spec based on tag v1.0
@ -788,7 +786,6 @@ def test_version_intersects_satisfies_semantic(lhs_str, rhs_str, expected):
),
],
)
@pytest.mark.skipif(sys.platform == "win32", reason="Not supported on Windows (yet)")
def test_git_versions_without_explicit_reference(
spec_str,
tested_intersects,
@ -799,7 +796,7 @@ def test_git_versions_without_explicit_reference(
):
repo_path, filename, commits = mock_git_version_info
monkeypatch.setattr(
spack.package_base.PackageBase, "git", "file://%s" % repo_path, raising=False
spack.package_base.PackageBase, "git", pathlib.Path(repo_path).as_uri(), raising=False
)
spec = spack.spec.Spec(spec_str)

View file

@ -10,7 +10,6 @@
import itertools
import os
import posixpath
import re
import sys
import urllib.parse
import urllib.request
@ -243,59 +242,6 @@ def _join(base_url, path, *extra, **kwargs):
)
git_re = (
r"^(?:([a-z]+)://)?" # 1. optional scheme
r"(?:([^@]+)@)?" # 2. optional user
r"([^:/~]+)?" # 3. optional hostname
r"(?(1)(?::([^:/]+))?|:)" # 4. :<optional port> if scheme else :
r"(.*[^/])/?$" # 5. path
)
def parse_git_url(url):
"""Parse git URL into components.
This parses URLs that look like:
* ``https://host.com:443/path/to/repo.git``, or
* ``git@host.com:path/to/repo.git``
Anything not matching those patterns is likely a local
file or invalid.
Returned components are as follows (optional values can be ``None``):
1. ``scheme`` (optional): git, ssh, http, https
2. ``user`` (optional): ``git@`` for github, username for http or ssh
3. ``hostname``: domain of server
4. ``port`` (optional): port on server
5. ``path``: path on the server, e.g. spack/spack
Returns:
(tuple): tuple containing URL components as above
Raises ``ValueError`` for invalid URLs.
"""
match = re.match(git_re, url)
if not match:
raise ValueError("bad git URL: %s" % url)
# initial parse
scheme, user, hostname, port, path = match.groups()
# special handling for ~ paths (they're never absolute)
if path.startswith("/~"):
path = path[1:]
if port is not None:
try:
port = int(port)
except ValueError:
raise ValueError("bad port in git url: %s" % url)
return (scheme, user, hostname, port, path)
def default_download_filename(url: str) -> str:
"""This method computes a default file name for a given URL.
Note that it makes no request, so this is not the same as the

View file

@ -5,6 +5,7 @@
import os
import re
from pathlib import Path
from typing import Dict, Optional, Tuple
from llnl.util.filesystem import mkdirp, working_dir
@ -14,8 +15,8 @@
import spack.paths
import spack.repo
import spack.util.executable
import spack.util.hash
import spack.util.spack_json as sjson
import spack.util.url
import spack.version
from .common import VersionLookupError
@ -60,9 +61,7 @@ def __init__(self, pkg_name):
def cache_key(self):
if not self._cache_key:
key_base = "git_metadata"
if not self.repository_uri.startswith("/"):
key_base += "/"
self._cache_key = key_base + self.repository_uri
self._cache_key = (Path(key_base) / self.repository_uri).as_posix()
# Cache data in misc_cache
# If this is the first lazy access, initialize the cache as well
@ -98,14 +97,7 @@ def fetcher(self):
@property
def repository_uri(self):
"""Identifier for git repos used within the repo and metadata caches."""
try:
components = [
str(c).lstrip("/") for c in spack.util.url.parse_git_url(self.pkg.git) if c
]
return os.path.join(*components)
except ValueError:
# If it's not a git url, it's a local path
return os.path.abspath(self.pkg.git)
return Path(spack.util.hash.b32_hash(self.pkg.git)[-7:])
def save(self):
"""Save the data to file"""
@ -136,9 +128,8 @@ def lookup_ref(self, ref) -> Tuple[Optional[str], int]:
known version prior to the commit, as well as the distance from that version
to the commit in the git repo. Those values are used to compare Version objects.
"""
dest = os.path.join(spack.paths.user_repos_cache_path, self.repository_uri)
if dest.endswith(".git"):
dest = dest[:-4]
pathlib_dest = Path(spack.paths.user_repos_cache_path) / self.repository_uri
dest = str(pathlib_dest)
# prepare a cache for the repository
dest_parent = os.path.dirname(dest)

View file

@ -3,6 +3,8 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import sys
from spack.package import *
@ -15,18 +17,38 @@ class BuildError(Package):
version("1.0", md5="0123456789abcdef0123456789abcdef")
def install(self, spec, prefix):
if sys.platform == "win32":
with open("configure.bat", "w") as f:
f.write(
"""
@ECHO off
ECHO checking build system type... x86_64-apple-darwin16.6.0
ECHO checking host system type... x86_64-apple-darwin16.6.0
ECHO checking for gcc... /Users/gamblin2/src/spack/lib/spack/env/clang/clang
ECHO checking whether the C compiler works... yes
ECHO checking for C compiler default output file name... a.out
ECHO checking for suffix of executables...
ECHO configure: error: in /path/to/some/file:
ECHO configure: error: cannot run C compiled programs.
EXIT /B 1
"""
)
Executable("configure.bat")("--prefix=%s" % self.prefix)
configure()
else:
with open("configure", "w") as f:
f.write(
"""#!/bin/sh\n
echo 'checking build system type... x86_64-apple-darwin16.6.0'
echo 'checking host system type... x86_64-apple-darwin16.6.0'
echo 'checking for gcc... /Users/gamblin2/src/spack/lib/spack/env/clang/clang'
echo 'checking whether the C compiler works... yes'
echo 'checking for C compiler default output file name... a.out'
echo 'checking for suffix of executables...'
echo 'configure: error: in /path/to/some/file:'
echo 'configure: error: cannot run C compiled programs.'
exit 1
"""
echo 'checking build system type... x86_64-apple-darwin16.6.0'
echo 'checking host system type... x86_64-apple-darwin16.6.0'
echo 'checking for gcc... /Users/gamblin2/src/spack/lib/spack/env/clang/clang'
echo 'checking whether the C compiler works... yes'
echo 'checking for C compiler default output file name... a.out'
echo 'checking for suffix of executables...'
echo 'configure: error: in /path/to/some/file:'
echo 'configure: error: cannot run C compiled programs.'
exit 1
"""
)
configure()

View file

@ -3,6 +3,8 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import sys
from spack.package import *
@ -15,16 +17,33 @@ class BuildWarnings(Package):
version("1.0", md5="0123456789abcdef0123456789abcdef")
def install(self, spec, prefix):
if sys.platform == "win32":
with open("configure.bat", "w") as f:
f.write(
"""
@ECHO off
ECHO 'checking for gcc... /Users/gamblin2/src/spack/lib/spack/env/clang/clang'
ECHO 'checking whether the C compiler works... yes'
ECHO 'checking for C compiler default output file name... a.out'
ECHO 'WARNING: ALL CAPITAL WARNING!'
ECHO 'checking for suffix of executables...'
ECHO 'foo.c:89: warning: some weird warning!'
EXIT /B 1
"""
)
Executable("configure.bat")("--prefix=%s" % self.prefix)
else:
with open("configure", "w") as f:
f.write(
"""#!/bin/sh\n
echo 'checking for gcc... /Users/gamblin2/src/spack/lib/spack/env/clang/clang'
echo 'checking whether the C compiler works... yes'
echo 'checking for C compiler default output file name... a.out'
echo 'WARNING: ALL CAPITAL WARNING!'
echo 'checking for suffix of executables...'
echo 'foo.c:89: warning: some weird warning!'
exit 1
"""
echo 'checking for gcc... /Users/gamblin2/src/spack/lib/spack/env/clang/clang'
echo 'checking whether the C compiler works... yes'
echo 'checking for C compiler default output file name... a.out'
echo 'WARNING: ALL CAPITAL WARNING!'
echo 'checking for suffix of executables...'
echo 'foo.c:89: warning: some weird warning!'
exit 1
"""
)
configure()

View file

@ -3,6 +3,8 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import os
from spack.package import *
@ -20,9 +22,8 @@ class PrintingPackage(Package):
def install(self, spec, prefix):
print("BEFORE INSTALL")
configure("--prefix=%s" % prefix)
make()
make("install")
mkdirp(prefix)
touch(os.path.join(prefix, "dummyfile"))
print("AFTER INSTALL")