Skip to content
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

Adapt mypy bazel rule to work with Bazel 8 #144

Merged
merged 3 commits into from
Feb 17, 2025

Conversation

alexander-scott
Copy link
Contributor

@alexander-scott alexander-scott commented Feb 13, 2025

This PR does 3 things:

  • 7230abf -> Bump to Bazel 8
  • 2315df6 -> Remove the opt-out of Bzlmod in the CI workflows, as Bzlmod is enabled by default and WORKSPACE is disabled by default in the latest versions.
  • 05c8059 -> Adapt path to external runfile in bazel rule due to this flag flip Flip --legacy_external_runfiles to false bazelbuild/bazel#23574 in Bazel 8. As detailed in the linked issue description, $RUNFILES/<main repo>/external/<external repo>/<path> is removed in favour of $RUNFILES/<external repo>/<path>. To get to the new external location from the <main repo> path, we just need to go up one directory and the rest of the path remains correct. It might seem a bit hacky, but I am not aware of an API making it easier to get to this new location. This wiki page also illustrates the tree change quite well https://github.com/bazelbuild/bazel/wiki/Updating-the-runfiles-tree-structure:
my-program.runfiles/
    my_workspace_name/
        my-program
-       external/
-           mypy_integration/
-               mypy/
-                   mypy
    mypy_integration/
        mypy/
            mypy

Currently this script invokes mypy at the `external/{REPO_NAME}/mypy/mypy` path.

This was previously valid, but from Bazel 8 onwards it's not valid, and the external
repo can no longer be found at that path. The correct path is now
`../{REPO_NAME}/mypy/mypy`.

More details at
bazelbuild/bazel#23574

This change adapts the path to mypy.
Also pin the version in the `examples` dir to prevent it
from getting broken by new incompatible upstream
versions.
@alexander-scott alexander-scott changed the title Adapt mypy script to work with --nolegacy_external_runfiles Adapt mypy script to work with Bazel 8 Feb 16, 2025
@alexander-scott alexander-scott changed the title Adapt mypy script to work with Bazel 8 Adapt mypy bazelrule to work with Bazel 8 Feb 16, 2025
@alexander-scott alexander-scott marked this pull request as ready for review February 16, 2025 18:08
@alexander-scott alexander-scott changed the title Adapt mypy bazelrule to work with Bazel 8 Adapt mypy bazel rule to work with Bazel 8 Feb 16, 2025
@alexander-scott
Copy link
Contributor Author

Hi @alexeagle @adzenith, if you have a minute, a review would be welcome on this change :) It should help an internal repo of mine which heavily uses the bazel-mypy-integration ruleset to migrate to Bazel 8

@@ -27,7 +22,7 @@ jobs:
- uses: actions/checkout@v4
- uses: p0deje/setup-bazel@0.9.0
with:
bazelrc: common --announce_rc --color=yes --enable_bzlmod=${{ matrix.bzlmodEnabled }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine we can do some cleanup in the root module if it's no longer built with WORKSPACE, I'll take that as a follow-up

Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@alexeagle alexeagle merged commit 2714306 into bazel-contrib:main Feb 17, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants