From c25868902d7bd35f8c6f6d5d0e63722321c13452 Mon Sep 17 00:00:00 2001 From: Michael Yochpaz Date: Tue, 13 Jan 2026 10:58:34 +0200 Subject: [PATCH 1/2] build(lint): consolidate linters under pre-commit Migrate all linting tools to pre-commit for unified local and CI execution. - Add .pre-commit-config.yaml with hooks for file checks, Python version consistency, EditorConfig, markdownlint-cli2, Ruff, mypy, and Mergify - Add conditional_hook.py wrapper for a graceful skip (with a warning) when requirements for linters (like Node.js for markdownlint-cli2) are not installed locally - Update Hatch lint environment with pre-commit integration - Rename .markdownlint-config.yaml to .markdownlint.yaml for markdownlint-cli2 compatibility - Update AGENTS.md and docs/develop.md with pre-commit documentation --- .github/workflows/check.yaml | 86 +---------- ...downlint-config.yaml => .markdownlint.yaml | 0 .pre-commit-config.yaml | 64 ++++++-- AGENTS.md | 14 +- docs/develop.md | 40 +++++ .../src/package_plugins/flit_core.py | 12 +- .../src/package_plugins/hooks.py | 4 +- e2e/mergify_lint.py | 40 +++-- e2e/setup_coverage.py | 4 +- .../src/package_plugins/stevedore.py | 2 +- pyproject.toml | 4 +- scripts/conditional_hook.py | 146 ++++++++++++++++++ 12 files changed, 294 insertions(+), 122 deletions(-) rename .markdownlint-config.yaml => .markdownlint.yaml (100%) create mode 100644 scripts/conditional_hook.py diff --git a/.github/workflows/check.yaml b/.github/workflows/check.yaml index b7b0eb05..33c71e1c 100644 --- a/.github/workflows/check.yaml +++ b/.github/workflows/check.yaml @@ -20,53 +20,19 @@ jobs: with: python-version: "3.11" # minimum supported lang version - - name: Install dependencies - run: python -m pip install hatch 'click!=8.3.0' - - - name: Run pre-commit hooks - run: hatch run lint:install-hooks && hatch run lint:precommit - - linter: - name: linter - runs-on: ubuntu-latest - if: ${{ !startsWith(github.ref, 'refs/tags') }} - - steps: - - uses: actions/checkout@v6 + - name: Set up Node.js (for markdownlint) + uses: actions/setup-node@v4 with: - fetch-depth: 0 + node-version: "20" - - name: Set up Python - uses: actions/setup-python@v6 - with: - python-version: "3.11" # minimum supported lang version + - name: Install Node.js dependencies + run: npm install -g markdownlint-cli2 - - name: Install dependencies + - name: Install Python dependencies run: python -m pip install hatch 'click!=8.3.0' - - name: Run - run: hatch run lint:check - - mypy: - name: mypy - runs-on: ubuntu-latest - if: ${{ !startsWith(github.ref, 'refs/tags') }} - - steps: - - uses: actions/checkout@v6 - with: - fetch-depth: 0 - - - name: Set up Python - uses: actions/setup-python@v6 - with: - python-version: "3.11" # minimum supported lang version - - - name: Install dependencies - run: python -m pip install hatch 'click!=8.3.0' - - - name: Check MyPy - run: hatch run mypy:check + - name: Run pre-commit hooks + run: hatch run lint:install-hooks && hatch run lint:precommit pkglint: name: pkglint @@ -89,22 +55,6 @@ jobs: - name: Run run: hatch run lint:pkglint - markdownlint: - # https://github.com/marketplace/actions/markdown-lint - name: markdownlint - runs-on: ubuntu-latest - if: ${{ !startsWith(github.ref, 'refs/tags') }} - steps: - - uses: actions/checkout@v6 - with: - fetch-depth: 0 - - uses: articulate/actions-markdownlint@v1.1.0 - with: - config: .markdownlint-config.yaml - # files: 'docs/**/*.md' - # ignore: node_modules - # version: 0.28.1 - docs: name: readthedocs runs-on: ubuntu-latest @@ -125,23 +75,3 @@ jobs: - name: Run run: hatch run docs:build - - super-linter: - name: super-linter - runs-on: ubuntu-latest - if: ${{ !startsWith(github.ref, 'refs/tags') }} - - steps: - - uses: actions/checkout@v6 - with: - fetch-depth: 0 - - name: Super-Linter - uses: super-linter/super-linter@v8.3.2 # x-release-please-version - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # To reuse the same Super-linter configuration that you use in the - # lint job without duplicating it, see - # https://github.com/super-linter/super-linter/blob/main/docs/run-linter-locally.md#share-environment-variables-between-environments - VALIDATE_ALL_CODEBASE: false - VALIDATE_MARKDOWN: true - VALIDATE_EDITORCONFIG: true diff --git a/.markdownlint-config.yaml b/.markdownlint.yaml similarity index 100% rename from .markdownlint-config.yaml rename to .markdownlint.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 680756d9..d27666db 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,26 +1,66 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.5.0 + rev: v6.0.0 hooks: - - id: end-of-file-fixer # Ensures single trailing newline - - id: trailing-whitespace # Removes trailing spaces - args: [--markdown-linebreak-ext=md] # Preserve markdown line breaks - - id: check-yaml # Validates YAML syntax - - id: check-merge-conflict # Prevents merge conflict markers - - id: check-toml # Validates TOML syntax + - id: end-of-file-fixer + - id: trailing-whitespace + args: [--markdown-linebreak-ext=md] + - id: check-yaml + - id: check-merge-conflict + - id: check-toml + + - repo: https://github.com/mgedmin/check-python-versions + rev: "0.24.0" + hooks: + - id: check-python-versions + args: ["--only", "pyproject.toml,.github/workflows/test.yaml"] + + - repo: https://github.com/editorconfig-checker/editorconfig-checker.python + rev: 3.2.1 + hooks: + - id: editorconfig-checker + alias: ec + # Exclude files that are auto-generated or strictly formatted elsewhere + exclude: | + (?x)^( + LICENSE| + .*\.lock| + dist/.* + )$ - repo: local hooks: - - id: hatch-lint - name: hatch lint check - entry: hatch run lint:check + - id: markdownlint + name: markdownlint + entry: python scripts/conditional_hook.py markdownlint-cli2 + args: ["--fix"] language: system - types: [python] - pass_filenames: false + types: [markdown] + require_serial: true + verbose: true + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.11.11 + hooks: + # Linter runs before formatter so fixes (like removing imports) get formatted + - id: ruff-check + args: [--fix] + types_or: [python, pyi] + - id: ruff-format + types_or: [python, pyi] + + - repo: local + hooks: - id: hatch-mypy name: hatch mypy check entry: hatch run mypy:check language: system types: [python] pass_filenames: false + + - id: mergify-lint + name: mergify lint + entry: hatch run lint:mergify + language: system + files: ^\.mergify\.yml$ + pass_filenames: false diff --git a/AGENTS.md b/AGENTS.md index 8e8df31f..16dbf87a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -74,15 +74,15 @@ hatch run lint:precommit # All linters and other pre-commit hooks ### Pre-commit Hooks -The project uses pre-commit hooks to automatically check file formatting: +Run all linters and formatters via pre-commit: -- **File endings**: Ensures all files end with a single newline -- **Whitespace**: Removes trailing whitespace -- **Syntax**: Validates YAML/TOML files -- **Conflicts**: Prevents committing merge conflict markers -- **Linters**: Runs the `mypy` and `ruff` linters +```bash +hatch run lint:precommit # Run all hooks manually +``` + +Pre-commit runs automatically on commit after installation with `hatch run lint:install-hooks`. -These run automatically on commit if installed with `hatch run lint:install-hooks`. +**Conditional hooks:** The markdownlint hook skips with a warning when Node.js is unavailable locally. In CI, missing Node.js fails the build. ## Safety and Permissions diff --git a/docs/develop.md b/docs/develop.md index ebccbbdd..8a6ebc46 100644 --- a/docs/develop.md +++ b/docs/develop.md @@ -1,5 +1,45 @@ # Developing these tools +## Pre-commit hooks + +The project uses [pre-commit](https://pre-commit.com/) to run linters and formatters automatically on each commit. This ensures consistent code quality across all contributions. + +### Setup + +Install the hooks once after cloning: + +```bash +hatch run lint:install-hooks +``` + +### Running hooks + +Hooks run automatically when you commit. To run all hooks manually: + +```bash +hatch run lint:precommit +``` + +### What the hooks check + +- **File formatting**: Trailing whitespace, final newlines, YAML/TOML syntax +- **Python**: Ruff (linting + formatting), mypy (type checking) +- **Markdown**: markdownlint (style and consistency) +- **Config validation**: EditorConfig, Mergify, Python version consistency + +### Optional: Markdown linting + +The markdownlint hook requires Node.js. If Node.js isn't installed on your system: + +- **Locally**: The hook skips with a warning—your commit proceeds normally +- **In CI**: The hook runs strictly and will fail if markdownlint fails to run + +To enable markdown linting locally, install Node.js and then: + +```bash +npm install -g markdownlint-cli2 +``` + ## Unit tests and linter The unit tests and linter now use [Hatch](https://hatch.pypa.io/) and a diff --git a/e2e/flit_core_override/src/package_plugins/flit_core.py b/e2e/flit_core_override/src/package_plugins/flit_core.py index f3186f0e..8ecb54c3 100644 --- a/e2e/flit_core_override/src/package_plugins/flit_core.py +++ b/e2e/flit_core_override/src/package_plugins/flit_core.py @@ -1,6 +1,5 @@ import logging import pathlib -import typing from packaging.requirements import Requirement from packaging.version import Version @@ -25,10 +24,15 @@ def build_wheel( # 'pip wheel'. # # https://flit.pypa.io/en/stable/bootstrap.html - logger.info('using override to build flit_core wheel in %s', sdist_root_dir) + logger.info("using override to build flit_core wheel in %s", sdist_root_dir) external_commands.run( - [str(build_env.python), '-m', 'flit_core.wheel', - '--outdir', str(ctx.wheels_build)], + [ + str(build_env.python), + "-m", + "flit_core.wheel", + "--outdir", + str(ctx.wheels_build), + ], cwd=str(sdist_root_dir), extra_environ=extra_environ, ) diff --git a/e2e/fromager_hooks/src/package_plugins/hooks.py b/e2e/fromager_hooks/src/package_plugins/hooks.py index 1bc824f4..13515266 100644 --- a/e2e/fromager_hooks/src/package_plugins/hooks.py +++ b/e2e/fromager_hooks/src/package_plugins/hooks.py @@ -47,9 +47,7 @@ def after_prebuilt_wheel( dist_version: str, wheel_filename: pathlib.Path, ) -> None: - logger.info( - f"running post build hook in {__name__} for {wheel_filename}" - ) + logger.info(f"running post build hook in {__name__} for {wheel_filename}") test_file = ctx.work_dir / "test-prebuilt.txt" logger.info(f"prebuilt-wheel hook writing to {test_file}") test_file.write_text(f"{dist_name}=={dist_version}") diff --git a/e2e/mergify_lint.py b/e2e/mergify_lint.py index 8fda6987..4319ee16 100644 --- a/e2e/mergify_lint.py +++ b/e2e/mergify_lint.py @@ -41,7 +41,7 @@ e2e_dir = pathlib.Path("e2e") # Look for CI suite scripts instead of individual test scripts ci_suite_jobs = set( - script.name[:-len(".sh")] for script in e2e_dir.glob("ci_*_suite.sh") + script.name[: -len(".sh")] for script in e2e_dir.glob("ci_*_suite.sh") ) print("found CI suite scripts:\n ", "\n ".join(sorted(ci_suite_jobs)), sep="") @@ -49,7 +49,11 @@ individual_e2e_scripts = set( script.name[len("test_") : -len(".sh")] for script in e2e_dir.glob("test_*.sh") ) -print("found individual e2e scripts:\n ", "\n ".join(sorted(individual_e2e_scripts)), sep="") +print( + "found individual e2e scripts:\n ", + "\n ".join(sorted(individual_e2e_scripts)), + sep="", +) # Remember if we should fail so we can apply all of the rules and then # exit with an error. @@ -66,23 +70,27 @@ for ci_suite_file in e2e_dir.glob("ci_*_suite.sh"): content = ci_suite_file.read_text(encoding="utf8") # Look for run_test "script_name" calls (excluding commented lines) - for line in content.split('\n'): + for line in content.split("\n"): # Skip lines that start with # (comments) stripped = line.strip() - if stripped.startswith('#'): + if stripped.startswith("#"): continue for match in re.finditer(r'run_test\s+"([^"]+)"', line): referenced_scripts.add(match.group(1)) -print("scripts referenced in CI suites:\n ", "\n ".join(sorted(referenced_scripts)), sep="") +print( + "scripts referenced in CI suites:\n ", + "\n ".join(sorted(referenced_scripts)), + sep="", +) # Find any individual e2e scripts that aren't referenced in any CI suite unreferenced_scripts = individual_e2e_scripts.difference(referenced_scripts) if unreferenced_scripts: - print(f"\nERROR: The following e2e scripts are not referenced in any CI suite:") + print("\nERROR: The following e2e scripts are not referenced in any CI suite:") for script in sorted(unreferenced_scripts): print(f" - {script}") - print(f"Please add these scripts to the appropriate CI suite script.") + print("Please add these scripts to the appropriate CI suite script.") RC = 1 else: print("✓ All individual e2e scripts are covered by CI suites!") @@ -99,15 +107,17 @@ ) ) # for macOS, only expect latest Python version and latest Rust version -expected_jobs.update(set( - str(combo).replace("'", "") - for combo in itertools.product( - python_versions[-1:], - rust_versions[-1:], - test_scripts, - ["macos-latest"], +expected_jobs.update( + set( + str(combo).replace("'", "") + for combo in itertools.product( + python_versions[-1:], + rust_versions[-1:], + test_scripts, + ["macos-latest"], + ) ) -)) +) if not expected_jobs.difference(existing_jobs): print("found rules for all expected jobs!") for job_name in sorted(expected_jobs.difference(existing_jobs)): diff --git a/e2e/setup_coverage.py b/e2e/setup_coverage.py index 6f11b3f6..4fc3a3f3 100644 --- a/e2e/setup_coverage.py +++ b/e2e/setup_coverage.py @@ -2,8 +2,9 @@ """Simple coverage setup for E2E tests.""" import pathlib -import sys import site +import sys + def setup_coverage() -> None: """Create coverage.pth file for subprocess coverage collection.""" @@ -30,5 +31,6 @@ def setup_coverage() -> None: print(f"Coverage setup complete: {cov_pth}") + if __name__ == "__main__": setup_coverage() diff --git a/e2e/stevedore_override/src/package_plugins/stevedore.py b/e2e/stevedore_override/src/package_plugins/stevedore.py index 99b35186..4135adba 100644 --- a/e2e/stevedore_override/src/package_plugins/stevedore.py +++ b/e2e/stevedore_override/src/package_plugins/stevedore.py @@ -4,7 +4,7 @@ from packaging.requirements import Requirement from packaging.version import Version -from fromager import context, sources, build_environment +from fromager import build_environment, context, sources logger = logging.getLogger(__name__) diff --git a/pyproject.toml b/pyproject.toml index 2356d2d4..6793cee1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -238,7 +238,7 @@ dependencies = [ "ruff", "packaging", "check-python-versions", - "pre-commit" + "pre-commit", ] [tool.hatch.envs.lint.scripts] check = [ @@ -257,6 +257,8 @@ pkglint = [ "twine check dist/*.tar.gz dist/*.whl", "check-python-versions --only pyproject.toml,.github/workflows/test.yaml", ] +# Local pre-commit hook scripts +mergify = "python ./e2e/mergify_lint.py" [tool.hatch.envs.e2e] description = "Run end-to-end tests." diff --git a/scripts/conditional_hook.py b/scripts/conditional_hook.py new file mode 100644 index 00000000..10863a9a --- /dev/null +++ b/scripts/conditional_hook.py @@ -0,0 +1,146 @@ +#!/usr/bin/env python3 +"""Conditional Pre-commit Hook Wrapper. + +Wraps pre-commit hook commands to allow conditional execution based on: +1. Execution context (CI vs Local) +2. Availability of runtime dependencies + +Design: +- In CI: Strict enforcement. Missing tools = configuration error (exit 1) +- Locally: Graceful degradation. Missing tools = warning + skip (exit 0) + +Usage: + python scripts/conditional_hook.py [args...] + +Examples: + python scripts/conditional_hook.py markdownlint --config .markdownlint.yaml . + python scripts/conditional_hook.py npx markdownlint-cli2 "**/*.md" +""" + +import os +import shutil +import subprocess +import sys +from typing import NoReturn + +# Environment variables that indicate CI environment +# Covers: GitHub Actions, GitLab CI, Travis, Jenkins, Azure Pipelines, CircleCI +CI_ENV_VARS = frozenset( + { + "CI", + "GITHUB_ACTIONS", + "GITLAB_CI", + "TRAVIS", + "JENKINS_URL", + "TF_BUILD", + "CIRCLECI", + "BUILDKITE", + "CODEBUILD_BUILD_ID", # AWS CodeBuild + } +) + + +def is_ci() -> bool: + """Detect if running in a CI environment.""" + return any(os.environ.get(var) for var in CI_ENV_VARS) + + +def supports_color() -> bool: + """Check if terminal supports color output.""" + if os.environ.get("NO_COLOR"): + return False + if os.environ.get("FORCE_COLOR"): + return True + return hasattr(sys.stderr, "isatty") and sys.stderr.isatty() + + +def format_message(prefix: str, message: str, color_code: str) -> str: + """Format a message with optional color.""" + if supports_color(): + reset = "\033[0m" + bold = "\033[1m" + return f"{color_code}{bold}{prefix}{reset} {color_code}{message}{reset}" + return f"{prefix} {message}" + + +def print_warning(message: str) -> None: + """Print a warning message (yellow).""" + yellow = "\033[33m" + print(format_message("⚠️ SKIPPED:", message, yellow), file=sys.stderr) + + +def print_error(message: str) -> None: + """Print an error message (red).""" + red = "\033[31m" + print(format_message("❌ CI ERROR:", message, red), file=sys.stderr) + + +def print_info(message: str) -> None: + """Print an info message (for verbose output).""" + print(f" {message}", file=sys.stderr) + + +def find_executable(name: str) -> str | None: + """Find an executable in PATH.""" + return shutil.which(name) + + +def run_tool(executable: str, args: list[str]) -> int: + """Run the tool and return its exit code.""" + cmd = [executable, *args] + try: + result = subprocess.run(cmd, check=False) + return result.returncode + except FileNotFoundError: + print_error(f"Executable '{executable}' not found during execution") + return 1 + except PermissionError: + print_error(f"Permission denied executing '{executable}'") + return 1 + except Exception as e: + print_error(f"Failed to execute '{executable}': {e}") + return 1 + + +def main() -> NoReturn: + """Main entry point.""" + if len(sys.argv) < 2: + print( + "Usage: python conditional_hook.py [args...] [files...]", + file=sys.stderr, + ) + print("\nExample:", file=sys.stderr) + print( + " python conditional_hook.py markdownlint --config .markdownlint.yaml", + file=sys.stderr, + ) + sys.exit(2) + + executable = sys.argv[1] + args = sys.argv[2:] + exe_path = find_executable(executable) + + if not exe_path: + if is_ci(): + print_error( + f"Required executable '{executable}' not found in CI environment." + ) + print_info("Ensure the CI workflow installs all required dependencies.") + print_info("For Node.js tools: actions/setup-node + npm install -g ") + sys.exit(1) + else: + if "markdownlint" in executable: + print_warning("markdownlint requires Node.js (skipping)") + print_info("To enable: Install Node.js, then run:") + print_info(" npm install -g markdownlint-cli2") + else: + print_warning(f"'{executable}' is not installed (skipping)") + print_info(f"Install '{executable}' and ensure it's in your PATH") + sys.exit(0) + + exit_code = run_tool(exe_path, args) + sys.exit(exit_code) + + +if __name__ == "__main__": + main() From 704147f720124fbec6e31eb8ac448ffdd1356c0c Mon Sep 17 00:00:00 2001 From: Michael Yochpaz Date: Sun, 18 Jan 2026 18:37:13 +0200 Subject: [PATCH 2/2] Replace `markdownlint` with `MDFormat` for Markdown linting --- .github/workflows/check.yaml | 8 -- .markdownlint.yaml | 14 --- .mdformat.toml | 9 ++ .mergify.yml | 2 +- .pre-commit-config.yaml | 15 ++- AGENTS.md | 2 +- CONTRIBUTING.md | 5 +- README.md | 18 ++-- docs/develop.md | 15 +-- docs/files.md | 32 +++---- docs/http-retry.md | 5 + docs/using.md | 174 +++++++++++++++++++---------------- scripts/conditional_hook.py | 146 ----------------------------- 13 files changed, 148 insertions(+), 297 deletions(-) delete mode 100644 .markdownlint.yaml create mode 100644 .mdformat.toml delete mode 100644 scripts/conditional_hook.py diff --git a/.github/workflows/check.yaml b/.github/workflows/check.yaml index 33c71e1c..85aaf2c1 100644 --- a/.github/workflows/check.yaml +++ b/.github/workflows/check.yaml @@ -20,14 +20,6 @@ jobs: with: python-version: "3.11" # minimum supported lang version - - name: Set up Node.js (for markdownlint) - uses: actions/setup-node@v4 - with: - node-version: "20" - - - name: Install Node.js dependencies - run: npm install -g markdownlint-cli2 - - name: Install Python dependencies run: python -m pip install hatch 'click!=8.3.0' diff --git a/.markdownlint.yaml b/.markdownlint.yaml deleted file mode 100644 index f4eab340..00000000 --- a/.markdownlint.yaml +++ /dev/null @@ -1,14 +0,0 @@ -# really, this is a rule? -commands-show-output: false - -# Don't check line length -line-length: false - -# We only publish HTML, so allow all HTML inline. -no-inline-html: false - -# Don't require specifying code language -fenced-code-language: false - -# We sometimes put URLs in docs without making them links -no-bare-urls: false diff --git a/.mdformat.toml b/.mdformat.toml new file mode 100644 index 00000000..51523622 --- /dev/null +++ b/.mdformat.toml @@ -0,0 +1,9 @@ +# Use consecutive numbering for ordered lists (1, 2, 3 instead of 1, 1, 1) +number = true + +# Preserve original line wrapping +wrap = "keep" + +[plugin.tables] +# Strip extra spaces from GFM tables (don't pad columns for alignment) +compact_tables = true diff --git a/.mergify.yml b/.mergify.yml index 596fb901..4f1db753 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -7,7 +7,7 @@ pull_request_rules: - "title~=^e2e:" - files~=pyproject.toml - files~=.github/ - - files~=.markdownlint-config.yaml + - files~=.mdformat.toml - files~=tests/ - files~=e2e/ actions: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d27666db..558f4fcf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -28,16 +28,13 @@ repos: dist/.* )$ - - repo: local + - repo: https://github.com/hukkin/mdformat + rev: 1.0.0 hooks: - - id: markdownlint - name: markdownlint - entry: python scripts/conditional_hook.py markdownlint-cli2 - args: ["--fix"] - language: system - types: [markdown] - require_serial: true - verbose: true + - id: mdformat + additional_dependencies: + - mdformat-gfm # GitHub Flavored Markdown support (tables, autolinks, etc.) + - mdformat-simple-breaks # Use --- for thematic breaks instead of 70 underscores - repo: https://github.com/astral-sh/ruff-pre-commit rev: v0.11.11 diff --git a/AGENTS.md b/AGENTS.md index 16dbf87a..0de9dffa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -82,7 +82,7 @@ hatch run lint:precommit # Run all hooks manually Pre-commit runs automatically on commit after installation with `hatch run lint:install-hooks`. -**Conditional hooks:** The markdownlint hook skips with a warning when Node.js is unavailable locally. In CI, missing Node.js fails the build. +**Markdown formatting:** The mdformat hook formats Markdown files using a pure Python formatter with GitHub Flavored Markdown support. ## Safety and Permissions diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index efe85384..ff066d75 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -23,6 +23,7 @@ Fromager thrives on practical, well-tested contributions. This guide summarizes ### Prerequisites - Python 3.11 or newer + - `hatch` for environment and task management ```bash @@ -358,7 +359,7 @@ EnvKey = typing.Annotated[str, BeforeValidator(_validate_envkey)] ### Commands | Task | Command | -| ------ | --------- | +| -- | -- | | Run tests | `hatch run test:test` | | Check code quality | `hatch run lint:check` | | Fix formatting | `hatch run lint:fix` | @@ -367,7 +368,7 @@ EnvKey = typing.Annotated[str, BeforeValidator(_validate_envkey)] ### Standards | Standard | Requirement | -| ---------- | ------------- | +| -- | -- | | Type annotations | Required for every function | | Docstrings | Required on public APIs | | Tests | Required for new behavior | diff --git a/README.md b/README.md index 52ecf236..968ba496 100644 --- a/README.md +++ b/README.md @@ -7,13 +7,13 @@ wheels from source. Fromager is designed to guarantee that: -* Every binary package you install was built from source in a reproducible environment compatible with your own. +- Every binary package you install was built from source in a reproducible environment compatible with your own. -* All dependencies are also built from source, no prebuilt binaries. +- All dependencies are also built from source, no prebuilt binaries. -* The build tools themselves are built from source, ensuring a fully transparent toolchain. +- The build tools themselves are built from source, ensuring a fully transparent toolchain. -* Builds can be customized for your needs: applying patches, adjusting compiler options, or producing build variants. +- Builds can be customized for your needs: applying patches, adjusting compiler options, or producing build variants. ## Design Principles @@ -23,9 +23,9 @@ Fromager automates the build process with sensible defaults that work for most P Fromager can also build wheels in collections, rather than individually. Managing dependencies as a unified group ensures that: -* Packages built against one another remain ABI-compatible. +- Packages built against one another remain ABI-compatible. -* All versions are resolved consistently, so the resulting wheels can be installed together without conflicts. +- All versions are resolved consistently, so the resulting wheels can be installed together without conflicts. This approach makes Fromager especially useful in Python-heavy domains like AI, where reproducibility and compatibility across complex dependency trees are essential. @@ -61,9 +61,9 @@ GITHUB_TOKEN= ## Additional docs -* [Using fromager](docs/using.md) -* [Package build customization instructions](docs/customization.md) -* [Developer instructions](docs/develop.md) +- [Using fromager](docs/using.md) +- [Package build customization instructions](docs/customization.md) +- [Developer instructions](docs/develop.md) ## What's with the name? diff --git a/docs/develop.md b/docs/develop.md index 8a6ebc46..e3330f75 100644 --- a/docs/develop.md +++ b/docs/develop.md @@ -24,22 +24,9 @@ hatch run lint:precommit - **File formatting**: Trailing whitespace, final newlines, YAML/TOML syntax - **Python**: Ruff (linting + formatting), mypy (type checking) -- **Markdown**: markdownlint (style and consistency) +- **Markdown**: mdformat (formatting with GitHub Flavored Markdown support) - **Config validation**: EditorConfig, Mergify, Python version consistency -### Optional: Markdown linting - -The markdownlint hook requires Node.js. If Node.js isn't installed on your system: - -- **Locally**: The hook skips with a warning—your commit proceeds normally -- **In CI**: The hook runs strictly and will fail if markdownlint fails to run - -To enable markdown linting locally, install Node.js and then: - -```bash -npm install -g markdownlint-cli2 -``` - ## Unit tests and linter The unit tests and linter now use [Hatch](https://hatch.pypa.io/) and a diff --git a/docs/files.md b/docs/files.md index f48f8e41..328350ca 100644 --- a/docs/files.md +++ b/docs/files.md @@ -17,10 +17,10 @@ fromager bootstrap --skip-constraints package1 package2 When `--skip-constraints` is used: -* No `constraints.txt` file will be created in the work directory -* Packages with conflicting version requirements can be built in the same collection -* The resulting wheel collection may not represent a pip-installable set of packages -* Other output files (`build-order.json`, `graph.json`) are still generated normally +- No `constraints.txt` file will be created in the work directory +- Packages with conflicting version requirements can be built in the same collection +- The resulting wheel collection may not represent a pip-installable set of packages +- Other output files (`build-order.json`, `graph.json`) are still generated normally This option is useful for building large package indexes or collections where version conflicts are acceptable. @@ -157,10 +157,10 @@ wheels-repo └── simple ``` -* The `build` sub-directoy holds temporary builds. We use it as the output directory when building the wheel because we can't predict the filename, and so using an empty directory with a name we know gives us a way to find the file and move it into the `downloads` directory after it's built -* The `downloads` sub-directory contains the wheels in `.whl` format that fromager builds combined with the pre-built wheels so we can create a local package index in `simple` -* The `prebuilt` sub-directory contains wheels that are being used as prebuilt -* The `simple` sub-directory is laid out as a simple local wheel index. +- The `build` sub-directoy holds temporary builds. We use it as the output directory when building the wheel because we can't predict the filename, and so using an empty directory with a name we know gives us a way to find the file and move it into the `downloads` directory after it's built +- The `downloads` sub-directory contains the wheels in `.whl` format that fromager builds combined with the pre-built wheels so we can create a local package index in `simple` +- The `prebuilt` sub-directory contains wheels that are being used as prebuilt +- The `simple` sub-directory is laid out as a simple local wheel index. For example, the `wheels-repo` for `stevedore` package looks as follows: @@ -207,14 +207,14 @@ work-dir ``` -* The `build-order.json` file is an output file that contains the bottom-up order in which the dependencies need to be built for a specific wheel. You can find more details in the [build-order.json documentation](https://fromager.readthedocs.io/en/latest/files.html#build-order-json) -* The `constraints.txt` is the output file, produced by fromager, showing all of the versions of the packages that are install-time dependencies of the top-level items (note: this file is not generated when using the `--skip-constraints` option) -* The `graph.json` is an output file that contains all the paths fromager can take to resolve a dependency during building the wheel. You can find more details in the [graph.json documentation](https://fromager.readthedocs.io/en/latest/files.html#graph-json) -* The `logs` sub-directory contains detailed logs for fromager's `build-sequence` command including various settings and overrides for each individual package and its dependencies whose wheel was built by fromager. Each log file also contains information about build-backend dependencies if present for a given package -* The `work-dir` also includes sub-directories for the package and its dependencies. These sub-directories include various types of requirements files including `build-backend-requirements.txt`, `build-sdists-requirements.txt`, `build-system-requirements.txt` and the general `requirements.txt`. -* Files like `build.log` which store the logs generated by pip and `build-meta.json` that stores the metadata for the build are also located in `work-dir`. -* These sub-directories also include all the other relevant information for a particular package. Each sub-directory of the package will also contain the unpacked source code of each wheel that is used for the build if `--no-cleanup` option of fromager is used. -* For example, in the above directory structure, for `simple-package-foo` requirement, we will have a subdirectory titled `simple-package-foo` that holds the unpacked source code +- The `build-order.json` file is an output file that contains the bottom-up order in which the dependencies need to be built for a specific wheel. You can find more details in the [build-order.json documentation](https://fromager.readthedocs.io/en/latest/files.html#build-order-json) +- The `constraints.txt` is the output file, produced by fromager, showing all of the versions of the packages that are install-time dependencies of the top-level items (note: this file is not generated when using the `--skip-constraints` option) +- The `graph.json` is an output file that contains all the paths fromager can take to resolve a dependency during building the wheel. You can find more details in the [graph.json documentation](https://fromager.readthedocs.io/en/latest/files.html#graph-json) +- The `logs` sub-directory contains detailed logs for fromager's `build-sequence` command including various settings and overrides for each individual package and its dependencies whose wheel was built by fromager. Each log file also contains information about build-backend dependencies if present for a given package +- The `work-dir` also includes sub-directories for the package and its dependencies. These sub-directories include various types of requirements files including `build-backend-requirements.txt`, `build-sdists-requirements.txt`, `build-system-requirements.txt` and the general `requirements.txt`. +- Files like `build.log` which store the logs generated by pip and `build-meta.json` that stores the metadata for the build are also located in `work-dir`. +- These sub-directories also include all the other relevant information for a particular package. Each sub-directory of the package will also contain the unpacked source code of each wheel that is used for the build if `--no-cleanup` option of fromager is used. +- For example, in the above directory structure, for `simple-package-foo` requirement, we will have a subdirectory titled `simple-package-foo` that holds the unpacked source code For example, the `work-dir` for `stevedore` package after `bootstrap` command looks as follows: diff --git a/docs/http-retry.md b/docs/http-retry.md index 76f1616c..c0100364 100644 --- a/docs/http-retry.md +++ b/docs/http-retry.md @@ -7,8 +7,11 @@ Fromager includes enhanced HTTP retry functionality to handle network failures, The retry system provides: - **Exponential backoff with jitter** to avoid thundering herd problems + - **Configurable retry attempts** (default: 8 retries) + - **Smart error handling** for common network issues: + - HTTP 5xx server errors (500, 502, 503, 504) - HTTP 429 rate limiting - Connection timeouts and broken connections @@ -16,7 +19,9 @@ The retry system provides: - DNS resolution failures - **GitHub API rate limit handling** with proper reset time detection + - **GitHub authentication** automatically applied for GitHub API requests via `GITHUB_TOKEN` environment variable + - **Temporary file handling** to prevent partial downloads ## Configuration diff --git a/docs/using.md b/docs/using.md index 100ce1ec..4850a449 100644 --- a/docs/using.md +++ b/docs/using.md @@ -13,26 +13,26 @@ be performed in isolation. The `bootstrap` command -* Creates an empty package repository for +- Creates an empty package repository for [wheels](https://packaging.python.org/en/latest/specifications/binary-distribution-format/) -* Downloads the [source +- Downloads the [source distributions](https://packaging.python.org/en/latest/glossary/#term-Source-Distribution-or-sdist) for the input packages and places them under `sdists-repo/downloads/`. -* Recurses through the dependencies - * Firstly, any build system dependency specified in the +- Recurses through the dependencies + - Firstly, any build system dependency specified in the pyproject.toml build-system.requires section as per [PEP517](https://peps.python.org/pep-0517) - * Secondly, any build backend dependency returned from the + - Secondly, any build backend dependency returned from the get_requires_for_build_wheel() build backend hook (PEP517 again) - * Lastly, any install-time dependencies of the project as per the + - Lastly, any install-time dependencies of the project as per the wheel's [core metadata](https://packaging.python.org/en/latest/specifications/core-metadata/) `Requires-Dist` list. -* As each wheel is built, it is placed in a [PEP503 "simple" package +- As each wheel is built, it is placed in a [PEP503 "simple" package repository](https://peps.python.org/pep-0503/) under `wheels-repo/simple`. -* The order the dependencies need to be built bottom-up is written to +- The order the dependencies need to be built bottom-up is written to `build-order.json`. Wheels are built by running `pip wheel` configured so it will only @@ -51,39 +51,46 @@ that take a lot of time to compile. For each package being bootstrapped, fromager follows these key steps: 1. **Version Resolution** - Determines the specific version to build based on: - * Version constraints and requirements specifications - * Previous bootstrap history (if available) - * Available sources (PyPI, git repositories, or prebuilt wheels) + + - Version constraints and requirements specifications + - Previous bootstrap history (if available) + - Available sources (PyPI, git repositories, or prebuilt wheels) 2. **Cache Checking** - Looks for existing wheels in multiple locations: - * Local build cache (`wheels-repo/build/`) - * Local download cache (`wheels-repo/downloads/`) - * Remote wheel server cache (if configured) + + - Local build cache (`wheels-repo/build/`) + - Local download cache (`wheels-repo/downloads/`) + - Remote wheel server cache (if configured) 3. **Source Preparation** (if no cached wheel found): - * Downloads source distribution or clones git repository - * Unpacks and applies any patches via overrides - * Prepares the source tree for building + + - Downloads source distribution or clones git repository + - Unpacks and applies any patches via overrides + - Prepares the source tree for building 4. **Build Dependencies Resolution** - Recursively processes three types of dependencies: - * **Build System** - Basic tools needed to understand the build (e.g., setuptools, poetry-core) - * **Build Backend** - Additional dependencies returned by build backend hooks - * **Build Sdist** - Dependencies specifically needed for source distribution creation + + - **Build System** - Basic tools needed to understand the build (e.g., setuptools, poetry-core) + - **Build Backend** - Additional dependencies returned by build backend hooks + - **Build Sdist** - Dependencies specifically needed for source distribution creation 5. **Build Process** - Creates the distribution: - * Builds source distribution (sdist) with any patches applied - * Builds wheel from the prepared source (unless `--sdist-only` mode) - * Updates the local wheel repository mirror + + - Builds source distribution (sdist) with any patches applied + - Builds wheel from the prepared source (unless `--sdist-only` mode) + - Updates the local wheel repository mirror 6. **Dependency Discovery** - Extracts installation dependencies from: - * Built wheel metadata (preferred method) - * Source distribution metadata (in `--sdist-only` mode) + + - Built wheel metadata (preferred method) + - Source distribution metadata (in `--sdist-only` mode) 7. **Recursive Processing** - Repeats the entire process for each discovered installation dependency 8. **Build Order Tracking** - Maintains dependency graph and build order in: - * `build-order.json` - Sequential build order for production builds - * `graph.json` - Complete dependency relationship graph + + - `build-order.json` - Sequential build order for production builds + - `graph.json` - Complete dependency relationship graph The process continues recursively until all dependencies are resolved and built, ensuring a complete bottom-up build order where each package's dependencies are @@ -101,16 +108,16 @@ fromager bootstrap --skip-constraints package1==1.0 package2==2.0 When this option is used: -* The `constraints.txt` file generation is bypassed -* Packages with conflicting version requirements can be built in the same run -* The dependency resolution and build order logic still applies to individual packages -* Other output files (`build-order.json`, `graph.json`) are generated normally +- The `constraints.txt` file generation is bypassed +- Packages with conflicting version requirements can be built in the same run +- The dependency resolution and build order logic still applies to individual packages +- Other output files (`build-order.json`, `graph.json`) are generated normally This option is useful for: -* Building large package indexes that may contain multiple versions -* Testing scenarios requiring conflicting package versions -* Creating wheel collections where pip-installability is not required +- Building large package indexes that may contain multiple versions +- Testing scenarios requiring conflicting package versions +- Creating wheel collections where pip-installability is not required **Important:** The resulting wheel collection may not be installable as a coherent set using pip. @@ -134,39 +141,46 @@ The outputs are one patched source distribution and one built wheel. The process follows these steps: 1. **Version Resolution** - Determines the exact source to build: - * Resolves the specified version against the provided source server - * Locates the source distribution URL for the target version - * Validates that the requested version is available + + - Resolves the specified version against the provided source server + - Locates the source distribution URL for the target version + - Validates that the requested version is available 2. **Source Acquisition** - Downloads and prepares the source code: - * Downloads source distribution from the specified server URL - * Saves source distribution to the sdist repository - * Logs source download location and metadata + + - Downloads source distribution from the specified server URL + - Saves source distribution to the sdist repository + - Logs source download location and metadata 3. **Source Preparation** - Prepares source for building: - * Unpacks the downloaded source distribution - * Applies any configured patches via overrides system - * Handles source code modifications (vendoring, etc.) - * Creates prepared source tree in working directory + + - Unpacks the downloaded source distribution + - Applies any configured patches via overrides system + - Handles source code modifications (vendoring, etc.) + - Creates prepared source tree in working directory 4. **Build Environment Setup** - Creates isolated build environment: - * Determines build system requirements - * Installs build dependencies into the isolated environment + + - Determines build system requirements + - Installs build dependencies into the isolated environment 5. **Source Distribution Creation** - Builds patched sdist: - * Creates new source distribution including any applied patches - * Preserves modifications made during source preparation - * Saves patched sdist to the sdist repo. + + - Creates new source distribution including any applied patches + - Preserves modifications made during source preparation + - Saves patched sdist to the sdist repo. 6. **Wheel Building** - Compiles the final wheel: - * Uses prepared source and build environment - * Applies any build-time configuration overrides - * Compiles extensions and processes package files - * Creates wheel in the wheels repo + + - Uses prepared source and build environment + - Applies any build-time configuration overrides + - Compiles extensions and processes package files + - Creates wheel in the wheels repo 7. **Post-Build Processing** - Finalizes the build: - * Runs configured post-build hooks - * Updates wheel repository mirror + + - Runs configured post-build hooks + - Updates wheel repository mirror The build command provides a focused, single-package build process suitable for individual package compilation or integration into larger build systems. @@ -185,38 +199,44 @@ for any wheels that have already been built with the current settings. For each package in the sequence: 1. **Build Order Reading** - Loads the build order file containing: - * Package names and versions to build - * Source URLs and types (PyPI, git, prebuilt) - * Dependency relationships and constraints + + - Package names and versions to build + - Source URLs and types (PyPI, git, prebuilt) + - Dependency relationships and constraints 2. **Build Status Checking** - Determines if building is needed: - * Checks local wheel repository for existing builds - * Checks remote wheel server cache (if configured) - * Skips builds if wheel exists (unless `--force` flag used) - * Validates build tags match expected values + + - Checks local wheel repository for existing builds + - Checks remote wheel server cache (if configured) + - Skips builds if wheel exists (unless `--force` flag used) + - Validates build tags match expected values 3. **Prebuilt Wheel Handling** - For packages marked as prebuilt: - * Downloads wheel from specified URL - * Runs prebuilt wheel hooks for any post-download processing - * Updates local wheel repository mirror + + - Downloads wheel from specified URL + - Runs prebuilt wheel hooks for any post-download processing + - Updates local wheel repository mirror 4. **Source-to-Wheel Build Process** - Identical to what the `build` command does, for packages requiring compilation: - * **Source Download** - Fetches source distribution from configured server - * **Source Preparation** - Unpacks source and applies patches/overrides - * **Build Environment** - Creates isolated build environment with dependencies - * **Sdist Creation** - Builds new source distribution with applied patches - * **Wheel Building** - Compiles wheel from prepared source - * **Post-build Hooks** - Runs any configured post-build processing + + - **Source Download** - Fetches source distribution from configured server + - **Source Preparation** - Unpacks source and applies patches/overrides + - **Build Environment** - Creates isolated build environment with dependencies + - **Sdist Creation** - Builds new source distribution with applied patches + - **Wheel Building** - Compiles wheel from prepared source + - **Post-build Hooks** - Runs any configured post-build processing 5. **Repository Management** - After each successful build: - * Updates local wheel repository mirror - * Makes wheels available for subsequent builds in the sequence - * Ensures proper wheel server state for dependency resolution + + - Updates local wheel repository mirror + - Makes wheels available for subsequent builds in the sequence + - Ensures proper wheel server state for dependency resolution 6. **Summary Generation** - Upon completion: - * Creates markdown and JSON summary reports - * Categorizes results (new builds, prebuilt wheels, skipped builds) - * Reports build statistics and platform-specific wheel counts + + - Creates markdown and JSON summary reports + - Categorizes results (new builds, prebuilt wheels, skipped builds) + - Reports build statistics and platform-specific wheel counts The build sequence ensures proper dependency order where each package's dependencies are available before building the package itself, enabling reliable diff --git a/scripts/conditional_hook.py b/scripts/conditional_hook.py deleted file mode 100644 index 10863a9a..00000000 --- a/scripts/conditional_hook.py +++ /dev/null @@ -1,146 +0,0 @@ -#!/usr/bin/env python3 -"""Conditional Pre-commit Hook Wrapper. - -Wraps pre-commit hook commands to allow conditional execution based on: -1. Execution context (CI vs Local) -2. Availability of runtime dependencies - -Design: -- In CI: Strict enforcement. Missing tools = configuration error (exit 1) -- Locally: Graceful degradation. Missing tools = warning + skip (exit 0) - -Usage: - python scripts/conditional_hook.py [args...] - -Examples: - python scripts/conditional_hook.py markdownlint --config .markdownlint.yaml . - python scripts/conditional_hook.py npx markdownlint-cli2 "**/*.md" -""" - -import os -import shutil -import subprocess -import sys -from typing import NoReturn - -# Environment variables that indicate CI environment -# Covers: GitHub Actions, GitLab CI, Travis, Jenkins, Azure Pipelines, CircleCI -CI_ENV_VARS = frozenset( - { - "CI", - "GITHUB_ACTIONS", - "GITLAB_CI", - "TRAVIS", - "JENKINS_URL", - "TF_BUILD", - "CIRCLECI", - "BUILDKITE", - "CODEBUILD_BUILD_ID", # AWS CodeBuild - } -) - - -def is_ci() -> bool: - """Detect if running in a CI environment.""" - return any(os.environ.get(var) for var in CI_ENV_VARS) - - -def supports_color() -> bool: - """Check if terminal supports color output.""" - if os.environ.get("NO_COLOR"): - return False - if os.environ.get("FORCE_COLOR"): - return True - return hasattr(sys.stderr, "isatty") and sys.stderr.isatty() - - -def format_message(prefix: str, message: str, color_code: str) -> str: - """Format a message with optional color.""" - if supports_color(): - reset = "\033[0m" - bold = "\033[1m" - return f"{color_code}{bold}{prefix}{reset} {color_code}{message}{reset}" - return f"{prefix} {message}" - - -def print_warning(message: str) -> None: - """Print a warning message (yellow).""" - yellow = "\033[33m" - print(format_message("⚠️ SKIPPED:", message, yellow), file=sys.stderr) - - -def print_error(message: str) -> None: - """Print an error message (red).""" - red = "\033[31m" - print(format_message("❌ CI ERROR:", message, red), file=sys.stderr) - - -def print_info(message: str) -> None: - """Print an info message (for verbose output).""" - print(f" {message}", file=sys.stderr) - - -def find_executable(name: str) -> str | None: - """Find an executable in PATH.""" - return shutil.which(name) - - -def run_tool(executable: str, args: list[str]) -> int: - """Run the tool and return its exit code.""" - cmd = [executable, *args] - try: - result = subprocess.run(cmd, check=False) - return result.returncode - except FileNotFoundError: - print_error(f"Executable '{executable}' not found during execution") - return 1 - except PermissionError: - print_error(f"Permission denied executing '{executable}'") - return 1 - except Exception as e: - print_error(f"Failed to execute '{executable}': {e}") - return 1 - - -def main() -> NoReturn: - """Main entry point.""" - if len(sys.argv) < 2: - print( - "Usage: python conditional_hook.py [args...] [files...]", - file=sys.stderr, - ) - print("\nExample:", file=sys.stderr) - print( - " python conditional_hook.py markdownlint --config .markdownlint.yaml", - file=sys.stderr, - ) - sys.exit(2) - - executable = sys.argv[1] - args = sys.argv[2:] - exe_path = find_executable(executable) - - if not exe_path: - if is_ci(): - print_error( - f"Required executable '{executable}' not found in CI environment." - ) - print_info("Ensure the CI workflow installs all required dependencies.") - print_info("For Node.js tools: actions/setup-node + npm install -g ") - sys.exit(1) - else: - if "markdownlint" in executable: - print_warning("markdownlint requires Node.js (skipping)") - print_info("To enable: Install Node.js, then run:") - print_info(" npm install -g markdownlint-cli2") - else: - print_warning(f"'{executable}' is not installed (skipping)") - print_info(f"Install '{executable}' and ensure it's in your PATH") - sys.exit(0) - - exit_code = run_tool(exe_path, args) - sys.exit(exit_code) - - -if __name__ == "__main__": - main()