diff --git a/.github/matchers/pylint.json b/.github/matchers/pylint.json new file mode 100644 index 00000000..e3a6bd16 --- /dev/null +++ b/.github/matchers/pylint.json @@ -0,0 +1,32 @@ +{ + "problemMatcher": [ + { + "severity": "warning", + "pattern": [ + { + "regexp": "^([^:]+):(\\d+):(\\d+): ([A-DF-Z]\\d+): \\033\\[[\\d;]+m([^\\033]+).*$", + "file": 1, + "line": 2, + "column": 3, + "code": 4, + "message": 5 + } + ], + "owner": "pylint-warning" + }, + { + "severity": "error", + "pattern": [ + { + "regexp": "^([^:]+):(\\d+):(\\d+): (E\\d+): \\033\\[[\\d;]+m([^\\033]+).*$", + "file": 1, + "line": 2, + "column": 3, + "code": 4, + "message": 5 + } + ], + "owner": "pylint-error" + } + ] +} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0dcd29cf..efabd285 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,18 +15,23 @@ concurrency: cancel-in-progress: true jobs: - pre-commit: - name: Pre-commit checks (mypy, flake8, etc.) + lint: + name: Linters (mypy, flake8, etc.) runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 - uses: pre-commit/action@v2.0.3 - - run: pipx run nox -s check_manifest + - name: Check manifest + run: pipx run nox -s check_manifest + - name: PyLint checks + run: | + echo "::add-matcher::$GITHUB_WORKSPACE/.github/matchers/pylint.json" + pipx run nox -s pylint test: name: Test cibuildwheel on ${{ matrix.os }} - needs: pre-commit + needs: lint runs-on: ${{ matrix.os }} strategy: matrix: @@ -67,7 +72,7 @@ jobs: test-emulated: name: Test emulated cibuildwheel using qemu - needs: pre-commit + needs: lint runs-on: ubuntu-latest timeout-minutes: 180 steps: diff --git a/cibuildwheel/__main__.py b/cibuildwheel/__main__.py index d5323e69..306e73ab 100644 --- a/cibuildwheel/__main__.py +++ b/cibuildwheel/__main__.py @@ -237,7 +237,7 @@ def print_preamble(platform: str, options: Options, identifiers: List[str]) -> N print(f"Cache folder: {CIBW_CACHE_PATH}") - warnings = detect_warnings(platform=platform, options=options, identifiers=identifiers) + warnings = detect_warnings(options=options, identifiers=identifiers) if warnings: print("\nWarnings:") for warning in warnings: @@ -273,7 +273,7 @@ def get_build_identifiers( return [config.identifier for config in python_configurations] -def detect_warnings(platform: str, options: Options, identifiers: List[str]) -> List[str]: +def detect_warnings(*, options: Options, identifiers: List[str]) -> List[str]: warnings = [] # warn about deprecated {python} and {pip} diff --git a/cibuildwheel/architecture.py b/cibuildwheel/architecture.py index c6df9e36..437ff5e8 100644 --- a/cibuildwheel/architecture.py +++ b/cibuildwheel/architecture.py @@ -73,20 +73,18 @@ class Architecture(Enum): @staticmethod def all_archs(platform: PlatformName) -> "Set[Architecture]": - if platform == "linux": - return { + all_archs_map = { + "linux": { Architecture.x86_64, Architecture.i686, Architecture.aarch64, Architecture.ppc64le, Architecture.s390x, - } - elif platform == "macos": - return {Architecture.x86_64, Architecture.arm64, Architecture.universal2} - elif platform == "windows": - return {Architecture.x86, Architecture.AMD64, Architecture.ARM64} - else: - assert_never(platform) + }, + "macos": {Architecture.x86_64, Architecture.arm64, Architecture.universal2}, + "windows": {Architecture.x86, Architecture.AMD64, Architecture.ARM64}, + } + return all_archs_map[platform] @staticmethod def bitness_archs(platform: PlatformName, bitness: Literal["64", "32"]) -> "Set[Architecture]": diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 20e5aacb..bc313a7d 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -108,7 +108,9 @@ class DockerContainer: assert isinstance(self.name, str) - subprocess.run(["docker", "rm", "--force", "-v", self.name], stdout=subprocess.DEVNULL) + subprocess.run( + ["docker", "rm", "--force", "-v", self.name], stdout=subprocess.DEVNULL, check=False + ) self.name = None def copy_into(self, from_path: Path, to_path: PurePath) -> None: diff --git a/cibuildwheel/environment.py b/cibuildwheel/environment.py index ec8a8e36..ba66d6e2 100644 --- a/cibuildwheel/environment.py +++ b/cibuildwheel/environment.py @@ -66,7 +66,7 @@ class EnvironmentAssignmentRaw: def __repr__(self) -> str: return f"{self.name}: {self.value}" - def evaluated_value(self, **kwargs: Any) -> str: + def evaluated_value(self, **_: Any) -> str: return self.value diff --git a/cibuildwheel/functools_cached_property_38.py b/cibuildwheel/functools_cached_property_38.py new file mode 100644 index 00000000..e1c45ac2 --- /dev/null +++ b/cibuildwheel/functools_cached_property_38.py @@ -0,0 +1,65 @@ +from threading import RLock +from typing import Any, Callable, Generic, Optional, Type, TypeVar, overload + +__all__ = ["cached_property"] + +_NOT_FOUND = object() + +_T = TypeVar("_T") + + +class cached_property(Generic[_T]): + def __init__(self, func: Callable[[Any], _T]): + self.func = func + self.attrname: Optional[str] = None + self.__doc__ = func.__doc__ + self.lock = RLock() + + def __set_name__(self, owner: Type[Any], name: str) -> None: + if self.attrname is None: + self.attrname = name + elif name != self.attrname: + raise TypeError( + "Cannot assign the same cached_property to two different names " + f"({self.attrname!r} and {name!r})." + ) + + @overload + def __get__(self, instance: None, owner: Optional[Type[Any]] = ...) -> "cached_property[_T]": + ... + + @overload + def __get__(self, instance: object, owner: Optional[Type[Any]] = ...) -> _T: + ... + + def __get__(self, instance: Optional[object], owner: Optional[Type[Any]] = None) -> Any: + if instance is None: + return self + if self.attrname is None: + raise TypeError( + "Cannot use cached_property instance without calling __set_name__ on it." + ) + try: + cache = instance.__dict__ + except AttributeError: # not all objects have __dict__ (e.g. class defines slots) + msg = ( + f"No '__dict__' attribute on {type(instance).__name__!r} " + f"instance to cache {self.attrname!r} property." + ) + raise TypeError(msg) from None + val = cache.get(self.attrname, _NOT_FOUND) + if val is _NOT_FOUND: + with self.lock: + # check if another thread filled cache while we awaited lock + val = cache.get(self.attrname, _NOT_FOUND) + if val is _NOT_FOUND: + val = self.func(instance) + try: + cache[self.attrname] = val + except TypeError: + msg = ( + f"The '__dict__' attribute on {type(instance).__name__!r} instance " + f"does not support item assignment for caching {self.attrname!r} property." + ) + raise TypeError(msg) from None + return val diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 35791059..8478bf09 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -303,11 +303,11 @@ def build_on_docker( log.step_end() -def build(options: Options, tmp_path: Path) -> None: +def build(options: Options, tmp_path: Path) -> None: # pylint: disable=unused-argument try: # check docker is installed subprocess.run(["docker", "--version"], check=True, stdout=subprocess.DEVNULL) - except Exception: + except subprocess.CalledProcessError: print( "cibuildwheel: Docker not found. Docker is required to run Linux builds. " "If you're building on Travis CI, add `services: [docker]` to your .travis.yml." diff --git a/cibuildwheel/logger.py b/cibuildwheel/logger.py index 6a12d782..0a4be14d 100644 --- a/cibuildwheel/logger.py +++ b/cibuildwheel/logger.py @@ -158,7 +158,8 @@ class Logger: sys.stdout.flush() self.active_fold_group_name = None - def _fold_group_identifier(self, name: str) -> str: + @staticmethod + def _fold_group_identifier(name: str) -> str: """ Travis doesn't like fold groups identifiers that have spaces in. This method converts them to ascii identifiers @@ -174,17 +175,11 @@ class Logger: @property def colors(self) -> "Colors": - if self.colors_enabled: - return Colors(enabled=True) - else: - return Colors(enabled=False) + return Colors(enabled=self.colors_enabled) @property def symbols(self) -> "Symbols": - if self.unicode_enabled: - return Symbols(unicode=True) - else: - return Symbols(unicode=False) + return Symbols(unicode=self.unicode_enabled) def build_description_from_identifier(identifier: str) -> str: @@ -263,8 +258,6 @@ def file_supports_unicode(file_obj: IO[AnyStr]) -> bool: return "utf" in codec_info.name -""" -Global instance of the Logger. -""" +# Global instance of the Logger. # (there's only one stdout per-process, so a global instance is justified) log = Logger() diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 420fa2a5..a5818612 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -1,3 +1,4 @@ +import functools import os import platform import re @@ -5,7 +6,7 @@ import shutil import subprocess import sys from pathlib import Path -from typing import Any, Dict, List, NamedTuple, Sequence, Set, Tuple, cast +from typing import Dict, List, NamedTuple, Sequence, Set, Tuple, cast from filelock import FileLock @@ -108,7 +109,7 @@ def install_cpython(tmp: Path, version: str, url: str) -> Path: return installation_path / "bin" / "python3" -def install_pypy(tmp: Path, version: str, url: str) -> Path: +def install_pypy(tmp: Path, url: str) -> Path: pypy_tar_bz2 = url.rsplit("/", 1)[-1] extension = ".tar.bz2" assert pypy_tar_bz2.endswith(extension) @@ -136,7 +137,7 @@ def setup_python( if implementation_id.startswith("cp"): base_python = install_cpython(tmp, python_configuration.version, python_configuration.url) elif implementation_id.startswith("pp"): - base_python = install_pypy(tmp, python_configuration.version, python_configuration.url) + base_python = install_pypy(tmp, python_configuration.url) else: raise ValueError("Unknown Python implementation") assert base_python.exists() @@ -460,17 +461,12 @@ def build(options: Options, tmp_path: Path) -> None: # rosetta2 will provide the emulation with just the arch prefix. arch_prefix = ["arch", "-x86_64"] else: - raise RuntimeError( - "don't know how to emulate {testing_arch} on {machine_arch}" - ) + msg = f"don't know how to emulate {testing_arch} on {machine_arch}" + raise RuntimeError(msg) # define a custom 'call' function that adds the arch prefix each time - def call_with_arch(*args: PathOrStr, **kwargs: Any) -> None: - call(*arch_prefix, *args, **kwargs) - - def shell_with_arch(command: str, **kwargs: Any) -> None: - command = " ".join(arch_prefix) + " " + command - shell(command, **kwargs) + call_with_arch = functools.partial(call, *arch_prefix) + shell_with_arch = functools.partial(shell, *arch_prefix) # Use --no-download to ensure determinism by using seed libraries # built into virtualenv diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 0ef87990..75ee7f6b 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -1,3 +1,4 @@ +import functools import os import sys import traceback @@ -31,6 +32,7 @@ from .util import ( BuildSelector, DependencyConstraints, TestSelector, + cached_property, resources_dir, selector_matches, strtobool, @@ -139,7 +141,8 @@ def _dig_first(*pairs: Tuple[Mapping[str, Setting], str], ignore_empty: bool = F return value - raise KeyError(key) + last_key = pairs[-1][1] + raise KeyError(last_key) class OptionsReader: @@ -310,14 +313,16 @@ class OptionsReader: if table is None: raise ConfigOptionError(f"{name} does not accept a table") return table["sep"].join(table["item"].format(k=k, v=v) for k, v in result.items()) - elif isinstance(result, list): + + if isinstance(result, list): if sep is None: raise ConfigOptionError(f"{name} does not accept a list") return sep.join(result) - elif isinstance(result, int): + + if isinstance(result, int): return str(result) - else: - return result + + return result class Options: @@ -345,12 +350,10 @@ class Options: return None - @property + @cached_property def package_requires_python_str(self) -> Optional[str]: - if not hasattr(self, "_package_requires_python_str"): - args = self.command_line_arguments - self._package_requires_python_str = get_requires_python_str(Path(args.package_dir)) - return self._package_requires_python_str + args = self.command_line_arguments + return get_requires_python_str(Path(args.package_dir)) @property def globals(self) -> GlobalOptions: @@ -574,9 +577,7 @@ def compute_options( return options -_all_pinned_docker_images: Optional[ConfigParser] = None - - +@functools.lru_cache(maxsize=None) def _get_pinned_docker_images() -> Mapping[str, Mapping[str, str]]: """ This looks like a dict of dicts, e.g. @@ -585,13 +586,11 @@ def _get_pinned_docker_images() -> Mapping[str, Mapping[str, str]]: 'pypy_x86_64': {'manylinux2010': '...' } ... } """ - global _all_pinned_docker_images - if _all_pinned_docker_images is None: - pinned_docker_images_file = resources_dir / "pinned_docker_images.cfg" - _all_pinned_docker_images = ConfigParser() - _all_pinned_docker_images.read(pinned_docker_images_file) - return _all_pinned_docker_images + pinned_docker_images_file = resources_dir / "pinned_docker_images.cfg" + all_pinned_docker_images = ConfigParser() + all_pinned_docker_images.read(pinned_docker_images_file) + return all_pinned_docker_images def deprecated_selectors(name: str, selector: str, *, error: bool = False) -> None: diff --git a/cibuildwheel/projectfiles.py b/cibuildwheel/projectfiles.py index fece392f..8a6311de 100644 --- a/cibuildwheel/projectfiles.py +++ b/cibuildwheel/projectfiles.py @@ -23,11 +23,11 @@ class Analyzer(ast.NodeVisitor): def __init__(self) -> None: self.requires_python: Optional[str] = None - def visit(self, content: ast.AST) -> None: - for node in ast.walk(content): - for child in ast.iter_child_nodes(node): - child.parent = node # type: ignore[attr-defined] - super().visit(content) + def visit(self, node: ast.AST) -> None: + for inner_node in ast.walk(node): + for child in ast.iter_child_nodes(inner_node): + child.parent = inner_node # type: ignore[attr-defined] + super().visit(node) def visit_keyword(self, node: ast.keyword) -> None: self.generic_visit(node) @@ -46,7 +46,7 @@ def setup_py_python_requires(content: str) -> Optional[str]: analyzer = Analyzer() analyzer.visit(tree) return analyzer.requires_python or None - except Exception: + except Exception: # pylint: disable=broad-except return None diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py index 265d3747..18df677d 100644 --- a/cibuildwheel/util.py +++ b/cibuildwheel/util.py @@ -41,6 +41,20 @@ from platformdirs import user_cache_path from cibuildwheel.typing import Final, Literal, PathOrStr, PlatformName +__all__ = [ + "resources_dir", + "MANYLINUX_ARCHS", + "call", + "shell", + "format_safe", + "prepare_command", + "get_build_verbosity_extra_flags", + "read_python_configs", + "selector_matches", + "strtobool", + "cached_property", +] + resources_dir: Final = Path(__file__).parent / "resources" install_certifi_script: Final = resources_dir / "install_certifi.py" @@ -118,8 +132,9 @@ def call( def shell( - command: str, env: Optional[Dict[str, str]] = None, cwd: Optional[PathOrStr] = None + *commands: str, env: Optional[Dict[str, str]] = None, cwd: Optional[PathOrStr] = None ) -> None: + command = " ".join(commands) print(f"+ {command}") subprocess.run(command, env=env, cwd=cwd, shell=True, check=True) @@ -152,11 +167,11 @@ def format_safe(template: str, **kwargs: Any) -> str: re.VERBOSE, ) - # we use a lambda for repl to prevent re.sub interpreting backslashes - # in repl as escape sequences + # we use a function for repl to prevent re.sub interpreting backslashes + # in repl as escape sequences. result = re.sub( pattern=find_pattern, - repl=lambda _: str(value), + repl=lambda _: str(value), # pylint: disable=cell-var-from-loop string=result, ) @@ -288,18 +303,14 @@ def download(url: str, dest: Path) -> None: repeat_num = 3 for i in range(repeat_num): try: - response = urllib.request.urlopen(url, context=context) - except Exception: + with urllib.request.urlopen(url, context=context) as response: + dest.write_bytes(response.read()) + return + + except urllib.error.URLError: if i == repeat_num - 1: raise sleep(3) - continue - break - - try: - dest.write_bytes(response.read()) - finally: - response.close() class DependencyConstraints: @@ -319,6 +330,7 @@ class DependencyConstraints: specific_stem = self.base_file_path.stem + f"-python{version_parts[0]}{version_parts[1]}" specific_name = specific_stem + self.base_file_path.suffix specific_file_path = self.base_file_path.with_name(specific_name) + if specific_file_path.exists(): return specific_file_path else: @@ -477,7 +489,7 @@ def _parse_constraints_for_virtualenv( assert dependency_constraint_flags[0] == "-c" constraint_path = Path(dependency_constraint_flags[1]) assert constraint_path.exists() - with constraint_path.open() as constraint_file: + with constraint_path.open(encoding="utf-8") as constraint_file: for line in constraint_file: line = line.strip() if len(line) == 0: @@ -543,3 +555,9 @@ def virtualenv( env = os.environ.copy() env["PATH"] = os.pathsep.join(paths + [env["PATH"]]) return env + + +if sys.version_info >= (3, 8): + from functools import cached_property +else: + from .functools_cached_property_38 import cached_property diff --git a/noxfile.py b/noxfile.py index 62908c04..4479ab60 100644 --- a/noxfile.py +++ b/noxfile.py @@ -1,15 +1,19 @@ +import os import shutil import sys from pathlib import Path import nox -nox.options.sessions = ["lint", "check_manifest", "tests"] +nox.options.sessions = ["lint", "pylint", "check_manifest", "tests"] PYTHON_ALL_VERSIONS = ["3.6", "3.7", "3.8", "3.9", "3.10"] DIR = Path(__file__).parent.resolve() +if os.environ.get("CI", None): + nox.options.error_on_missing_interpreters = True + @nox.session def tests(session: nox.Session) -> None: @@ -34,6 +38,16 @@ def lint(session: nox.Session) -> None: session.run("pre-commit", "run", "--all-files", *session.posargs) +@nox.session +def pylint(session: nox.Session) -> None: + """ + Run pylint. + """ + + session.install("pylint", ".") + session.run("pylint", "cibuildwheel", *session.posargs) + + @nox.session def check_manifest(session: nox.Session) -> None: """ diff --git a/pyproject.toml b/pyproject.toml index 3e487866..6fc342a9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,3 +84,37 @@ ignore = [ "requirements-dev.txt", "noxfile.py", ] + +[tool.pylint] +master.py-version = "3.6" +master.jobs = "0" +master.fail-on = ["E", "F"] +master.fail-under = "9.8" +reports.output-format = "colorized" +messages_control.enable = [ + "useless-suppression", +] +messages_control.disable = [ + "abstract-class-instantiated", # filelock triggers this + "duplicate-code", + "fixme", + "invalid-name", + "line-too-long", + "missing-class-docstring", + "missing-function-docstring", + "missing-module-docstring", + "no-else-break", + "no-else-return", + "protected-access", + "too-few-public-methods", + "too-many-arguments", + "too-many-branches", + "too-many-instance-attributes", + "too-many-lines", + "too-many-locals", + "too-many-nested-blocks", + "too-many-return-statements", + "too-many-statements", + "unsubscriptable-object", + "wrong-import-position", +] diff --git a/setup.py b/setup.py index 70fd3b0e..621ccf27 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,6 @@ extras = { "types-pyyaml", "types-click", "types-requests", - "types-toml", ], }