Skip to content

Commit

Permalink
windows: use sys.platform == "win32" instead of is_windows (spack…
Browse files Browse the repository at this point in the history
…#35640)

`mypy` only understands `sys.platform == "win32"`, not indirect assignments of that
value to things like `is_windows`. If we don't use the accepted platform checks, `mypy`
registers many Windows-only symbols as not present on Linux, when it should skip the
checks for platform-specific code.
  • Loading branch information
tgamblin authored Mar 5, 2023
1 parent 4561536 commit 42a0241
Show file tree
Hide file tree
Showing 37 changed files with 133 additions and 196 deletions.
37 changes: 17 additions & 20 deletions lib/spack/llnl/util/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import sys
import tempfile
from contextlib import contextmanager
from sys import platform as _platform
from typing import Callable, List, Match, Optional, Tuple, Union

from llnl.util import tty
Expand All @@ -26,9 +25,7 @@
from spack.util.executable import Executable, which
from spack.util.path import path_to_os_path, system_path_filter

is_windows = _platform == "win32"

if not is_windows:
if sys.platform != "win32":
import grp
import pwd
else:
Expand Down Expand Up @@ -154,7 +151,7 @@ def lookup(name):


def getuid():
if is_windows:
if sys.platform == "win32":
import ctypes

if ctypes.windll.shell32.IsUserAnAdmin() == 0:
Expand All @@ -167,7 +164,7 @@ def getuid():
@system_path_filter
def rename(src, dst):
# On Windows, os.rename will fail if the destination file already exists
if is_windows:
if sys.platform == "win32":
# Windows path existence checks will sometimes fail on junctions/links/symlinks
# so check for that case
if os.path.exists(dst) or os.path.islink(dst):
Expand Down Expand Up @@ -196,7 +193,7 @@ def _get_mime_type():
"""Generate method to call `file` system command to aquire mime type
for a specified path
"""
if is_windows:
if sys.platform == "win32":
# -h option (no-dereference) does not exist in Windows
return file_command("-b", "--mime-type")
else:
Expand Down Expand Up @@ -551,7 +548,7 @@ def get_owner_uid(path, err_msg=None):
else:
p_stat = os.stat(path)

if _platform != "win32":
if sys.platform != "win32":
owner_uid = p_stat.st_uid
else:
sid = win32security.GetFileSecurity(
Expand Down Expand Up @@ -584,7 +581,7 @@ def group_ids(uid=None):
Returns:
(list of int): gids of groups the user is a member of
"""
if is_windows:
if sys.platform == "win32":
tty.warn("Function is not supported on Windows")
return []

Expand All @@ -604,7 +601,7 @@ def group_ids(uid=None):
@system_path_filter(arg_slice=slice(1))
def chgrp(path, group, follow_symlinks=True):
"""Implement the bash chgrp function on a single path"""
if is_windows:
if sys.platform == "win32":
raise OSError("Function 'chgrp' is not supported on Windows")

if isinstance(group, str):
Expand Down Expand Up @@ -1131,7 +1128,7 @@ def open_if_filename(str_or_file, mode="r"):
@system_path_filter
def touch(path):
"""Creates an empty file at the specified path."""
if is_windows:
if sys.platform == "win32":
perms = os.O_WRONLY | os.O_CREAT
else:
perms = os.O_WRONLY | os.O_CREAT | os.O_NONBLOCK | os.O_NOCTTY
Expand Down Expand Up @@ -1193,7 +1190,7 @@ def temp_cwd():
yield tmp_dir
finally:
kwargs = {}
if is_windows:
if sys.platform == "win32":
kwargs["ignore_errors"] = False
kwargs["onerror"] = readonly_file_handler(ignore_errors=True)
shutil.rmtree(tmp_dir, **kwargs)
Expand Down Expand Up @@ -1438,7 +1435,7 @@ def visit_directory_tree(root, visitor, rel_path="", depth=0):
try:
isdir = f.is_dir()
except OSError as e:
if is_windows and hasattr(e, "winerror") and e.winerror == 5 and islink:
if sys.platform == "win32" and hasattr(e, "winerror") and e.winerror == 5 and islink:
# if path is a symlink, determine destination and
# evaluate file vs directory
link_target = resolve_link_target_relative_to_the_link(f)
Expand Down Expand Up @@ -1547,11 +1544,11 @@ def readonly_file_handler(ignore_errors=False):
"""

def error_remove_readonly(func, path, exc):
if not is_windows:
if sys.platform != "win32":
raise RuntimeError("This method should only be invoked on Windows")
excvalue = exc[1]
if (
is_windows
sys.platform == "win32"
and func in (os.rmdir, os.remove, os.unlink)
and excvalue.errno == errno.EACCES
):
Expand Down Expand Up @@ -1581,7 +1578,7 @@ def remove_linked_tree(path):

# Windows readonly files cannot be removed by Python
# directly.
if is_windows:
if sys.platform == "win32":
kwargs["ignore_errors"] = False
kwargs["onerror"] = readonly_file_handler(ignore_errors=True)

Expand Down Expand Up @@ -2095,7 +2092,7 @@ def names(self):
# on non Windows platform
# Windows valid library extensions are:
# ['.dll', '.lib']
valid_exts = [".dll", ".lib"] if is_windows else [".dylib", ".so", ".a"]
valid_exts = [".dll", ".lib"] if sys.platform == "win32" else [".dylib", ".so", ".a"]
for ext in valid_exts:
i = name.rfind(ext)
if i != -1:
Expand Down Expand Up @@ -2243,7 +2240,7 @@ def find_libraries(libraries, root, shared=True, recursive=False, runtime=True):
message = message.format(find_libraries.__name__, type(libraries))
raise TypeError(message)

if is_windows:
if sys.platform == "win32":
static_ext = "lib"
# For linking (runtime=False) you need the .lib files regardless of
# whether you are doing a shared or static link
Expand Down Expand Up @@ -2275,7 +2272,7 @@ def find_libraries(libraries, root, shared=True, recursive=False, runtime=True):
# finally search all of root recursively. The search stops when the first
# match is found.
common_lib_dirs = ["lib", "lib64"]
if is_windows:
if sys.platform == "win32":
common_lib_dirs.extend(["bin", "Lib"])

for subdir in common_lib_dirs:
Expand Down Expand Up @@ -2410,7 +2407,7 @@ def _link(self, path, dest_dir):
# 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:
if sys.platform == "win32" and 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)
Expand Down
10 changes: 4 additions & 6 deletions lib/spack/llnl/util/symlink.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@
import errno
import os
import shutil
import sys
import tempfile
from os.path import exists, join
from sys import platform as _platform

from llnl.util import lang

is_windows = _platform == "win32"

if is_windows:
if sys.platform == "win32":
from win32file import CreateHardLink


Expand All @@ -23,7 +21,7 @@ def symlink(real_path, link_path):
On Windows, use junctions if os.symlink fails.
"""
if not is_windows:
if sys.platform != "win32":
os.symlink(real_path, link_path)
elif _win32_can_symlink():
# Windows requires target_is_directory=True when the target is a dir.
Expand Down Expand Up @@ -99,7 +97,7 @@ def _win32_is_junction(path):
if os.path.islink(path):
return False

if is_windows:
if sys.platform == "win32":
import ctypes.wintypes

GetFileAttributes = ctypes.windll.kernel32.GetFileAttributesW
Expand Down
3 changes: 1 addition & 2 deletions lib/spack/spack/cmd/unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
description = "run spack's unit tests (wrapper around pytest)"
section = "developer"
level = "long"
is_windows = sys.platform == "win32"


def setup_parser(subparser):
Expand Down Expand Up @@ -212,7 +211,7 @@ 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 not is_windows:
if sys.platform != "win32":
with spack.bootstrap.ensure_bootstrap_configuration():
spack.bootstrap.ensure_core_dependencies()
if pytest is None:
Expand Down
4 changes: 1 addition & 3 deletions lib/spack/spack/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@

__all__ = ["Compiler"]

is_windows = sys.platform == "win32"


@llnl.util.lang.memoized
def _get_compiler_version_output(compiler_path, version_arg, ignore_errors=()):
Expand Down Expand Up @@ -598,7 +596,7 @@ def search_regexps(cls, language):
suffixes = [""]
# Windows compilers generally have an extension of some sort
# as do most files on Windows, handle that case here
if is_windows:
if sys.platform == "win32":
ext = r"\.(?:exe|bat)"
cls_suf = [suf + ext for suf in cls.suffixes]
ext_suf = [ext]
Expand Down
11 changes: 5 additions & 6 deletions lib/spack/spack/detection/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import spack.util.spack_yaml
import spack.util.windows_registry

is_windows = sys.platform == "win32"
#: Information on a package that has been detected
DetectedPackage = collections.namedtuple("DetectedPackage", ["spec", "prefix"])

Expand Down Expand Up @@ -184,7 +183,7 @@ def library_prefix(library_dir):
elif "lib" in lowered_components:
idx = lowered_components.index("lib")
return os.sep.join(components[:idx])
elif is_windows and "bin" in lowered_components:
elif sys.platform == "win32" and "bin" in lowered_components:
idx = lowered_components.index("bin")
return os.sep.join(components[:idx])
else:
Expand Down Expand Up @@ -260,13 +259,13 @@ def find_windows_compiler_bundled_packages():


class WindowsKitExternalPaths(object):
if is_windows:
if sys.platform == "win32":
plat_major_ver = str(winOs.windows_version()[0])

@staticmethod
def find_windows_kit_roots():
"""Return Windows kit root, typically %programfiles%\\Windows Kits\\10|11\\"""
if not is_windows:
if sys.platform != "win32":
return []
program_files = os.environ["PROGRAMFILES(x86)"]
kit_base = os.path.join(
Expand Down Expand Up @@ -359,7 +358,7 @@ def compute_windows_program_path_for_package(pkg):
pkg (spack.package_base.PackageBase): package for which
Program Files location is to be computed
"""
if not is_windows:
if sys.platform != "win32":
return []
# note windows paths are fine here as this method should only ever be invoked
# to interact with Windows
Expand All @@ -379,7 +378,7 @@ def compute_windows_user_path_for_package(pkg):
installs see:
https://learn.microsoft.com/en-us/dotnet/api/system.environment.specialfolder?view=netframework-4.8
"""
if not is_windows:
if sys.platform != "win32":
return []

# Current user directory
Expand Down
6 changes: 2 additions & 4 deletions lib/spack/spack/detection/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
path_to_dict,
)

is_windows = sys.platform == "win32"


def common_windows_package_paths():
paths = WindowsCompilerExternalPaths.find_windows_compiler_bundled_packages()
Expand All @@ -57,7 +55,7 @@ def executables_in_path(path_hints):
path_hints (list): list of paths to be searched. If None the list will be
constructed based on the PATH environment variable.
"""
if is_windows:
if sys.platform == "win32":
path_hints.extend(common_windows_package_paths())
search_paths = llnl.util.filesystem.search_paths_for_executables(*path_hints)
return path_to_dict(search_paths)
Expand Down Expand Up @@ -149,7 +147,7 @@ def by_library(packages_to_check, path_hints=None):

path_to_lib_name = (
libraries_in_ld_and_system_library_path(path_hints=path_hints)
if not is_windows
if sys.platform != "win32"
else libraries_in_windows_paths(path_hints)
)

Expand Down
3 changes: 1 addition & 2 deletions lib/spack/spack/directory_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import spack.util.spack_json as sjson
from spack.error import SpackError

is_windows = sys.platform == "win32"
# Note: Posixpath is used here as opposed to
# os.path.join due to spack.spec.Spec.format
# requiring forward slash path seperators at this stage
Expand Down Expand Up @@ -346,7 +345,7 @@ def remove_install_directory(self, spec, deprecated=False):

# Windows readonly files cannot be removed by Python
# directly, change permissions before attempting to remove
if is_windows:
if sys.platform == "win32":
kwargs = {
"ignore_errors": False,
"onerror": fs.readonly_file_handler(ignore_errors=False),
Expand Down
2 changes: 0 additions & 2 deletions lib/spack/spack/fetch_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import os.path
import re
import shutil
import sys
import urllib.parse
from typing import List, Optional

Expand All @@ -53,7 +52,6 @@

#: List of all fetch strategies, created by FetchStrategy metaclass.
all_strategies = []
is_windows = sys.platform == "win32"

CONTENT_TYPE_MISMATCH_WARNING_TEMPLATE = (
"The contents of {subject} look like {content_type}. Either the URL"
Expand Down
3 changes: 1 addition & 2 deletions lib/spack/spack/hooks/sbang.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@

#: Groupdb does not exist on Windows, prevent imports
#: on supported systems
is_windows = sys.platform == "win32"
if not is_windows:
if sys.platform != "win32":
import grp

#: Spack itself also limits the shebang line to at most 4KB, which should be plenty.
Expand Down
9 changes: 3 additions & 6 deletions lib/spack/spack/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@
#: queue invariants).
STATUS_REMOVED = "removed"

is_windows = sys.platform == "win32"
is_osx = sys.platform == "darwin"


class InstallAction(object):
#: Don't perform an install
Expand Down Expand Up @@ -169,9 +166,9 @@ def _do_fake_install(pkg):
if not pkg.name.startswith("lib"):
library = "lib" + library

plat_shared = ".dll" if is_windows else ".so"
plat_static = ".lib" if is_windows else ".a"
dso_suffix = ".dylib" if is_osx else plat_shared
plat_shared = ".dll" if sys.platform == "win32" else ".so"
plat_static = ".lib" if sys.platform == "win32" else ".a"
dso_suffix = ".dylib" if sys.platform == "darwin" else plat_shared

# Install fake command
fs.mkdirp(pkg.prefix.bin)
Expand Down
Loading

0 comments on commit 42a0241

Please sign in to comment.