py_binary is no longer reproducible between exec and target configs

[BarrettStephen]

🐞 bug report

Affected Rule

The issue is caused by the rule: py_binary

Is this a regression?

Yes, the previous version in which this bug was not present was: 1.8.5

Description

We use pkg_tar (1.2.0) to package up py_binary and distribute it in some cases. Sometimes these go into docker images that rules_oci hashes. After upgrading to 2.0.1, I observed that the same py_binary is going into an image built in target or exec ends up with different hashes. I narrowed this down to this commit 352f405
Where build data is being included into the py_binary now via
"CONFIG_MODE": "EXEC" if _is_tool_config(ctx) else "TARGET",

so <target>.build_data.txt is different. This is the only difference between the built py_binaries.

I can't think of a usecase for knowing your build config mode at runtime in the py_binary, and it now makes the py_binary (and anything downstream of it) unreproducible between config and breaks path mapping as well. Can this be removed? Or if its needed for some reason, put behind stamp?

🔬 Minimal Reproduction

Build a py_binary and look in bazel-out for the build_data.txt.

🌍 Your Environment

Operating System:

  
Cent OS 9
5.14.0-687.el9.x86_64
  

Output of bazel version:

  
bazel 8.7.0
  

Rules_python version:

  
2.0.1
  

Anything else relevant?

I made this patch to fix it for us

Subject: [PATCH] Remove CONFIG_MODE from build data generation

Commit 352f405 added CONFIG_MODE to the build data generation environment,
which causes the build data file to differ between exec and target
configurations. This breaks build reproducibility and causes cache misses
when the same py_binary is built in both configurations.

Since the build data file is part of the output, having CONFIG_MODE in the
action environment means the same py_binary target produces different
outputs depending on whether it's in exec or target config, defeating Bazel's
caching and reproducibility guarantees.

This removes CONFIG_MODE from the environment to restore reproducibility.
---
 python/private/py_executable.bzl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl
index c2a4a8f8..c03fb2bf 100644
--- a/python/private/py_executable.bzl
+++ b/python/private/py_executable.bzl
@@ -1565,7 +1565,8 @@ def _write_build_data(ctx):
         env = {
             # Include config mode so that binaries can detect if they're
             # being used as a build tool or not, allowing for runtime optimizations.
-            "CONFIG_MODE": "EXEC" if _is_tool_config(ctx) else "TARGET",
+            # NOTE: Disabled to avoid cache misses between exec and target configs
+            # "CONFIG_MODE": "EXEC" if _is_tool_config(ctx) else "TARGET",
             "INFO_FILE": info_file.path if info_file else "",
             "OUTPUT": build_data.path,
             # Include this so it's explicit, otherwise, one has to detect
--
2.52.0

[aignas]

+1 to put it under stamp

[BarrettStephen]

Can we put the entire PyWriteBuildData action behind stamp? In our repo, the addition of this action to every py_binary added 76,573 actions, which is huge. I'm going to patch it out locally.

[BarrettStephen]

This patch puts it behind stamp:

Subject: [PATCH] perf: Only generate build data when stamping is enabled

The PyWriteBuildData action is unconditionally executed for every py_binary
target since rules_python 1.9.0 (commit 352f405f). This adds overhead to
every Python binary build, even when stamping is disabled and the build
data is not useful.

This change makes the build data generation conditional on stamping being
enabled. When stamping is disabled:
- The PyWriteBuildData action is not executed
- No build_data.txt file is created
- The stage2 bootstrap returns an empty string from get_build_data()

This reduces unnecessary action overhead for builds that don't use stamping,
which is the majority of development builds. The additional action cost is
not worth it unless stamping is explicitly enabled.

Context: https://github.com/bazel-contrib/rules_python/issues/3793#issuecomment-4501869743
---
 python/private/py_executable.bzl            | 9 ++++++---
 python/private/stage2_bootstrap_template.py | 3 +++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl
index c03fb2bf..a9ffdc86 100644
--- a/python/private/py_executable.bzl
+++ b/python/private/py_executable.bzl
@@ -874,7 +874,7 @@ def _create_stage2_bootstrap(
         template = template,
         output = output,
         substitutions = {
-            "%build_data_file%": runfiles_root_path(ctx, build_data_file.short_path),
+            "%build_data_file%": runfiles_root_path(ctx, build_data_file.short_path) if build_data_file else "",
             "%coverage_instrumented%": str(int(ctx.configuration.coverage_enabled and ctx.coverage_instrumented())),
             "%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime),
             "%import_all%": "True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False",
@@ -1478,8 +1478,11 @@ def _get_base_runfiles_for_binary(
     runfiles_without_exe = builders.RunfilesBuilder()
     runfiles_without_exe.add(app_runfiles)
     runfiles_without_exe.add(runtime_runfiles)
-    build_data_file = _write_build_data(ctx)
-    runfiles_without_exe.add(build_data_file)
+    if is_stamping_enabled(ctx):
+        build_data_file = _write_build_data(ctx)
+        runfiles_without_exe.add(build_data_file)
+    else:
+        build_data_file = None
 
     runfiles_without_exe = runfiles_without_exe.build(ctx)
 
diff --git a/python/private/stage2_bootstrap_template.py b/python/private/stage2_bootstrap_template.py
index acc980d3..5f7baf40 100644
--- a/python/private/stage2_bootstrap_template.py
+++ b/python/private/stage2_bootstrap_template.py
@@ -64,6 +64,9 @@ class BazelBinaryInfoModule(types.ModuleType):
     @cache
     def get_build_data(self):
         """Returns a string of the raw build data."""
+        # If BUILD_DATA_FILE is empty, stamping was disabled
+        if not self.BUILD_DATA_FILE:
+            return ""
         try:
             # Prefer dep via pypi
             import runfiles
-- 
2.52.0