fix: transition --stamp only for the build data dependency

news/stamp-transition-build-data.fixed.md

@@ -0,0 +1,3 @@
+(binaries/tests) The {obj}`stamp` attribute no longer transitions the
+{flag}`--stamp` flag for the whole target, so dependencies of executables with
+`stamp = 0` or `stamp = 1` are no longer built in a separate configuration.

python/private/attributes.bzl

@@ -512,9 +512,6 @@ See the [Accessing build information docs] for more information.
 :::{warning}
 Stamping can harm build performance by reducing cache hits and should
 be avoided if possible.
-
-In addition, this transitions the {flag}`--stamp <stamp>` flag, which can additional
-config state overhead.
 :::
 
 :::{note}

python/private/py_executable.bzl

@@ -85,6 +85,21 @@ _EXTERNAL_PATH_PREFIX = "external"
 _ZIP_RUNFILES_DIRECTORY_NAME = "runfiles"
 _INIT_PY = "__init__.py"
 
+def _build_data_stamp_transition_impl(settings, attr):
+    # stamp = 1 reads the real version file even in --nostamp builds. stamp = 0
+    # never reads it, and stamp = -1 follows --stamp, so both keep the config.
+    if attr.stamp == 1:
+        return {"//command_line_option:stamp": True}
+    return {"//command_line_option:stamp": settings["//command_line_option:stamp"]}
+
+# Applies the `stamp` attribute to the build data inputs only. Transitioning
+# the whole executable would rebuild its deps in a separate configuration.
+_build_data_stamp_transition = transition(
+    implementation = _build_data_stamp_transition_impl,
+    inputs = ["//command_line_option:stamp"],
+    outputs = ["//command_line_option:stamp"],
+)
+
 # Non-Google-specific attributes for executables
 # These attributes are for rules that accept Python sources.
 EXECUTABLE_ATTRS = dicts.add(
@@ -246,6 +261,7 @@ accepting arbitrary Python versions.
         "_uncachable_version_file": lambda: attrb.Label(
             default = "//python/private:uncachable_version_file",
             allow_files = True,
+            cfg = _build_data_stamp_transition,
         ),
         "_venvs_use_declare_symlink_flag": lambda: attrb.Label(
             default = labels.VENVS_USE_DECLARE_SYMLINK,
@@ -2196,9 +2212,6 @@ def _transition_executable_impl(settings, attr):
     if attr.python_version and attr.python_version not in ("PY2", "PY3"):
         settings[labels.PYTHON_VERSION] = attr.python_version
 
-    if attr.stamp != -1:
-        settings["//command_line_option:stamp"] = str(attr.stamp)
-
     return settings
 
 def create_executable_rule(*, attrs, **kwargs):
@@ -2252,13 +2265,11 @@ def create_executable_rule_builder(implementation, **kwargs):
             implementation = _transition_executable_impl,
             inputs = TRANSITION_LABELS + [
                 labels.PYTHON_VERSION,
-                "//command_line_option:stamp",
                 "//command_line_option:build_runfile_links",
                 "//command_line_option:enable_runfiles",
             ],
             outputs = TRANSITION_LABELS + [
                 labels.PYTHON_VERSION,
-                "//command_line_option:stamp",
                 "//command_line_option:build_runfile_links",
                 "//command_line_option:enable_runfiles",
             ],

tests/base_rules/py_executable_base_tests.bzl

@@ -654,6 +654,66 @@ def _test_pyi_deps_not_in_runfiles_when_flag_disabled(name, config):
 
 _tests.append(_test_pyi_deps_not_in_runfiles_when_flag_disabled)
 
+def _stamp_test_subject(name, config, *, stamp):
+    native.config_setting(
+        name = name + "_stamp_enabled",
+        values = {"stamp": "1"},
+    )
+    rt_util.helper_target(
+        config.rule,
+        name = name + "_subject",
+        srcs = [rt_util.empty_file(name + "_subject.py")],
+        data = select({
+            ":" + name + "_stamp_enabled": [rt_util.empty_file(name + "_stamp_on.txt")],
+            "//conditions:default": [rt_util.empty_file(name + "_stamp_off.txt")],
+        }),
+        stamp = stamp,
+    )
+
+def _test_stamp_disabled_keeps_deps_config(name, config):
+    _stamp_test_subject(name, config, stamp = 0)
+    analysis_test(
+        name = name,
+        impl = _test_stamp_disabled_keeps_deps_config_impl,
+        target = name + "_subject",
+        config_settings = {
+            "//command_line_option:stamp": True,
+        },
+    )
+
+def _test_stamp_disabled_keeps_deps_config_impl(env, target):
+    env.expect.that_collection(
+        [action.mnemonic for action in target.actions],
+    ).not_contains("PyWriteBuildData")
+    target = env.expect.that_target(target)
+    target.runfiles().contains_predicate(
+        matching.str_endswith(target.meta.format_str("/{test_name}_stamp_on.txt")),
+    )
+
+_tests.append(_test_stamp_disabled_keeps_deps_config)
+
+def _test_stamp_enabled_keeps_deps_config(name, config):
+    _stamp_test_subject(name, config, stamp = 1)
+    analysis_test(
+        name = name,
+        impl = _test_stamp_enabled_keeps_deps_config_impl,
+        target = name + "_subject",
+        config_settings = {
+            "//command_line_option:stamp": False,
+        },
+    )
+
+def _test_stamp_enabled_keeps_deps_config_impl(env, target):
+    target = env.expect.that_target(target)
+    target.runfiles().contains_predicate(
+        matching.str_endswith(target.meta.format_str("/{test_name}_stamp_off.txt")),
+    )
+    target.action_named("PyWriteBuildData").inputs().contains_predicate(
+        matching.file_basename_equals("uncachable_version_file.txt"),
+    )
+
+_tests.append(_test_stamp_enabled_keeps_deps_config)
+
 def _test_venv_output_prefix_with_path_separators(name, config):
     rt_util.helper_target(
         config.rule,