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

fix(rules): remove rules_python --incompatible_python_disallow_native_rules checking #2327

Merged
merged 12 commits into from
Oct 22, 2024

Conversation

rickeylev
Copy link
Collaborator

@rickeylev rickeylev commented Oct 22, 2024

When --incompatible_python_disallow_native_rules is enabled, all the core rules fail with
an error that rules_python should be used. This is incorrect, since the rules_python rules
are being used. What's happening is #2257
removed the magic migration tag when pystar is enabled, but the code to check the tag
was present wasn't removed. This went unnoticed because our CI doesn't set the migration
flag.

To fix, remove the validation logic entirely. If we're in the rules_python implementation,
then there is not need to perform this validation. It was just something copy/pasted from
the original code from Bazel itself.

Also update the bazelrc to always set --incompatible_python_disallow_native_rules.

Fixes #2326
Fixes #1645

@aignas
Copy link
Collaborator

aignas commented Oct 22, 2024

Should we also close #1645 since we would be testing this?

@aignas
Copy link
Collaborator

aignas commented Oct 22, 2024

This also needs a changelog item.

@rickeylev
Copy link
Collaborator Author

also fix 1645

Hrm. Bazel 6 doesn't have the flag, so setting it in the bazelrc files is problematic. I think what I'll have to do is set it as part of the CI config. I'll give that a try, but I might drop this from the PR if it ends up being a big pain.

needs changelog entry

Done

@rickeylev
Copy link
Collaborator Author

Figure out a way to get th bazelrcs updated. I changed CI to set --config=bazel7.x for Bazel 7 builds, and the bazelrcs have common:bazel7.x lines with the necessary flags.

@rickeylev rickeylev added this pull request to the merge queue Oct 22, 2024
Merged via the queue into bazelbuild:main with commit 72ddf0c Oct 22, 2024
4 checks passed
@rickeylev rickeylev deleted the fix.dont.check.native.allowed branch October 22, 2024 19:17
rickeylev added a commit to rickeylev/rules_python that referenced this pull request Oct 22, 2024
…_rules checking (bazelbuild#2327)

When --incompatible_python_disallow_native_rules is enabled, all the
core rules fail with
an error that rules_python should be used. This is incorrect, since the
rules_python rules
are being used. What's happening is
bazelbuild#2257
removed the magic migration tag when pystar is enabled, but the code to
check the tag
was present wasn't removed. This went unnoticed because our CI doesn't
set the migration
flag.

To fix, remove the validation logic entirely. If we're in the
rules_python implementation,
then there is not need to perform this validation. It was just something
copy/pasted from
the original code from Bazel itself.

Also update the bazelrc to always set
--incompatible_python_disallow_native_rules.

Fixes bazelbuild#2326
Fixes bazelbuild#1645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants