diff --git a/news/stamp-transition-build-data.fixed.md b/news/stamp-transition-build-data.fixed.md new file mode 100644 index 0000000000..fd4a0638e4 --- /dev/null +++ b/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. diff --git a/python/private/attributes.bzl b/python/private/attributes.bzl index 7fc87ed81e..0fd627f0de 100644 --- a/python/private/attributes.bzl +++ b/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 ` flag, which can additional -config state overhead. ::: :::{note} diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index e57562b43b..c1fb7588f8 100644 --- a/python/private/py_executable.bzl +++ b/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", ], diff --git a/tests/base_rules/py_executable_base_tests.bzl b/tests/base_rules/py_executable_base_tests.bzl index 0caa7ff112..d1be72deb1 100644 --- a/tests/base_rules/py_executable_base_tests.bzl +++ b/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,