fix(precompile): skip precompilation for directory and pyc inputs in srcs (#4113)
Precompiling `py_library`, `py_binary`, or `py_test` targets fails when
`srcs` contains directory artifacts or `.pyc` files. The precompiler
declares invalid actions on directories and drops non-`.py` sources from
default outputs.
This change:
* Expands direct sources to include Python files, `.pyc` files, and
directories.
* Skips precompilation for directories and non-Python files while
retaining them in default outputs.
* Updates `srcs` validation and documentation.
---------
Co-authored-by: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
diff --git a/news/4113.fixed.md b/news/4113.fixed.md
new file mode 100644
index 0000000..e852e1a
--- /dev/null
+++ b/news/4113.fixed.md
@@ -0,0 +1,4 @@
+(precompile) Fixed handling of directory and `.pyc` file inputs in
+{obj}`srcs` when {obj}`precompile` is enabled on {obj}`py_library`,
+{obj}`py_binary`, and {obj}`py_test` targets
+([#4113](https://github.com/bazel-contrib/rules_python/pull/4113)).
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
index 0b0dc97..72bf713 100644
--- a/python/private/BUILD.bazel
+++ b/python/private/BUILD.bazel
@@ -469,6 +469,7 @@
srcs = ["precompile.bzl"],
deps = [
":attributes",
+ ":common",
":flags",
":py_interpreter_program",
":toolchain_types",
diff --git a/python/private/attributes.bzl b/python/private/attributes.bzl
index e1e77cb..5b842f8 100644
--- a/python/private/attributes.bzl
+++ b/python/private/attributes.bzl
@@ -331,14 +331,29 @@
allow_files = True,
),
"srcs": lambda: attrb.LabelList(
- allow_files = [".py", ".py3"],
+ allow_files = True,
# Necessary for --compile_one_dependency to work.
flags = ["DIRECT_COMPILE_TIME_INPUT"],
doc = """
The list of Python source files that are processed to create the target. This
-includes all your checked-in code and may include generated source files. The
-`.py` files belong in `srcs` and library targets belong in `deps`. Other binary
-files that may be needed at run time belong in `data`.
+includes all your checked-in code and may include generated source files.
+
+Allowed file types:
+* `.py`
+* `.pyc`
+* directories
+
+Library targets belong in `deps`. Other binary files that may be needed at run
+time belong in `data`.
+
+:::{versionchanged} 2.3.2
+As an exception, empty targets in `srcs` that provide {obj}`PyInfo` are
+allowed. Ordinary library dependencies should remain in `deps`.
+:::
+
+:::{versionchanged} VERSION_NEXT_PATCH
+Allowed `.pyc` and directory inputs in `srcs`.
+:::
""",
),
"srcs_version": lambda: attrb.String(
diff --git a/python/private/common.bzl b/python/private/common.bzl
index 7e8c6de..5cff7f8 100644
--- a/python/private/common.bzl
+++ b/python/private/common.bzl
@@ -198,13 +198,21 @@
"""Convert a list of strings to comma separated value string."""
return ", ".join(sorted(values))
+def is_py_source(f):
+ """Whether the given file is considered a Python source file."""
+ return f.extension == "py"
+
def filter_to_py_srcs(srcs):
"""Filters .py files from the given list of files"""
+ return [f for f in srcs if is_py_source(f)]
- # TODO(b/203567235): Get the set of recognized extensions from
- # elsewhere, as there may be others. e.g. Bazel recognizes .py3
- # as a valid extension.
- return [f for f in srcs if f.extension == "py"]
+def filter_to_direct_sources(srcs):
+ """Filters Python sources, pyc files, and directory artifacts from srcs."""
+ return [
+ f
+ for f in srcs
+ if f.is_directory or is_py_source(f) or f.extension == "pyc"
+ ]
def collect_cc_info(ctx, extra_deps = []):
"""Collect C++ information from dependencies for Bazel.
@@ -398,7 +406,7 @@
# longer supported in `deps`.
files = target[DefaultInfo].files.to_list()
for f in files:
- if f.extension == "py":
+ if is_py_source(f):
py_info.transitive_sources.add(f)
py_info.merge_uses_shared_libraries(cc_helper.is_valid_shared_library_artifact(f))
for target in ctx.attr.pyi_deps:
diff --git a/python/private/precompile.bzl b/python/private/precompile.bzl
index 898dc3e..ceaa76c 100644
--- a/python/private/precompile.bzl
+++ b/python/private/precompile.bzl
@@ -15,7 +15,7 @@
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load(":attributes.bzl", "PrecompileAttr", "PrecompileInvalidationModeAttr", "PrecompileSourceRetentionAttr")
-load(":common.bzl", "actions_run")
+load(":common.bzl", "actions_run", "is_py_source")
load(":flags.bzl", "PrecompileFlag")
load(":py_interpreter_program.bzl", "PyInterpreterProgramInfo")
load(":toolchain_types.bzl", "EXEC_TOOLS_TOOLCHAIN_TYPE", "TARGET_TOOLCHAIN_TYPE")
@@ -98,7 +98,7 @@
file.
Returns:
- File of the generated pyc file.
+ File of the generated pyc file, or None if the source file was skipped.
"""
# Generating a file in another package is an error, so we have to skip
@@ -106,6 +106,12 @@
if ctx.label.package != src.owner.package:
return None
+ if src.is_directory:
+ return None
+
+ if not is_py_source(src):
+ return None
+
exec_tools_info = ctx.toolchains[EXEC_TOOLS_TOOLCHAIN_TYPE].exec_tools
target_toolchain = ctx.toolchains[TARGET_TOOLCHAIN_TYPE].py3_runtime
diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl
index 0e9c315..368a785 100644
--- a/python/private/py_executable.bzl
+++ b/python/private/py_executable.bzl
@@ -50,6 +50,7 @@
"create_py_info",
"create_windows_exe_launcher",
"csv",
+ "filter_to_direct_sources",
"filter_to_py_srcs",
"is_bool",
"is_windows_platform",
@@ -1198,13 +1199,13 @@
# precompiled pyc below) so the test-main validation can statically analyze
# the original source.
main_py_source = main_py
- direct_sources = filter_to_py_srcs(ctx.files.srcs)
+ direct_sources = filter_to_direct_sources(ctx.files.srcs)
precompile_result = maybe_precompile(ctx, direct_sources)
- required_py_files = precompile_result.keep_srcs
+ required_py_files = filter_to_py_srcs(precompile_result.keep_srcs)
required_pyc_files = []
implicit_pyc_files = []
- implicit_pyc_source_files = direct_sources
+ implicit_pyc_source_files = filter_to_py_srcs(direct_sources)
if ctx.attr.precompile == PrecompileAttr.ENABLED:
required_pyc_files.extend(precompile_result.pyc_files)
diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl
index 3118d3c..6282b71 100644
--- a/python/private/py_library.bzl
+++ b/python/private/py_library.bzl
@@ -34,6 +34,7 @@
"create_instrumented_files_info",
"create_output_group_info",
"create_py_info",
+ "filter_to_direct_sources",
"filter_to_py_srcs",
)
load(":common_labels.bzl", "labels")
@@ -128,19 +129,13 @@
):
continue
- found_match = False
- for file in files:
- if file.is_directory or file.extension in ("py", "py3"):
- found_match = True
- break
-
- if found_match:
+ if filter_to_direct_sources(files):
continue
fail(
("{} does not produce any py_library srcs files " +
- "(expected .py or .py3) and is not an empty target providing " +
- "PyInfo").format(
+ "(expected .py, .pyc, or directory) and is not an empty target " +
+ "providing PyInfo").format(
target.label,
),
attr = "srcs",
@@ -156,14 +151,14 @@
A list of modern providers to propagate.
"""
_validate_srcs(ctx)
- direct_sources = filter_to_py_srcs(ctx.files.srcs)
+ direct_sources = filter_to_direct_sources(ctx.files.srcs)
precompile_result = maybe_precompile(ctx, direct_sources)
- required_py_files = precompile_result.keep_srcs
+ required_py_files = filter_to_py_srcs(precompile_result.keep_srcs)
required_pyc_files = []
implicit_pyc_files = []
- implicit_pyc_source_files = direct_sources
+ implicit_pyc_source_files = filter_to_py_srcs(direct_sources)
precompile_attr = ctx.attr.precompile
precompile_flag = ctx.attr._precompile_flag[BuildSettingInfo].value
@@ -314,7 +309,7 @@
{obj}`ruleb.Rule` with the necessary settings
for creating a `py_library` rule.
"""
- builder = ruleb.Rule(
+ return ruleb.Rule(
implementation = py_library_impl,
doc = _DEFAULT_PY_LIBRARY_DOC,
exec_groups = dict(REQUIRED_EXEC_GROUP_BUILDERS),
@@ -326,13 +321,3 @@
ruleb.ToolchainType(EXEC_TOOLS_TOOLCHAIN_TYPE, mandatory = False),
],
)
- srcs_attr = builder.attrs.get("srcs")
- srcs_attr.set_allow_files(True)
- srcs_attr.set_doc(srcs_attr.doc() + """
-
-:::{versionchanged} 2.3.2
-As an exception, empty targets in `srcs` that provide {obj}`PyInfo` are
-allowed. Ordinary library dependencies should remain in `deps`.
-:::
-""")
- return builder
diff --git a/tests/base_rules/precompile/precompile_tests.bzl b/tests/base_rules/precompile/precompile_tests.bzl
index d2c1da6..63865a3 100644
--- a/tests/base_rules/precompile/precompile_tests.bzl
+++ b/tests/base_rules/precompile/precompile_tests.bzl
@@ -529,6 +529,148 @@
_tests.append(_test_precompile_enabled_succeeds)
+def _directory_impl(ctx):
+ out = ctx.actions.declare_directory(ctx.label.name)
+ ctx.actions.run_shell(
+ outputs = [out],
+ command = """\
+mkdir -p "$1"
+echo "x = 1" > "$1/foo.py"
+echo "y = 2" > "$1/bar.py"
+""",
+ arguments = [out.path],
+ mnemonic = "TestDirectory",
+ )
+ return [DefaultInfo(files = depset([out]))]
+
+_directory = rule(implementation = _directory_impl)
+
+def _test_directory_input(name):
+ rt_util.helper_target(
+ _directory,
+ name = name + "_dir.py",
+ )
+ rt_util.helper_target(
+ py_library,
+ name = name + "_subject",
+ srcs = ["lib.py", name + "_dir.py"],
+ precompile = "enabled",
+ )
+ analysis_test(
+ name = name,
+ impl = _test_directory_input_impl,
+ target = name + "_subject",
+ config_settings = _COMMON_CONFIG_SETTINGS,
+ )
+
+def _test_directory_input_impl(env, target):
+ target = env.expect.that_target(target)
+ target.default_outputs().contains_at_least_predicates([
+ matching.file_path_matches("__pycache__/lib.fakepy-45.pyc"),
+ matching.file_path_matches("/lib.py"),
+ matching.file_path_matches("/" + env.ctx.label.name + "_dir.py"),
+ ])
+ py_info = target.provider(PyInfo, factory = py_info_subject)
+ py_info.direct_pyc_files().contains_exactly([
+ "{package}/__pycache__/lib.fakepy-45.pyc",
+ ])
+ py_info.transitive_pyc_files().contains_exactly([
+ "{package}/__pycache__/lib.fakepy-45.pyc",
+ ])
+
+_tests.append(_test_directory_input)
+
+# buildifier: disable=function-docstring-header
+def _test_directory_input_succeeds(name):
+ """Verify that a `py_test` target with a directory input in srcs builds
+ and runs when precompiling is enabled.
+ """
+ _directory(
+ name = name + "_dir.py",
+ )
+ write_file(
+ name = name + "_main",
+ out = name + "_main.py",
+ content = [
+ "print('Hello from directory input test')",
+ "",
+ ],
+ )
+ py_test(
+ name = name,
+ srcs = [name + "_main.py", name + "_dir.py"],
+ main = name + "_main.py",
+ precompile = "enabled",
+ tags = ["no-pyrefly"],
+ )
+
+_tests.append(_test_directory_input_succeeds)
+
+def _test_pyc_source_input(name):
+ rt_util.helper_target(
+ write_file,
+ name = name + "_pyc",
+ out = name + "_foo.pyc",
+ content = [""],
+ )
+ rt_util.helper_target(
+ py_library,
+ name = name + "_subject",
+ srcs = ["lib.py", name + "_pyc"],
+ precompile = "enabled",
+ )
+ analysis_test(
+ name = name,
+ impl = _test_pyc_source_input_impl,
+ target = name + "_subject",
+ config_settings = _COMMON_CONFIG_SETTINGS,
+ )
+
+def _test_pyc_source_input_impl(env, target):
+ target = env.expect.that_target(target)
+ target.default_outputs().contains_at_least_predicates([
+ matching.file_path_matches("__pycache__/lib.fakepy-45.pyc"),
+ matching.file_path_matches("/lib.py"),
+ matching.file_path_matches("/" + env.ctx.label.name + "_foo.pyc"),
+ ])
+ py_info = target.provider(PyInfo, factory = py_info_subject)
+ py_info.direct_pyc_files().contains_exactly([
+ "{package}/__pycache__/lib.fakepy-45.pyc",
+ ])
+ py_info.transitive_pyc_files().contains_exactly([
+ "{package}/__pycache__/lib.fakepy-45.pyc",
+ ])
+
+_tests.append(_test_pyc_source_input)
+
+# buildifier: disable=function-docstring-header
+def _test_pyc_source_input_succeeds(name):
+ """Verify that a `py_test` target with a pyc input in srcs builds
+ and runs when precompiling is enabled.
+ """
+ write_file(
+ name = name + "_pyc",
+ out = name + "_foo.pyc",
+ content = [""],
+ )
+ write_file(
+ name = name + "_main",
+ out = name + "_main.py",
+ content = [
+ "print('Hello from pyc input test')",
+ "",
+ ],
+ )
+ py_test(
+ name = name,
+ srcs = [name + "_main.py", name + "_pyc"],
+ main = name + "_main.py",
+ precompile = "enabled",
+ tags = ["no-pyrefly"],
+ )
+
+_tests.append(_test_pyc_source_input_succeeds)
+
def runfiles_contains_at_least_predicates(runfiles, predicates):
for predicate in predicates:
runfiles.contains_predicate(predicate)
diff --git a/tests/base_rules/py_library/py_library_tests.bzl b/tests/base_rules/py_library/py_library_tests.bzl
index a3be7c4..80e3e1b 100644
--- a/tests/base_rules/py_library/py_library_tests.bzl
+++ b/tests/base_rules/py_library/py_library_tests.bzl
@@ -134,7 +134,9 @@
)
def _test_srcs_can_contain_tree_artifact_impl(env, target):
- env.expect.that_target(target).default_outputs().contains_exactly([])
+ env.expect.that_target(target).default_outputs().contains_exactly([
+ "{package}/{test_name}_tree.dir",
+ ])
_tests.append(_test_srcs_can_contain_tree_artifact)
