-
Notifications
You must be signed in to change notification settings - Fork 8
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
Support Bazel Modules #78
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome stuff, thanks @jondo2010 ! Looking forward to getting rules_appimage bzlmod'ed!
- It looks like the Python changes are unrelated? It would be nice to keep the PR diff clean.
- I'm wondering what's a good way to test the changes in CI. Maybe have a separate CI job that enables bzlmod (not sure we need to add it as a test matrix dimension). In any case adding a test would be great.
- Also please fix the linting issues.
Added a test step and linting fixes. |
import rules_python.python.runfiles.runfiles | ||
from python.runfiles import runfiles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is actually necessary -- Originally this worked by treating the rules_python
external folder itself as a Py module, but this doesn't work with bzlmod since the external package names/folders have suffixes and stuff. So this is now "correct".
@@ -36,6 +36,9 @@ jobs: | |||
- name: Run tests | |||
run: bazel test --test_env=NO_CLEANUP=1 //... | |||
|
|||
- name: Run tests (bzlmod enabled) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll need something like (untested):
- name: Run tests (bzlmod enabled) | |
- name: Run tests (bzlmod enabled) | |
if: ${{ matrix.bazel-version != '5.x' }} |
To make CI green
No description provided.