-
Notifications
You must be signed in to change notification settings - Fork 26
ci: require min-version bumps for co-changed internal packages #1694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+372
−1
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| #!/usr/bin/env python3 | ||
| """Enforce minimum-version bumps between co-changed internal packages. | ||
|
|
||
| The monorepo ships several packages that depend on one another | ||
| (``uipath`` -> ``uipath-platform`` -> ``uipath-core``). When a PR changes | ||
| the *source* of a dependency package (say ``uipath-core``) **and** the | ||
| source of one of its dependents (say ``uipath``), the dependent is almost | ||
| certainly relying on the new behaviour. If the dependent does not also | ||
| raise the lower bound of its requirement on the dependency, then anyone who | ||
| installs the dependent on its own can resolve an older dependency that | ||
| predates the new behaviour — a silent runtime break. | ||
|
|
||
| This check fails such a PR. For every pair of co-changed (dependency, | ||
| dependent) packages it requires the dependent's lower-bound constraint on | ||
| the dependency (the ``>=`` part of e.g. ``uipath-core>=0.5.8, <0.6.0``) to | ||
| be at least the dependency's new version declared in this PR. | ||
|
|
||
| The internal dependency graph is discovered from the pyproject files, so no | ||
| hard-coded list needs maintaining as packages are added. | ||
| """ | ||
|
|
||
| import re | ||
| import sys | ||
| from pathlib import Path | ||
| from typing import TypedDict | ||
|
|
||
| from check_version_uniqueness import get_changed_packages | ||
|
|
||
| try: | ||
| import tomllib | ||
| except ModuleNotFoundError: | ||
| import tomli as tomllib # type: ignore[no-redef] | ||
|
|
||
| PACKAGES_DIR = Path("packages") | ||
|
|
||
|
|
||
| class PackageInfo(TypedDict): | ||
| """Resolved metadata for a single monorepo package.""" | ||
|
|
||
| dir: str | ||
| name: str | ||
| version: str | ||
| dependencies: list[str] | ||
|
|
||
|
|
||
| def normalize_name(name: str) -> str: | ||
| """Normalize a PyPI project name (PEP 503): case-insensitive, -/_/. | ||
| treated as equivalent.""" | ||
| return re.sub(r"[-_.]+", "-", name).lower() | ||
|
|
||
|
|
||
| def version_key(version: str) -> tuple[int, ...]: | ||
| """Numeric sort key so ``0.5.17`` > ``0.5.8`` (``0.5.18rc1`` -> ``(0, 5, 18)``).""" | ||
| parts: list[int] = [] | ||
| for component in version.split("."): | ||
| digits = "" | ||
| for ch in component: | ||
| if ch.isdigit(): | ||
| digits += ch | ||
| else: | ||
| break | ||
| parts.append(int(digits) if digits else 0) | ||
| return tuple(parts) | ||
|
|
||
|
|
||
| def parse_requirement(requirement: str) -> tuple[str | None, str | None]: | ||
| """Extract (normalized name, lower-bound version) from a requirement string. | ||
|
|
||
| Returns the lower bound found in a ``>=`` clause, or ``None`` if there is | ||
| no ``>=`` constraint. The name is ``None`` if the string is unparseable. | ||
| """ | ||
| name_match = re.match(r"^\s*([A-Za-z0-9][A-Za-z0-9._-]*)", requirement) | ||
| if not name_match: | ||
| return None, None | ||
| name = normalize_name(name_match.group(1)) | ||
|
|
||
| lower: str | None = None | ||
| lower_match = re.search(r">=\s*([0-9][0-9A-Za-z._-]*)", requirement) | ||
| if lower_match: | ||
| lower = lower_match.group(1) | ||
| return name, lower | ||
|
|
||
|
|
||
| def load_package(package_dir: str) -> PackageInfo | None: | ||
| """Read a package's name, version and dependency list from pyproject.toml.""" | ||
| pyproject = PACKAGES_DIR / package_dir / "pyproject.toml" | ||
| if not pyproject.exists(): | ||
| return None | ||
| with open(pyproject, "rb") as f: | ||
| data = tomllib.load(f) | ||
| project = data.get("project", {}) | ||
| name = project.get("name") | ||
| version = project.get("version") | ||
| if not name or not version: | ||
| return None | ||
| return PackageInfo( | ||
| dir=package_dir, | ||
| name=name, | ||
| version=version, | ||
| dependencies=list(project.get("dependencies", [])), | ||
| ) | ||
|
|
||
|
|
||
| def get_all_packages() -> dict[str, PackageInfo]: | ||
| """Map package directory name -> package info for every package.""" | ||
| packages: dict[str, PackageInfo] = {} | ||
| if not PACKAGES_DIR.is_dir(): | ||
| return packages | ||
| for item in sorted(PACKAGES_DIR.iterdir()): | ||
| if item.is_dir() and (item / "pyproject.toml").exists(): | ||
| info = load_package(item.name) | ||
| if info: | ||
| packages[item.name] = info | ||
| return packages | ||
|
|
||
|
|
||
| def check(packages: dict[str, PackageInfo], changed: set[str]) -> list[str]: | ||
| """Return a list of violation messages (empty when the PR is compliant).""" | ||
| name_to_dir: dict[str, str] = { | ||
| normalize_name(info["name"]): pkg_dir for pkg_dir, info in packages.items() | ||
| } | ||
|
|
||
| violations: list[str] = [] | ||
| for dependent_dir in sorted(changed): | ||
| dependent = packages.get(dependent_dir) | ||
| if not dependent: | ||
| continue | ||
|
|
||
| for requirement in dependent["dependencies"]: | ||
| dep_name, lower = parse_requirement(requirement) | ||
| if dep_name is None: | ||
| continue | ||
|
|
||
| dep_dir = name_to_dir.get(dep_name) | ||
| # Only internal packages that *also* changed in this PR are in scope. | ||
| if dep_dir is None or dep_dir == dependent_dir or dep_dir not in changed: | ||
| continue | ||
|
|
||
| dep_version = packages[dep_dir]["version"] | ||
| dep_display = packages[dep_dir]["name"] | ||
|
|
||
| if lower is None: | ||
| violations.append( | ||
| f"{dependent['name']} requires '{requirement}' but has no '>=' lower bound on " | ||
| f"{dep_display}; pin it to >={dep_version} (both packages changed in this PR)." | ||
| ) | ||
| elif version_key(lower) < version_key(dep_version): | ||
| violations.append( | ||
| f"{dependent['name']} pins {dep_display}>={lower}, but {dep_display} was bumped to " | ||
| f"{dep_version} in this PR. Raise the minimum to >={dep_version}." | ||
| ) | ||
| else: | ||
| print(f"OK: {dependent['name']} requires {dep_display}>={lower} (>= new {dep_version})") | ||
|
|
||
| return violations | ||
|
|
||
|
|
||
| def main() -> int: | ||
| packages = get_all_packages() | ||
| if not packages: | ||
| print("No packages found.") | ||
| return 0 | ||
|
|
||
| changed = set(get_changed_packages()) | ||
| if not changed: | ||
| print("No source changes to internal packages detected.") | ||
| return 0 | ||
|
|
||
| print(f"Changed packages: {', '.join(sorted(changed))}") | ||
|
|
||
| violations = check(packages, changed) | ||
| if violations: | ||
| print("\nDependency version bump check FAILED:\n", file=sys.stderr) | ||
| for v in violations: | ||
| print(f" - {v}", file=sys.stderr) | ||
| print( | ||
| "\nWhen you change an internal package and a dependent of it in the same PR, " | ||
| "the dependent must require the dependency's new version so a standalone install " | ||
| "cannot resolve an older, incompatible release.", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
|
|
||
| print("\nAll co-changed internal dependencies have an up-to-date minimum version.") | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| #!/usr/bin/env python3 | ||
| """Tests for check_dependency_version_bumps.py.""" | ||
|
|
||
| from unittest import mock | ||
|
|
||
| from check_dependency_version_bumps import ( | ||
| PackageInfo, | ||
| check, | ||
| normalize_name, | ||
| parse_requirement, | ||
| version_key, | ||
| ) | ||
|
|
||
|
|
||
| def pkg(name: str, version: str, dependencies: list[str] | None = None) -> PackageInfo: | ||
| return PackageInfo( | ||
| dir=name, | ||
| name=name, | ||
| version=version, | ||
| dependencies=dependencies or [], | ||
| ) | ||
|
|
||
|
|
||
| class TestVersionKey: | ||
| def test_numeric_components(self): | ||
| assert version_key("0.5.18") == (0, 5, 18) | ||
|
|
||
| def test_compares_numerically_not_lexically(self): | ||
| # The trap a string compare would fall into: "0.5.8" > "0.5.17". | ||
| assert version_key("0.5.17") > version_key("0.5.8") | ||
|
|
||
| def test_strips_prerelease_suffix(self): | ||
| assert version_key("0.5.18rc1") == (0, 5, 18) | ||
|
|
||
|
|
||
| class TestNormalizeName: | ||
| def test_case_and_separators_equivalent(self): | ||
| assert normalize_name("UiPath_Core") == normalize_name("uipath-core") | ||
| assert normalize_name("uipath.core") == "uipath-core" | ||
|
|
||
|
|
||
| class TestParseRequirement: | ||
| def test_extracts_name_and_lower_bound(self): | ||
| assert parse_requirement("uipath-core>=0.5.8, <0.6.0") == ("uipath-core", "0.5.8") | ||
|
|
||
| def test_no_lower_bound(self): | ||
| assert parse_requirement("click") == ("click", None) | ||
| assert parse_requirement("httpx<1.0") == ("httpx", None) | ||
|
|
||
| def test_whitespace_after_operator(self): | ||
| assert parse_requirement("uipath-core >= 0.5.8") == ("uipath-core", "0.5.8") | ||
|
|
||
|
|
||
| class TestCheck: | ||
| def _packages(self) -> dict[str, PackageInfo]: | ||
| return { | ||
| "uipath-core": pkg("uipath-core", "0.5.18"), | ||
| "uipath-platform": pkg( | ||
| "uipath-platform", "0.1.60", ["uipath-core>=0.5.8, <0.6.0"] | ||
| ), | ||
| "uipath": pkg( | ||
| "uipath", | ||
| "2.10.74", | ||
| [ | ||
| "uipath-core>=0.5.8, <0.6.0", | ||
| "uipath-platform>=0.1.59, <0.2.0", | ||
| "click>=8.3.1", | ||
| ], | ||
| ), | ||
| } | ||
|
|
||
| def test_passes_when_only_dependency_changed(self): | ||
| # uipath-core changed alone -> dependents not touched, nothing to enforce. | ||
| assert check(self._packages(), {"uipath-core"}) == [] | ||
|
|
||
| def test_passes_when_only_dependent_changed(self): | ||
| assert check(self._packages(), {"uipath"}) == [] | ||
|
|
||
| def test_fails_when_co_changed_without_min_bump(self): | ||
| # uipath-core bumped to 0.5.18 but uipath still pins >=0.5.8. | ||
| violations = check(self._packages(), {"uipath-core", "uipath"}) | ||
| assert len(violations) == 1 | ||
| assert "uipath" in violations[0] | ||
| assert "0.5.18" in violations[0] | ||
|
|
||
| def test_passes_when_min_raised_to_new_version(self): | ||
| packages = self._packages() | ||
| packages["uipath"]["dependencies"] = [ | ||
| "uipath-core>=0.5.18, <0.6.0", | ||
| "click>=8.3.1", | ||
| ] | ||
| assert check(packages, {"uipath-core", "uipath"}) == [] | ||
|
|
||
| def test_passes_when_min_already_above_new_version(self): | ||
| packages = self._packages() | ||
| packages["uipath"]["dependencies"] = ["uipath-core>=0.6.0, <0.7.0"] | ||
| assert check(packages, {"uipath-core", "uipath"}) == [] | ||
|
|
||
| def test_fails_when_no_lower_bound_on_co_changed_dep(self): | ||
| packages = self._packages() | ||
| packages["uipath"]["dependencies"] = ["uipath-core"] | ||
| violations = check(packages, {"uipath-core", "uipath"}) | ||
| assert len(violations) == 1 | ||
| assert "no '>=' lower bound" in violations[0] | ||
|
|
||
| def test_external_dependencies_are_ignored(self): | ||
| # click is not an internal package, so it is never in scope. | ||
| assert check(self._packages(), {"uipath"}) == [] | ||
|
|
||
| def test_transitive_chain_each_edge_enforced(self): | ||
| # All three changed: uipath must bump core AND platform; platform must bump core. | ||
| packages = self._packages() | ||
| violations = check(packages, {"uipath-core", "uipath-platform", "uipath"}) | ||
| # uipath->core (stale), uipath->platform (0.1.59 < 0.1.60), platform->core (stale) | ||
| assert len(violations) == 3 | ||
|
|
||
| def test_no_self_reference(self): | ||
| packages = {"uipath": pkg("uipath", "2.0.0", ["uipath>=1.0.0"])} | ||
| assert check(packages, {"uipath"}) == [] | ||
|
|
||
|
|
||
| class TestMain: | ||
| def _run(self, packages: dict[str, PackageInfo], changed: list[str]) -> int: | ||
| from check_dependency_version_bumps import main | ||
|
|
||
| with ( | ||
| mock.patch("check_dependency_version_bumps.get_all_packages", return_value=packages), | ||
| mock.patch("check_dependency_version_bumps.get_changed_packages", return_value=changed), | ||
| ): | ||
| return main() | ||
|
|
||
| def test_returns_zero_when_compliant(self): | ||
| packages = { | ||
| "uipath-core": pkg("uipath-core", "0.5.18"), | ||
| "uipath": pkg("uipath", "2.0.0", ["uipath-core>=0.5.18, <0.6.0"]), | ||
| } | ||
| assert self._run(packages, ["uipath-core", "uipath"]) == 0 | ||
|
|
||
| def test_returns_one_on_violation(self): | ||
| packages = { | ||
| "uipath-core": pkg("uipath-core", "0.5.18"), | ||
| "uipath": pkg("uipath", "2.0.0", ["uipath-core>=0.5.8, <0.6.0"]), | ||
| } | ||
| assert self._run(packages, ["uipath-core", "uipath"]) == 1 | ||
|
|
||
| def test_returns_zero_when_no_changes(self): | ||
| assert self._run({"uipath-core": pkg("uipath-core", "0.5.18")}, []) == 0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| name: Check Dependency Version Bumps | ||
|
|
||
| on: | ||
| workflow_call: | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| check-dependency-bumps: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Setup Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.11' | ||
|
|
||
| - name: Enforce min-version bumps for co-changed internal packages | ||
| env: | ||
| GITHUB_EVENT_NAME: ${{ github.event_name }} | ||
| BASE_SHA: ${{ github.event.pull_request.base.sha }} | ||
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | ||
| run: python .github/scripts/check_dependency_version_bumps.py | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.