From 505207c39b84c0a09ffbdf6948884df3e7a70da8 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Sep 2022 09:35:10 -0400 Subject: [PATCH 1/4] chore: drop [all] extra Signed-off-by: Henry Schreiner --- setup.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/setup.py b/setup.py index 4cd124ecf..982eb33f7 100644 --- a/setup.py +++ b/setup.py @@ -46,6 +46,4 @@ *extras["bin"], ] -extras["all"] = sum(extras.values(), []) - setup(extras_require=extras) From 929ecac7f86fb9972f3cfa304bffd097d5e2d05b Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Sep 2022 10:30:53 -0400 Subject: [PATCH 2/4] chore(bin): remove ghapi requirement Signed-off-by: Henry Schreiner --- bin/inspect_all_known_projects.py | 48 +++++++++++++++++++++---------- pyproject.toml | 1 - setup.py | 1 - 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/bin/inspect_all_known_projects.py b/bin/inspect_all_known_projects.py index 542e42b80..3f91b2b56 100755 --- a/bin/inspect_all_known_projects.py +++ b/bin/inspect_all_known_projects.py @@ -1,4 +1,17 @@ #!/usr/bin/env python3 + +""" +Check known projects for usage of requires-python. + +Usage: + + ./bin/inspect_all_known_projects.py --online=$GITHUB_TOKEN + +This will cache the results to all_known_setup.yaml; you can reprint +the results without the `--online` setting. +""" + + from __future__ import annotations import ast @@ -7,7 +20,7 @@ import click import yaml -from ghapi.core import GhApi, HTTP404NotFoundError +from github import Github, GithubException from rich import print from cibuildwheel.projectfiles import Analyzer @@ -47,33 +60,38 @@ def check_repo(name: str, contents: str) -> str: class MaybeRemote: - def __init__(self, cached_file: Path | str, *, online: bool) -> None: - self.online = online - if self.online: - self.contents: dict[str, dict[str, str | None]] = { + github: Github | None + contents: dict[str, dict[str, str | None]] + + def __init__(self, cached_file: Path | str, *, online: str | None) -> None: + if online is not None: + self.github = Github(online) + self.contents = { "setup.py": {}, "setup.cfg": {}, "pyproject.toml": {}, } else: + self.github = None with open(cached_file) as f: self.contents = yaml.safe_load(f) def get(self, repo: str, filename: str) -> str | None: - if self.online: + if self.github: try: - self.contents[filename][repo] = ( - GhApi(*repo.split("/")).get_content(filename).decode() - ) - except HTTP404NotFoundError: + gh_file = self.github.get_repo(repo).get_contents(filename) + except GithubException: self.contents[filename][repo] = None + else: + assert not isinstance(gh_file, list) + self.contents[filename][repo] = gh_file.decoded_content.decode(encoding="utf-8") + return self.contents[filename][repo] elif repo in self.contents[filename]: return self.contents[filename][repo] else: - raise RuntimeError( - f"Trying to access {repo}:{filename} and not in cache, rebuild cache" - ) + msg = f"Trying to access {repo}:{filename} and not in cache, rebuild cache" + raise RuntimeError(msg) def save(self, filename: Path | str) -> None: with open(filename, "w") as f: @@ -87,8 +105,8 @@ def on_each(self, repos: list[str]) -> Iterator[tuple[str, str, str | None]]: @click.command() -@click.option("--online", is_flag=True, help="Remember to set GITHUB_TOKEN") -def main(online: bool) -> None: +@click.option("--online", help="Set to $GITHUB_TOKEN") +def main(online: str | None) -> None: with open(DIR / "../docs/data/projects.yml") as f: known = yaml.safe_load(f) diff --git a/pyproject.toml b/pyproject.toml index 6c78cdb09..b0f04177f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,7 +67,6 @@ module = [ "pytest", # ignored in pre-commit to speed up check "bashlex", "importlib_resources", - "ghapi.*", ] ignore_missing_imports = true diff --git a/setup.py b/setup.py index 982eb33f7..b1481a9e5 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,6 @@ ], "bin": [ "click", - "ghapi", "pip-tools", "pygithub", "pyyaml", From 534cada6a5f15341b404a6cb9287974b378fa3be Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Sep 2022 13:11:46 -0400 Subject: [PATCH 3/4] chore: cleaner exception tracebacks Signed-off-by: Henry Schreiner --- bin/run_example_ci_configs.py | 6 ++++-- bin/update_pythons.py | 9 ++++++--- cibuildwheel/__main__.py | 3 ++- cibuildwheel/bashlex_eval.py | 18 ++++++++++-------- cibuildwheel/functools_cached_property_38.py | 11 ++++------- cibuildwheel/linux.py | 3 ++- cibuildwheel/logger.py | 6 ++++-- cibuildwheel/macos.py | 6 ++++-- cibuildwheel/oci_container.py | 3 ++- cibuildwheel/options.py | 15 ++++++++++----- cibuildwheel/windows.py | 3 ++- test/test_dependency_versions.py | 3 ++- test/utils.py | 6 ++++-- unit_test/main_tests/conftest.py | 3 ++- unit_test/main_tests/main_options_test.py | 3 ++- unit_test/option_prepare_test.py | 3 ++- 16 files changed, 62 insertions(+), 39 deletions(-) diff --git a/bin/run_example_ci_configs.py b/bin/run_example_ci_configs.py index b96dfd2d9..9cd28daaf 100755 --- a/bin/run_example_ci_configs.py +++ b/bin/run_example_ci_configs.py @@ -81,7 +81,8 @@ def ci_service_for_config_file(config_file): if service.name == service_name: return service - raise ValueError(f"unknown ci service for config file {config_file}") + msg = f"unknown ci service for config file {config_file}" + raise ValueError(msg) @click.command() @@ -100,7 +101,8 @@ def run_example_ci_configs(config_files=None): for config_file in config_files: service = ci_service_for_config_file(config_file) if service.name in configs_by_service: - raise Exception("You cannot specify more than one config per CI service") + msg = "You cannot specify more than one config per CI service" + raise Exception(msg) configs_by_service[service.name] = config_file if git_repo_has_changes(): diff --git a/bin/update_pythons.py b/bin/update_pythons.py index 6dac0b765..0227c361b 100755 --- a/bin/update_pythons.py +++ b/bin/update_pythons.py @@ -141,7 +141,8 @@ def update_version_windows(self, spec: Specifier) -> ConfigWinCP: releases = [r for r in releases if self.get_arch_file(r)] if not releases: - raise RuntimeError(f"PyPy Win {self.arch} not found for {spec}! {self.releases}") + msg = f"PyPy Win {self.arch} not found for {spec}! {self.releases}" + raise RuntimeError(msg) version_arch = "win32" if self.arch == "32" else "win_amd64" @@ -159,13 +160,15 @@ def update_version_windows(self, spec: Specifier) -> ConfigWinCP: def update_version_macos(self, spec: Specifier) -> ConfigMacOS: if self.arch != "64": - raise RuntimeError("Other archs not supported yet on macOS") + msg = "Other archs not supported yet on macOS" + raise RuntimeError(msg) releases = [r for r in self.releases if spec.contains(r["python_version"])] releases = sorted(releases, key=lambda r: r["pypy_version"]) # type: ignore[no-any-return] if not releases: - raise RuntimeError(f"PyPy macOS {self.arch} not found for {spec}!") + msg = f"PyPy macOS {self.arch} not found for {spec}!" + raise RuntimeError(msg) release = releases[-1] version = release["python_version"] diff --git a/cibuildwheel/__main__.py b/cibuildwheel/__main__.py index 4d1402d8c..0c80ead6e 100644 --- a/cibuildwheel/__main__.py +++ b/cibuildwheel/__main__.py @@ -149,7 +149,8 @@ def main() -> None: try: (project_dir,) = temp_dir.iterdir() except ValueError: - raise SystemExit("invalid sdist: didn't contain a single dir") from None + msg = "invalid sdist: didn't contain a single dir" + raise SystemExit(msg) from None # This is now the new package dir args.package_dir = project_dir.resolve() diff --git a/cibuildwheel/bashlex_eval.py b/cibuildwheel/bashlex_eval.py index 47e4aea3a..01093a623 100644 --- a/cibuildwheel/bashlex_eval.py +++ b/cibuildwheel/bashlex_eval.py @@ -32,7 +32,8 @@ def evaluate( command_node = bashlex.parsesingle(value) if len(command_node.parts) != 1: - raise ValueError(f'"{value}" has too many parts') + msg = f"{value!r} has too many parts" + raise ValueError(msg) value_word_node = command_node.parts[0] @@ -54,7 +55,8 @@ def evaluate_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: elif node.kind == "parameter": return evaluate_parameter_node(node, context=context) else: - raise ValueError(f'Unsupported bash construct: "{node.kind}"') + msg = f"Unsupported bash construct: {node.kind!r}" + raise ValueError(msg) def evaluate_word_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: @@ -65,10 +67,8 @@ def evaluate_word_node(node: bashlex.ast.node, context: NodeExecutionContext) -> part_value = evaluate_node(part, context=context) if part_string not in value: - raise RuntimeError( - f'bash parse failed. part "{part_string}" not found in "{value}". ' - f'Word was "{node.word}". Full input was "{context.input}"' - ) + msg = f"bash parse failed. part {part_string!r} not found in {value!r}. Word was {node.word!r}. Full input was {context.input!r}" + raise RuntimeError(msg) value = value.replace(part_string, part_value, 1) @@ -95,9 +95,11 @@ def evaluate_nodes_as_compound_command( result += evaluate_command_node(node, context=context) elif node.kind == "operator": if node.op != ";": - raise ValueError(f'Unsupported bash operator: "{node.op}"') + msg = f"Unsupported bash operator: {node.op!r}" + raise ValueError(msg) else: - raise ValueError(f'Unsupported bash node in compound command: "{node.kind}"') + msg = f"Unsupported bash node in compound command: {node.kind!r}" + raise ValueError(msg) return result diff --git a/cibuildwheel/functools_cached_property_38.py b/cibuildwheel/functools_cached_property_38.py index cbe4537af..a83c40daf 100644 --- a/cibuildwheel/functools_cached_property_38.py +++ b/cibuildwheel/functools_cached_property_38.py @@ -21,10 +21,8 @@ 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})." - ) + msg = "Cannot assign the same cached_property to two different names ({self.attrname!r} and {name!r})." + raise TypeError(msg) @overload def __get__(self, instance: None, owner: type[Any] | None = ...) -> cached_property[_T]: @@ -38,9 +36,8 @@ def __get__(self, instance: object | None, owner: type[Any] | None = None) -> An if instance is None: return self if self.attrname is None: - raise TypeError( - "Cannot use cached_property instance without calling __set_name__ on it." - ) + msg = "Cannot use cached_property instance without calling __set_name__ on it." + raise TypeError(msg) try: cache = instance.__dict__ except AttributeError: # not all objects have __dict__ (e.g. class defines slots) diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 5720074dd..a64c829e1 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -367,7 +367,8 @@ def build(options: Options, tmp_path: Path) -> None: # pylint: disable=unused-a cwd = Path.cwd() abs_package_dir = options.globals.package_dir.resolve() if cwd != abs_package_dir and cwd not in abs_package_dir.parents: - raise Exception("package_dir must be inside the working directory") + msg = "package_dir must be inside the working directory" + raise Exception(msg) container_project_path = PurePosixPath("/project") container_package_dir = container_project_path / abs_package_dir.relative_to(cwd) diff --git a/cibuildwheel/logger.py b/cibuildwheel/logger.py index 6f408b474..95bd97911 100644 --- a/cibuildwheel/logger.py +++ b/cibuildwheel/logger.py @@ -197,14 +197,16 @@ def build_description_from_identifier(identifier: str) -> str: elif python_interpreter == "pp": build_description += "PyPy" else: - raise Exception("unknown python") + msg = "unknown python {python_interpreter!r}" + raise Exception(msg) build_description += f" {python_version[0]}.{python_version[1:]} " try: build_description += PLATFORM_IDENTIFIER_DESCRIPTIONS[platform_identifier] except KeyError as e: - raise Exception("unknown platform") from e + msg = f"unknown platform {platform_identifier!r}" + raise Exception(msg) from e return build_description diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index f15a04f87..db863504e 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -146,7 +146,8 @@ def setup_python( elif implementation_id.startswith("pp"): base_python = install_pypy(tmp, python_configuration.url) else: - raise ValueError("Unknown Python implementation") + msg = "Unknown Python implementation" + raise ValueError(msg) assert base_python.exists() log.step("Setting up build environment...") @@ -466,7 +467,8 @@ def build(options: Options, tmp_path: Path) -> None: ) ) else: - raise RuntimeError("unreachable") + msg = "unreachable" + raise RuntimeError(msg) # skip this test continue diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 91714c7e1..c0c0cc9d6 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -58,7 +58,8 @@ def __init__( engine: ContainerEngine = "docker", ): if not image: - raise ValueError("Must have a non-empty image to run.") + msg = "Must have a non-empty image to run." + raise ValueError(msg) self.image = image self.simulate_32_bit = simulate_32_bit diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 75e367c6d..6c08de42f 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -137,7 +137,8 @@ def _dig_first(*pairs: tuple[Mapping[str, Setting], str], ignore_empty: bool = F _dig_first((dict1, "key1"), (dict2, "key2"), ...) """ if not pairs: - raise ValueError("pairs cannot be empty") + msg = "pairs cannot be empty" + raise ValueError(msg) for dict_like, key in pairs: if key in dict_like: @@ -207,13 +208,15 @@ def __init__( if config_overrides is not None: if not isinstance(config_overrides, list): - raise ConfigOptionError("'tool.cibuildwheel.overrides' must be a list") + msg = "'tool.cibuildwheel.overrides' must be a list" + raise ConfigOptionError(msg) for config_override in config_overrides: select = config_override.pop("select", None) if not select: - raise ConfigOptionError("'select' must be set in an override") + msg = "'select' must be set in an override" + raise ConfigOptionError(msg) if isinstance(select, list): select = " ".join(select) @@ -327,14 +330,16 @@ def get( if isinstance(result, dict): if table is None: - raise ConfigOptionError(f"{name!r} does not accept a table") + msg = f"{name!r} does not accept a table" + raise ConfigOptionError(msg) return table["sep"].join( item for k, v in result.items() for item in _inner_fmt(k, v, table["item"]) ) if isinstance(result, list): if sep is None: - raise ConfigOptionError(f"{name!r} does not accept a list") + msg = f"{name!r} does not accept a list" + raise ConfigOptionError(msg) return sep.join(result) if isinstance(result, int): diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 06bdc8b86..9e7dfff54 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -137,7 +137,8 @@ def setup_python( assert python_configuration.url is not None base_python = install_pypy(tmp, python_configuration.arch, python_configuration.url) else: - raise ValueError("Unknown Python implementation") + msg = "Unknown Python implementation" + raise ValueError(msg) assert base_python.exists() log.step("Setting up build environment...") diff --git a/test/test_dependency_versions.py b/test/test_dependency_versions.py index a4616a768..d447706b8 100644 --- a/test/test_dependency_versions.py +++ b/test/test_dependency_versions.py @@ -108,7 +108,8 @@ def test_pinned_versions(tmp_path, python_version, build_frontend_env): w for w in utils.expected_wheels("spam", "0.1.0") if "-cp39" in w or "-pp39" in w ] else: - raise ValueError("unhandled python version") + msg = "unhandled python version" + raise ValueError(msg) assert set(actual_wheels) == set(expected_wheels) diff --git a/test/utils.py b/test/utils.py index 5d6105164..c0f6b1269 100644 --- a/test/utils.py +++ b/test/utils.py @@ -23,7 +23,8 @@ elif sys.platform in ["win32", "cygwin"]: platform = "windows" else: - raise Exception("Unsupported platform") + msg = f"Unsupported platform {sys.platform!r}" + raise Exception(msg) def cibuildwheel_get_build_identifiers(project_path, env=None, *, prerelease_pythons=False): @@ -210,7 +211,8 @@ def expected_wheels( ) else: - raise Exception("unsupported platform") + msg = f"Unsupported platform {platform!r}" + raise Exception(msg) for platform_tag in platform_tags: wheels.append(f"{package_name}-{package_version}-{python_abi_tag}-{platform_tag}.whl") diff --git a/unit_test/main_tests/conftest.py b/unit_test/main_tests/conftest.py index c90fb5551..1c7aac4a6 100644 --- a/unit_test/main_tests/conftest.py +++ b/unit_test/main_tests/conftest.py @@ -31,7 +31,8 @@ def mock_protection(monkeypatch): """ def fail_on_call(*args, **kwargs): - raise RuntimeError("This should never be called") + msg = "This should never be called" + raise RuntimeError(msg) def ignore_call(*args, **kwargs): pass diff --git a/unit_test/main_tests/main_options_test.py b/unit_test/main_tests/main_options_test.py index 2ca1b25d5..2a34ad0b0 100644 --- a/unit_test/main_tests/main_options_test.py +++ b/unit_test/main_tests/main_options_test.py @@ -126,7 +126,8 @@ def get_default_repair_command(platform): elif platform == "windows": return "" else: - raise ValueError("Unknown platform", platform) + msg = f"Unknown platform: {platform!r}" + raise ValueError(msg) @pytest.mark.parametrize("repair_command", [None, "repair", "repair -w {dest_dir} {wheel}"]) diff --git a/unit_test/option_prepare_test.py b/unit_test/option_prepare_test.py index 9b21d3295..20beea4c8 100644 --- a/unit_test/option_prepare_test.py +++ b/unit_test/option_prepare_test.py @@ -19,7 +19,8 @@ @pytest.fixture def mock_build_container(monkeypatch): def fail_on_call(*args, **kwargs): - raise RuntimeError("This should never be called") + msg = "This should never be called" + raise RuntimeError(msg) def ignore_call(*args, **kwargs): pass From c302a1d7f9ae4037d3ee52a1b0c1d1d4c99d2e10 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 13 Sep 2022 08:14:41 -0400 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Joe Rickerby --- cibuildwheel/functools_cached_property_38.py | 2 +- cibuildwheel/logger.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/functools_cached_property_38.py b/cibuildwheel/functools_cached_property_38.py index a83c40daf..879cdb620 100644 --- a/cibuildwheel/functools_cached_property_38.py +++ b/cibuildwheel/functools_cached_property_38.py @@ -21,7 +21,7 @@ def __set_name__(self, owner: type[Any], name: str) -> None: if self.attrname is None: self.attrname = name elif name != self.attrname: - msg = "Cannot assign the same cached_property to two different names ({self.attrname!r} and {name!r})." + msg = f"Cannot assign the same cached_property to two different names ({self.attrname!r} and {name!r})." raise TypeError(msg) @overload diff --git a/cibuildwheel/logger.py b/cibuildwheel/logger.py index 95bd97911..fe44e5f47 100644 --- a/cibuildwheel/logger.py +++ b/cibuildwheel/logger.py @@ -197,7 +197,7 @@ def build_description_from_identifier(identifier: str) -> str: elif python_interpreter == "pp": build_description += "PyPy" else: - msg = "unknown python {python_interpreter!r}" + msg = f"unknown python {python_interpreter!r}" raise Exception(msg) build_description += f" {python_version[0]}.{python_version[1:]} "