Skip to content

Commit

Permalink
fix(rules): remove rules_python --incompatible_python_disallow_native…
Browse files Browse the repository at this point in the history
…_rules checking (#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
#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
  • Loading branch information
rickeylev committed Oct 22, 2024
1 parent 0c0492d commit 1eebc2a
Show file tree
Hide file tree
Showing 21 changed files with 23 additions and 83 deletions.
2 changes: 2 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ buildifier:
bazel: "7.x"
environment:
RULES_PYTHON_ENABLE_PYSTAR: "1"
build_flags:
- "--config=bazel7.x"
test_flags:
# The doc check tests fail because the Starlark implementation makes the
# PyInfo and PyRuntimeInfo symbols become documented.
Expand Down
2 changes: 2 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,6 @@ build:rtd --stamp
# Some bzl files contain repos only available under bzlmod
build:rtd --enable_bzlmod

common:bazel7.x --incompatible_python_disallow_native_rules

build --lockfile_mode=update
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ A brief description of the categories of changes:
- Nothing yet

### Fixed
- Nothing yet
* (rules) Setting `--incompatible_python_disallow_native_rules` no longer
causes rules_python rules to fail.
([#2326](https://github.com/bazelbuild/rules_python/issues/2326).

### Added
- Nothing yet
Expand Down
2 changes: 2 additions & 0 deletions examples/build_file_generation/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ build --enable_runfiles
# The bzlmod version of this example is in examples/bzlmod_build_file_generation
# Once WORKSPACE support is dropped, this example can be entirely deleted.
build --experimental_enable_bzlmod=false

common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions examples/bzlmod/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ test --test_output=errors --enable_runfiles

# Windows requires these for multi-python support:
build --enable_runfiles
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions examples/bzlmod_build_file_generation/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ build --enable_runfiles
common --experimental_enable_bzlmod

coverage --java_runtime_version=remotejdk_11
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions examples/multi_python_versions/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ test --test_output=errors
build --enable_runfiles

coverage --java_runtime_version=remotejdk_11
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions examples/pip_parse/.bazelrc
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
# https://docs.bazel.build/versions/main/best-practices.html#using-the-bazelrc-file
try-import %workspace%/user.bazelrc
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions examples/pip_parse_vendored/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ build --enable_runfiles
# Vendoring requirements.bzl files isn't necessary under bzlmod
# When workspace support is dropped, this example can be removed.
build --noexperimental_enable_bzlmod
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions examples/pip_repository_annotations/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ try-import %workspace%/user.bazelrc
# This example is WORKSPACE specific. The equivalent functionality
# is in examples/bzlmod as the `whl_mods` feature.
build --experimental_enable_bzlmod=false
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions examples/py_proto_library/.bazelrc
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
# The equivalent bzlmod behavior is covered by examples/bzlmod/py_proto_library
common --noenable_bzlmod
common:bazel7.x --incompatible_python_disallow_native_rules
8 changes: 1 addition & 7 deletions gazelle/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,4 @@ build --incompatible_default_to_explicit_init_py
# Windows makes use of runfiles for some rules
build --enable_runfiles

# Do NOT implicitly create empty __init__.py files in the runfiles tree.
# By default, these are created in every directory containing Python source code
# or shared libraries, and every parent directory of those directories,
# excluding the repo root directory. With this flag set, we are responsible for
# creating (possibly empty) __init__.py files and adding them to the srcs of
# Python targets as required.
build --incompatible_default_to_explicit_init_py
common:bazel7.x --incompatible_python_disallow_native_rules
71 changes: 0 additions & 71 deletions python/private/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,15 @@ load(":cc_helper.bzl", "cc_helper")
load(":py_info.bzl", "PyInfo", "PyInfoBuilder")
load(":py_internal.bzl", "py_internal")
load(":reexports.bzl", "BuiltinPyInfo")
load(
":semantics.bzl",
"NATIVE_RULES_MIGRATION_FIX_CMD",
"NATIVE_RULES_MIGRATION_HELP_URL",
)

_testing = testing
_platform_common = platform_common
_coverage_common = coverage_common
_py_builtins = py_internal
PackageSpecificationInfo = getattr(py_internal, "PackageSpecificationInfo", None)

# Extensions without the dot
_PYTHON_SOURCE_EXTENSIONS = ["py"]

# NOTE: Must stay in sync with the value used in rules_python
_MIGRATION_TAG = "__PYTHON_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__"

def create_binary_semantics_struct(
*,
create_executable,
Expand Down Expand Up @@ -477,65 +468,3 @@ def target_platform_has_any_constraint(ctx, constraints):
if ctx.target_platform_has_constraint(constraint_value):
return True
return False

def check_native_allowed(ctx):
"""Check if the usage of the native rule is allowed.
Args:
ctx: rule context to check
"""
if not ctx.fragments.py.disallow_native_rules:
return

if _MIGRATION_TAG in ctx.attr.tags:
return

# NOTE: The main repo name is empty in *labels*, but not in
# ctx.workspace_name
is_main_repo = not bool(ctx.label.workspace_name)
if is_main_repo:
check_label = ctx.label
else:
# package_group doesn't allow @repo syntax, so we work around that
# by prefixing external repos with a fake package path. This also
# makes it easy to enable or disable all external repos.
check_label = Label("@//__EXTERNAL_REPOS__/{workspace}/{package}".format(
workspace = ctx.label.workspace_name,
package = ctx.label.package,
))
allowlist = ctx.attr._native_rules_allowlist
if allowlist:
allowed = ctx.attr._native_rules_allowlist[PackageSpecificationInfo].contains(check_label)
allowlist_help = str(allowlist.label).replace("@//", "//")
else:
allowed = False
allowlist_help = ("no allowlist specified; all disallowed; specify one " +
"with --python_native_rules_allowlist")
if not allowed:
if ctx.attr.generator_function:
generator = "{generator_function}(name={generator_name}) in {generator_location}".format(
generator_function = ctx.attr.generator_function,
generator_name = ctx.attr.generator_name,
generator_location = ctx.attr.generator_location,
)
else:
generator = "No generator (called directly in BUILD file)"

msg = (
"{target} not allowed to use native.{rule}\n" +
"Generated by: {generator}\n" +
"Allowlist: {allowlist}\n" +
"Migrate to using @rules_python, see {help_url}\n" +
"FIXCMD: {fix_cmd} --target={target} --rule={rule} " +
"--generator_name={generator_name} --location={generator_location}"
)
fail(msg.format(
target = str(ctx.label).replace("@//", "//"),
rule = _py_builtins.get_rule_name(ctx),
generator = generator,
allowlist = allowlist_help,
generator_name = ctx.attr.generator_name,
generator_location = ctx.attr.generator_location,
help_url = NATIVE_RULES_MIGRATION_HELP_URL,
fix_cmd = NATIVE_RULES_MIGRATION_FIX_CMD,
))
2 changes: 0 additions & 2 deletions python/private/py_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ load(":builders.bzl", "builders")
load(":cc_helper.bzl", "cc_helper")
load(
":common.bzl",
"check_native_allowed",
"collect_imports",
"collect_runfiles",
"create_instrumented_files_info",
Expand Down Expand Up @@ -269,7 +268,6 @@ def _get_build_info(ctx, cc_toolchain):
def _validate_executable(ctx):
if ctx.attr.python_version != "PY3":
fail("It is not allowed to use Python 2")
check_native_allowed(ctx)

def _declare_executable_file(ctx):
if target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints):
Expand Down
2 changes: 0 additions & 2 deletions python/private/py_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ load(
load(":builders.bzl", "builders")
load(
":common.bzl",
"check_native_allowed",
"collect_imports",
"collect_runfiles",
"create_instrumented_files_info",
Expand Down Expand Up @@ -70,7 +69,6 @@ def py_library_impl(ctx, *, semantics):
Returns:
A list of modern providers to propagate.
"""
check_native_allowed(ctx)
direct_sources = filter_to_py_srcs(ctx.files.srcs)

precompile_result = semantics.maybe_precompile(ctx, direct_sources)
Expand Down
1 change: 1 addition & 0 deletions tests/integration/compile_pip_requirements/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ test --test_output=errors

# Windows requires these for multi-python support:
build --enable_runfiles
common:bazel7.x --incompatible_python_disallow_native_rules
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
test --test_output=errors
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions tests/integration/ignore_root_user_error/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ test --test_output=errors

# Windows requires these for multi-python support:
build --enable_runfiles
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions tests/integration/local_toolchains/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ common --lockfile_mode=off
test --test_output=errors
# Windows requires these for multi-python support:
build --enable_runfiles
common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions tests/integration/pip_parse/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ build --enable_runfiles
# https://docs.bazel.build/versions/main/best-practices.html#using-the-bazelrc-file
try-import %workspace%/user.bazelrc

common:bazel7.x --incompatible_python_disallow_native_rules
1 change: 1 addition & 0 deletions tests/integration/py_cc_toolchain_registered/.bazelrc
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
# This aids debugging on failure
build --toolchain_resolution_debug=python
common:bazel7.x --incompatible_python_disallow_native_rules

0 comments on commit 1eebc2a

Please sign in to comment.