From 42a02411b478f1fa3f6ab9c5e30b92b4df64d085 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sun, 5 Mar 2023 07:58:05 -0800 Subject: windows: use `sys.platform == "win32"` instead of `is_windows` (#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. --- .../repos/builtin.mock/packages/cmake/package.py | 4 +-- var/spack/repos/builtin/packages/cmake/package.py | 8 +++-- var/spack/repos/builtin/packages/perl/package.py | 24 +++++++-------- var/spack/repos/builtin/packages/python/package.py | 34 ++++++++++------------ var/spack/repos/builtin/packages/re2c/package.py | 4 +-- var/spack/repos/builtin/packages/scons/package.py | 4 +-- var/spack/repos/builtin/packages/wrf/package.py | 10 +++---- 7 files changed, 39 insertions(+), 49 deletions(-) (limited to 'var') diff --git a/var/spack/repos/builtin.mock/packages/cmake/package.py b/var/spack/repos/builtin.mock/packages/cmake/package.py index 19c1a6f1c1..82e9c073d3 100644 --- a/var/spack/repos/builtin.mock/packages/cmake/package.py +++ b/var/spack/repos/builtin.mock/packages/cmake/package.py @@ -8,8 +8,6 @@ import sys from spack.package import * -is_windows = sys.platform == "win32" - def check(condition, msg): """Raise an install error if condition is False.""" @@ -59,7 +57,7 @@ class Cmake(Package): os.environ["for_install"] == "for_install", "Couldn't read env var set in compile envieonmnt", ) - cmake_exe_ext = ".exe" if is_windows else "" + cmake_exe_ext = ".exe" if sys.platform == "win32" else "" cmake_exe = join_path(prefix.bin, "cmake{}".format(cmake_exe_ext)) touch(cmake_exe) set_executable(cmake_exe) diff --git a/var/spack/repos/builtin/packages/cmake/package.py b/var/spack/repos/builtin/packages/cmake/package.py index da0f3c16b5..368c71965b 100644 --- a/var/spack/repos/builtin/packages/cmake/package.py +++ b/var/spack/repos/builtin/packages/cmake/package.py @@ -10,8 +10,6 @@ import sys import spack.build_environment from spack.package import * -is_windows = sys.platform == "win32" - class Cmake(Package): """A cross-platform, open-source build system. CMake is a family of @@ -187,7 +185,11 @@ class Cmake(Package): default=False, description="Enables the generation of html and man page documentation", ) - variant("ncurses", default=not is_windows, description="Enables the build of the ncurses gui") + variant( + "ncurses", + default=sys.platform != "win32", + description="Enables the build of the ncurses gui", + ) # See https://gitlab.kitware.com/cmake/cmake/-/issues/21135 conflicts( diff --git a/var/spack/repos/builtin/packages/perl/package.py b/var/spack/repos/builtin/packages/perl/package.py index 429530d564..a0f534b74d 100644 --- a/var/spack/repos/builtin/packages/perl/package.py +++ b/var/spack/repos/builtin/packages/perl/package.py @@ -23,8 +23,6 @@ from llnl.util.symlink import symlink from spack.operating_systems.mac_os import macos_version from spack.package import * -is_windows = sys.platform == "win32" - class Perl(Package): # Perl doesn't use Autotools, it should subclass Package """Perl 5 is a highly capable, feature-rich programming language with over @@ -76,7 +74,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package extendable = True - if not is_windows: + if sys.platform != "win32": depends_on("gdbm@:1.23") # Bind us below gdbm-1.20 due to API change: https://github.com/Perl/perl5/issues/18915 depends_on("gdbm@:1.19", when="@:5.35") @@ -277,13 +275,13 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package return config_args def configure(self, spec, prefix): - if is_windows: + if sys.platform == "win32": return configure = Executable("./Configure") configure(*self.configure_args()) def build(self, spec, prefix): - if is_windows: + if sys.platform == "win32": pass else: make() @@ -291,7 +289,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package @run_after("build") @on_package_attributes(run_tests=True) def build_test(self): - if is_windows: + if sys.platform == "win32": win32_dir = os.path.join(self.stage.source_path, "win32") with working_dir(win32_dir): nmake("test", ignore_quotes=True) @@ -299,7 +297,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package make("test") def install(self, spec, prefix): - if is_windows: + if sys.platform == "win32": win32_dir = os.path.join(self.stage.source_path, "win32") with working_dir(win32_dir): nmake("install", *self.nmake_arguments, ignore_quotes=True) @@ -308,7 +306,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package @run_after("install") def symlink_windows(self): - if not is_windows: + if sys.platform != "win32": return win_install_path = os.path.join(self.prefix.bin, "MSWin32") if self.is_64bit(): @@ -331,7 +329,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package spec = self.spec maker = make cpan_dir = join_path("cpanm", "cpanm") - if is_windows: + if sys.platform == "win32": maker = nmake cpan_dir = join_path(self.stage.source_path, cpan_dir) if "+cpanm" in spec: @@ -352,7 +350,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package if perl_lib_dirs: perl_lib_path = ":".join(perl_lib_dirs) env.prepend_path("PERL5LIB", perl_lib_path) - if is_windows: + if sys.platform == "win32": env.append_path("PATH", self.prefix.bin) def setup_dependent_build_environment(self, env, dependent_spec): @@ -382,7 +380,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package mkdirp(module.perl_lib_dir) def setup_build_environment(self, env): - if is_windows: + if sys.platform == "win32": env.append_path("PATH", self.prefix.bin) return @@ -410,7 +408,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package frustrates filter_file on some filesystems (NFSv4), so make them temporarily writable. """ - if is_windows: + if sys.platform == "win32": return kwargs = {"ignore_absent": True, "backup": False, "string": False} @@ -478,7 +476,7 @@ class Perl(Package): # Perl doesn't use Autotools, it should subclass Package """ for ver in ("", self.spec.version): ext = "" - if is_windows: + if sys.platform == "win32": ext = ".exe" path = os.path.join(self.prefix.bin, "{0}{1}{2}".format(self.spec.name, ver, ext)) if os.path.exists(path): diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py index e52e5bc87a..747c5cd6e6 100644 --- a/var/spack/repos/builtin/packages/python/package.py +++ b/var/spack/repos/builtin/packages/python/package.py @@ -22,8 +22,6 @@ from spack.package import * from spack.util.environment import is_system_path from spack.util.prefix import Prefix -is_windows = sys.platform == "win32" - class Python(Package): """The Python programming language.""" @@ -125,12 +123,12 @@ class Python(Package): # See https://legacy.python.org/dev/peps/pep-0394/ variant( "pythoncmd", - default=not is_windows, + default=sys.platform != "win32", description="Symlink 'python3' executable to 'python' (not PEP 394 compliant)", ) # Optional Python modules - variant("readline", default=not is_windows, description="Build readline module") + variant("readline", default=sys.platform != "win32", description="Build readline module") variant("ssl", default=True, description="Build ssl module") variant("sqlite3", default=True, description="Build sqlite3 module") variant("dbm", default=True, description="Build dbm module") @@ -147,7 +145,7 @@ class Python(Package): variant("crypt", default=True, description="Build crypt module", when="@:3.12 platform=darwin") variant("crypt", default=True, description="Build crypt module", when="@:3.12 platform=cray") - if not is_windows: + if sys.platform != "win32": depends_on("pkgconfig@0.9.0:", type="build") depends_on("gettext +libxml2", when="+libxml2") depends_on("gettext ~libxml2", when="~libxml2") @@ -298,7 +296,7 @@ class Python(Package): variants += "~tix" # Some modules are platform-dependent - if not is_windows: + if sys.platform != "win32": try: python("-c", "import crypt", error=os.devnull) variants += "+crypt" @@ -542,7 +540,7 @@ class Python(Package): and an appropriately set prefix. """ with working_dir(self.stage.source_path, create=True): - if is_windows: + if sys.platform == "win32": pass else: options = getattr(self, "configure_flag_args", []) @@ -557,7 +555,7 @@ class Python(Package): # Windows builds use a batch script to drive # configure and build in one step with working_dir(self.stage.source_path): - if is_windows: + if sys.platform == "win32": pcbuild_root = os.path.join(self.stage.source_path, "PCbuild") builder_cmd = os.path.join(pcbuild_root, "build.bat") try: @@ -580,7 +578,7 @@ class Python(Package): :py:attr:``~.AutotoolsPackage.install_targets`` """ with working_dir(self.stage.source_path): - if is_windows: + if sys.platform == "win32": self.win_installer(prefix) else: make(*self.install_targets) @@ -593,7 +591,7 @@ class Python(Package): If this isn't done, they'll have CC and CXX set to Spack's generic cc and c++. We want them to be bound to whatever compiler they were built with.""" - if is_windows: + if sys.platform == "win32": return kwargs = {"ignore_absent": True, "backup": False, "string": True} @@ -605,7 +603,7 @@ class Python(Package): @run_after("install") def symlink(self): - if is_windows: + if sys.platform == "win32": return spec = self.spec prefix = self.prefix @@ -723,7 +721,7 @@ class Python(Package): # in that order if using python@3.11.0, for example. version = self.spec.version for ver in [version.up_to(2), version.up_to(1), ""]: - if not is_windows: + if sys.platform != "win32": path = os.path.join(self.prefix.bin, "python{0}".format(ver)) else: path = os.path.join(self.prefix, "python{0}.exe".format(ver)) @@ -772,11 +770,11 @@ print(json.dumps(config)) """ dag_hash = self.spec.dag_hash() - lib_prefix = "lib" if not is_windows else "" + lib_prefix = "lib" if sys.platform != "win32" else "" if dag_hash not in self._config_vars: # Default config vars version = self.version.up_to(2) - if is_windows: + if sys.platform == "win32": version = str(version).split(".")[0] config = { # get_config_vars @@ -886,9 +884,9 @@ print(json.dumps(config)) @property def libs(self): py_version = self.version.up_to(2) - if is_windows: + if sys.platform == "win32": py_version = str(py_version).replace(".", "") - lib_prefix = "lib" if not is_windows else "" + lib_prefix = "lib" if sys.platform != "win32" else "" # The values of LDLIBRARY and LIBRARY aren't reliable. Intel Python uses a # static binary but installs shared libraries, so sysconfig reports # libpythonX.Y.a but only libpythonX.Y.so exists. So we add our own paths, too. @@ -1121,7 +1119,7 @@ print(json.dumps(config)) # fact that LDSHARED is set in the environment, therefore we export # the variable only if the new value is different from what we got # from the sysconfigdata file: - if config_link != new_link and not is_windows: + if config_link != new_link and sys.platform != "win32": env.set(link_var, new_link) def setup_dependent_run_environment(self, env, dependent_spec): @@ -1180,7 +1178,7 @@ print(json.dumps(config)) view.link(new_link_target, dst, spec=self.spec) def remove_files_from_view(self, view, merge_map): - bin_dir = self.spec.prefix.bin if not is_windows else self.spec.prefix + bin_dir = self.spec.prefix.bin if sys.platform != "win32" else self.spec.prefix for src, dst in merge_map.items(): if not path_contains_subdirectory(src, bin_dir): view.remove_file(src, dst) diff --git a/var/spack/repos/builtin/packages/re2c/package.py b/var/spack/repos/builtin/packages/re2c/package.py index 9311140f16..a135b2f381 100644 --- a/var/spack/repos/builtin/packages/re2c/package.py +++ b/var/spack/repos/builtin/packages/re2c/package.py @@ -7,8 +7,6 @@ import sys from spack.package import * -is_windows = sys.platform == "win32" - class Re2c(Package): """re2c: a free and open-source lexer generator for C and C++""" @@ -33,7 +31,7 @@ class Re2c(Package): @property def make_tool(self): - if is_windows: + if sys.platform == "win32": return ninja else: return make diff --git a/var/spack/repos/builtin/packages/scons/package.py b/var/spack/repos/builtin/packages/scons/package.py index bb5bfea725..095fa5e275 100644 --- a/var/spack/repos/builtin/packages/scons/package.py +++ b/var/spack/repos/builtin/packages/scons/package.py @@ -6,8 +6,6 @@ import sys from spack.package import * -is_windows = sys.platform == "win32" - class Scons(PythonPackage): """SCons is a software construction tool""" @@ -55,7 +53,7 @@ class Scons(PythonPackage): env.prepend_path("PYTHONPATH", self.prefix.lib.scons) def setup_dependent_package(self, module, dspec): - if is_windows: + if sys.platform == "win32": module.scons = Executable(self.spec.prefix.Scripts.scons) else: module.scons = Executable(self.spec.prefix.bin.scons) diff --git a/var/spack/repos/builtin/packages/wrf/package.py b/var/spack/repos/builtin/packages/wrf/package.py index 1318a4367d..c186d7785e 100644 --- a/var/spack/repos/builtin/packages/wrf/package.py +++ b/var/spack/repos/builtin/packages/wrf/package.py @@ -5,18 +5,16 @@ import glob import re +import sys import time from os.path import basename from subprocess import PIPE, Popen -from sys import platform, stdout from llnl.util import tty from spack.package import * -is_windows = platform == "win32" - -if not is_windows: +if sys.platform != "win32": from fcntl import F_GETFL, F_SETFL, fcntl from os import O_NONBLOCK @@ -333,7 +331,7 @@ class Wrf(Package): ) p = Popen("./configure", stdin=PIPE, stdout=PIPE, stderr=PIPE) - if not is_windows: + if sys.platform != "win32": setNonBlocking(p.stdout) setNonBlocking(p.stderr) @@ -358,7 +356,7 @@ class Wrf(Package): time.sleep(0.1) # Try to do a bit of rate limiting stallcounter += 1 continue - stdout.write(line) + sys.stdout.write(line) stallcounter = 0 outputbuf += line if "Enter selection" in outputbuf or "Compile for nesting" in outputbuf: -- cgit v1.2.3-70-g09d2