fix(stamping): skip build data spawn when --stamp=false

[alloveras]

Summary

PyWriteBuildData is dirty on every build with a cold Skyframe when outputs are not downloaded. That is, once per py_binary and py_test in the build. When stamping is off, the output is a constant, so the action spawn is pure overhead.

Root Cause Analysis

The output is declared with _py_builtins.declare_constant_metadata_file. Constant metadata always reports itself as unchanged, so Bazel cannot validate it from the action cache. It stats the output on disk instead.

Under --remote_download_outputs=minimal the output is not on disk. The stat fails and the action is dirty. It re-executes, hits the remote cache, is still not downloaded, and is dirty again on the next cold-server build. Setting --remote_download_regex to match the file avoids this, at the cost of downloading it, which confirms the stat is the trigger.

The use of a constant metadata file is justified (as per the comment in the code). With stamping on, the action reads volatile-status.txt, which changes every build. The build data file is in the binary's runfiles, so without it its consumers would invalidate on every build due to timestamps.

However, with stamping off, there is no volatile input. There is nothing to shield. Unfortunately, the current implementation keeps the additional cost (constant metadata being always unchanged plus failed stat syscalls) for none of the benefits.

Change

When is_stamping_enabled(ctx) is false, the content of the file is known at analysis time:

TARGET <label>
STAMPED FALSE

So declare an ordinary output and write it with ctx.actions.write. No spawn, and no constant metadata output.

Note this is broader than --nostamp. is_stamping_enabled is also false for the exec configuration and for stamp = 0, so tool-config binaries benefit from the fix even when --stamp=true is set at the top level.

The stamped path is unchanged and keeps its constant metadata output.

Compatibility

The output is byte-identical. The label is written with str(ctx.label) to match "TARGET": str(ctx.label) in the spawn's env, which yields the canonical @@//pkg:target form. ctx.actions.args().add(ctx.label) renders the display form and drops the @@.

After the change, the tests in tests/build_data/build_data_test.py pass without any modifications.

Alternative considered

Keeping the spawn and declaring an ordinary output would also fix the dirtiness. Writing at analysis time is lighter, since it needs no subprocess.

Environment

rules_python 2.2.0, Bazel 9.2.0, Linux. Reproduce with --remote_download_outputs=minimal against a warm remote cache, then bazel shutdown and rebuild.

[Copilot]

🟡 Changes recommended

Address the two moderate compatibility findings.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR avoids unnecessary build-data writer spawns when stamping is disabled while preserving stamped-build behavior.

Changes:

• Writes unstamped build data directly during analysis.
• Retains constant metadata handling for stamped builds.
• Reuses the build-data output across both paths.

File summaries

File	Summary	Findings
python/private/py_executable.bzl	Updates build-data generation based on stamping status.	Two moderate findings: use str(ctx.label) for canonical labels and preserve platform-appropriate newline and final-newline behavior (2 votes; 1 vote).

Review details

Suppressed comments (1)

python/private/py_executable.bzl:1651

• This branch is not byte-identical to the old writer on all supported configurations. The old action uses str(ctx.label) (the canonical @@... form under bzlmod) and the PowerShell writer emits platform newlines, while multiline Args renders the Label display form and uses LF. Materialize the canonical label and a platform-appropriate separator, including the final newline.

            content = ctx.actions.args()
                .set_param_file_format("multiline")
                .add(ctx.label, format = "TARGET %s")
                .add("STAMPED FALSE"),

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[aignas]

LGTM, but please add a news entry to indicate what has been fixed.

Thank you for the fix!

[alloveras]

but please add a news entry to indicate what has been fixed.

I added the news entry. I copied the concise style of the existing ones but let me know if you want a bit more detail.