From a3cdab5e2670792d5c31b28606722fe11b8d4356 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 11 Oct 2024 11:25:34 -0700 Subject: [PATCH] fix(precompiling)!: make binary-level precompile opt-in/opt-opt work (#2243) This makes binary-level opt-in/opt-out of precompiling work as intended. Previously, when `pyc_collection=include_pyc` was set on a binary, only transitive libraries that had explicitly enabled precompiling were being included (which was moot anyways -- libraries put their files in runfiles, so no matter what, their files were included). The intent was that, when a binary set `pyc_collection=include_pyc`, then precompiled files would be used for all its transitive dependencies (unless they had, at the target-level, disabled precompiling). Conversely, if `pyc_collection=disabled` was set, the precompiled files would not be used (unless a target had, at the target level, enabled precompiling). To make it work as desired, the basic fix is to make it so that libraries have a place to put the implicit pyc files (the ones automatically generated), and have the binaries include those when requested. The net effect is a library has 4 sets of files it produces: * required py files: py source files that should always go into the binary's runfiles * required pyc files: precompiled pyc files that should always go into the binary's runfiles (e.g., when a library sets `precompile=enabled` directly). * implicit pyc files: precompiled pyc files for a library that are always generated, but it's up to the binary if they go into the runfiles * implicit pyc source files: the source py file for an implicit pyc file. When a binary *doesn't* include the implicit pyc file, it must include the source py file (otherwise none of the library's code ends up included). Similarly, in order to allow a binary to decide what files are used, libraries must stop putting the py/pyc files into runfiles themselves. While this is potentially a breaking change, I found that, within Google, there was no reliance on this behavior, so should be safe enough. That said, I added `--add_srcs_to_runfiles` to restore the previous behavior to aid in transitioning. **BREAKING CHANGES** 1. `py_library` no longer puts its srcs into runfiles directly. 2. Removed `--precompile_add_to_runfiles` 3. Removed `--pyc_collection` 4. `precompile=if_generated_source` removed 5. `precompile_source_retention=omit_if_generated_source` removed Though 2 through 5 are technically breaking changes, I don't think precompiling was very usable anyways, so usages of those flags/values is rare. Fixes https://github.com/bazelbuild/rules_python/issues/2212 --- CHANGELOG.md | 20 +- .../python/config_settings/index.md | 63 +-- docs/precompiling.md | 36 +- python/config_settings/BUILD.bazel | 27 +- python/private/BUILD.bazel | 4 + python/private/common/attributes.bzl | 49 +- python/private/common/common.bzl | 32 +- python/private/common/common_bazel.bzl | 59 ++- python/private/common/py_executable.bzl | 131 ++++-- python/private/common/py_library.bzl | 44 +- python/private/enum.bzl | 13 +- python/private/flags.bzl | 69 +-- python/private/proto/py_proto_library.bzl | 41 +- python/private/py_info.bzl | 42 +- python/private/py_package.bzl | 22 +- .../precompile/precompile_tests.bzl | 436 +++++++++++++----- tests/support/support.bzl | 2 +- 17 files changed, 746 insertions(+), 344 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5473dc529f..abaa2bba39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,14 @@ A brief description of the categories of changes: [x.x.x]: https://github.com/bazelbuild/rules_python/releases/tag/x.x.x ### Changed +* **BREAKING** `py_library` no longer puts its source files or generated pyc + files in runfiles; it's the responsibility of consumers (e.g. binaries) to + populate runfiles with the necessary files. Adding source files to runfiles + can be temporarily restored by setting {obj}`--add_srcs_to_runfiles=enabled`, + but this flag will be removed in a subsequent releases. +* {obj}`PyInfo.transitive_sources` is now added to runfiles. These files are + `.py` files that are required to be added to runfiles by downstream binaries + (or equivalent). * (toolchains) `py_runtime.implementation_name` now defaults to `cpython` (previously it defaulted to None). @@ -50,6 +58,8 @@ A brief description of the categories of changes: * (bzlmod) In hybrid bzlmod with WORKSPACE builds, `python_register_toolchains(register_toolchains=True)` is respected ([#1675](https://github.com/bazelbuild/rules_python/issues/1675)). +* (precompiling) The {obj}`pyc_collection` attribute now correctly + enables (or disables) using pyc files from targets transitively ### Added * (py_wheel) Now supports `compress = (True|False)` to allow disabling @@ -66,14 +76,20 @@ A brief description of the categories of changes: * `3.10 -> 3.10.15` * `3.11 -> 3.11.10` * `3.12 -> 3.12.7` -[20241008]: https://github.com/indygreg/python-build-standalone/releases/tag/20241008 * (coverage) Add support for python 3.13 and bump `coverage.py` to 7.6.1. * (bzlmod) Add support for `download_only` flag to disable usage of `sdists` when {bzl:attr}`pip.parse.experimental_index_url` is set. +* (api) PyInfo fields: {obj}`PyInfo.transitive_implicit_pyc_files`, + {obj}`PyInfo.transitive_implicit_pyc_source_files`. +[20241008]: https://github.com/indygreg/python-build-standalone/releases/tag/20241008 ### Removed -* Nothing yet +* (precompiling) {obj}`--precompile_add_to_runfiles` has been removed. +* (precompiling) {obj}`--pyc_collection` has been removed. The `pyc_collection` + attribute now bases its default on {obj}`--precompile`. +* (precompiling) The {obj}`precompile=if_generated_source` value has been removed. +* (precompiling) The {obj}`precompile_source_retention=omit_if_generated_source` value has been removed. ## [0.36.0] - 2024-09-24 diff --git a/docs/api/rules_python/python/config_settings/index.md b/docs/api/rules_python/python/config_settings/index.md index 645e4e2246..511a218eef 100644 --- a/docs/api/rules_python/python/config_settings/index.md +++ b/docs/api/rules_python/python/config_settings/index.md @@ -5,6 +5,25 @@ # //python/config_settings +:::{bzl:flag} add_srcs_to_runfiles +Determines if the `srcs` of targets are added to their runfiles. + +More specifically, the sources added to runfiles are the `.py` files in `srcs`. +If precompiling is performed, it is the `.py` files that are kept according +to {obj}`precompile_source_retention`. + +Values: +* `auto`: (default) Automatically decide the effective value; the current + behavior is `disabled`. +* `disabled`: Don't add `srcs` to a target's runfiles. +* `enabled`: Add `srcs` to a target's runfiles. +::::{versionadded} 0.37.0 +:::: +::::{deprecated} 0.37.0 +This is a transition flag and will be removed in a subsequent release. +:::: +::: + :::{bzl:flag} python_version Determines the default hermetic Python toolchain version. This can be set to one of the values that `rules_python` maintains. @@ -42,12 +61,8 @@ Values: * `auto`: (default) Automatically decide the effective value based on environment, target platform, etc. -* `enabled`: Compile Python source files at build time. Note that - {bzl:obj}`--precompile_add_to_runfiles` affects how the compiled files are included into - a downstream binary. +* `enabled`: Compile Python source files at build time. * `disabled`: Don't compile Python source files at build time. -* `if_generated_source`: Compile Python source files, but only if they're a - generated file. * `force_enabled`: Like `enabled`, except overrides target-level setting. This is mostly useful for development, testing enabling precompilation more broadly, or as an escape hatch if build-time compiling is not available. @@ -56,6 +71,9 @@ Values: broadly, or as an escape hatch if build-time compiling is not available. :::{versionadded} 0.33.0 ::: +:::{versionchanged} 0.37.0 +The `if_generated_source` value was removed +::: :::: ::::{bzl:flag} precompile_source_retention @@ -73,45 +91,14 @@ Values: target platform, etc. * `keep_source`: Include the original Python source. * `omit_source`: Don't include the orignal py source. -* `omit_if_generated_source`: Keep the original source if it's a regular source - file, but omit it if it's a generated file. :::{versionadded} 0.33.0 ::: :::{versionadded} 0.36.0 The `auto` value ::: -:::: - -::::{bzl:flag} precompile_add_to_runfiles -Determines if a target adds its compiled files to its runfiles. - -When a target compiles its files, but doesn't add them to its own runfiles, it -relies on a downstream target to retrieve them from -{bzl:obj}`PyInfo.transitive_pyc_files` - -Values: -* `always`: Always include the compiled files in the target's runfiles. -* `decided_elsewhere`: Don't include the compiled files in the target's - runfiles; they are still added to {bzl:obj}`PyInfo.transitive_pyc_files`. See - also: {bzl:obj}`py_binary.pyc_collection` attribute. This is useful for allowing - incrementally enabling precompilation on a per-binary basis. -:::{versionadded} 0.33.0 -::: -:::: - -::::{bzl:flag} pyc_collection -Determine if `py_binary` collects transitive pyc files. - -:::{note} -This flag is overridden by the target level `pyc_collection` attribute. -::: - -Values: -* `include_pyc`: Include `PyInfo.transitive_pyc_files` as part of the binary. -* `disabled`: Don't include `PyInfo.transitive_pyc_files` as part of the binary. -:::{versionadded} 0.33.0 -::: +:::{versionchanged} 0.37.0 +The `omit_if_generated_source` value was removed :::: ::::{bzl:flag} py_linux_libc diff --git a/docs/precompiling.md b/docs/precompiling.md index 52678e63ea..6eadc4042b 100644 --- a/docs/precompiling.md +++ b/docs/precompiling.md @@ -20,24 +20,24 @@ While precompiling helps runtime performance, it has two main costs: ## Binary-level opt-in -Because of the costs of precompiling, it may not be feasible to globally enable it -for your repo for everything. For example, some binaries may be -particularly large, and doubling the number of runfiles isn't doable. +Binary-level opt-in allows enabling precompiling on a per-target basic. This is +useful for situations such as: -If this is the case, there's an alternative way to more selectively and -incrementally control precompiling on a per-binry basis. +* Globally enabling precompiling in your `.bazelrc` isn't feasible. This may + be because some targets don't work with precompiling, e.g. because they're too + big. +* Enabling precompiling for build tools (exec config targets) separately from + target-config programs. -To use this approach, the two basic steps are: -1. Disable pyc files from being automatically added to runfiles: - {bzl:obj}`--@rules_python//python/config_settings:precompile_add_to_runfiles=decided_elsewhere`, -2. Set the `pyc_collection` attribute on the binaries/tests that should or should - not use precompiling. +To use this approach, set the {bzl:attr}`pyc_collection` attribute on the +binaries/tests that should or should not use precompiling. Then change the +{bzl:flag}`--precompile` default. -The default for the `pyc_collection` attribute is controlled by the flag -{bzl:obj}`--@rules_python//python/config_settings:pyc_collection`, so you +The default for the {bzl:attr}`pyc_collection` attribute is controlled by the flag +{bzl:obj}`--@rules_python//python/config_settings:precompile`, so you can use an opt-in or opt-out approach by setting its value: -* targets must opt-out: `--@rules_python//python/config_settings:pyc_collection=include_pyc` -* targets must opt-in: `--@rules_python//python/config_settings:pyc_collection=disabled` +* targets must opt-out: `--@rules_python//python/config_settings:precompile=enabled` +* targets must opt-in: `--@rules_python//python/config_settings:precompile=disabled` ## Advanced precompiler customization @@ -48,7 +48,7 @@ not work as well for remote execution builds. To customize the precompiler, two mechanisms are available: * The exec tools toolchain allows customizing the precompiler binary used with - the `precompiler` attribute. Arbitrary binaries are supported. + the {bzl:attr}`precompiler` attribute. Arbitrary binaries are supported. * The execution requirements can be customized using `--@rules_python//tools/precompiler:execution_requirements`. This is a list flag that can be repeated. Each entry is a key=value that is added to the @@ -92,3 +92,9 @@ Note that any execution requirements values can be specified in the flag. `foo.cpython-39.opt-2.pyc`). This works fine (it's all byte code), but also means the interpreter `-O` argument can't be used -- doing so will cause the interpreter to look for the non-existent `opt-N` named files. +* Targets with the same source files and different exec properites will result + in action conflicts. This most commonly occurs when a `py_binary` and + `py_library` have the same source files. To fix, modify both targets so + they have the same exec properties. If this is difficult because unsupported + exec groups end up being passed to the Python rules, please file an issue + to have those exec groups added to the Python rules. diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 9fb395741b..b55213b5d6 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -2,12 +2,11 @@ load("@bazel_skylib//rules:common_settings.bzl", "string_flag") load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION", "MINOR_MAPPING", "PYTHON_VERSIONS") load( "//python/private:flags.bzl", + "AddSrcsToRunfilesFlag", "BootstrapImplFlag", "ExecToolsToolchainFlag", - "PrecompileAddToRunfilesFlag", "PrecompileFlag", "PrecompileSourceRetentionFlag", - "PycCollectionFlag", ) load( "//python/private/pypi:flags.bzl", @@ -33,6 +32,14 @@ construct_config_settings( versions = PYTHON_VERSIONS, ) +string_flag( + name = "add_srcs_to_runfiles", + build_setting_default = AddSrcsToRunfilesFlag.AUTO, + values = AddSrcsToRunfilesFlag.flag_values(), + # NOTE: Only public because it is dependency of public rules. + visibility = ["//visibility:public"], +) + string_flag( name = "exec_tools_toolchain", build_setting_default = ExecToolsToolchainFlag.DISABLED, @@ -68,22 +75,6 @@ string_flag( visibility = ["//visibility:public"], ) -string_flag( - name = "precompile_add_to_runfiles", - build_setting_default = PrecompileAddToRunfilesFlag.ALWAYS, - values = sorted(PrecompileAddToRunfilesFlag.__members__.values()), - # NOTE: Only public because it's an implicit dependency - visibility = ["//visibility:public"], -) - -string_flag( - name = "pyc_collection", - build_setting_default = PycCollectionFlag.DISABLED, - values = sorted(PycCollectionFlag.__members__.values()), - # NOTE: Only public because it's an implicit dependency - visibility = ["//visibility:public"], -) - string_flag( name = "bootstrap_impl", build_setting_default = BootstrapImplFlag.SYSTEM_PYTHON, diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index 6fb4a1ccc2..cee77c5836 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -297,6 +297,10 @@ bzl_library( name = "py_package_bzl", srcs = ["py_package.bzl"], visibility = ["//:__subpackages__"], + deps = [ + ":builders_bzl", + ":py_info_bzl", + ], ) bzl_library( diff --git a/python/private/common/attributes.bzl b/python/private/common/attributes.bzl index 0299e85657..56e8a66e19 100644 --- a/python/private/common/attributes.bzl +++ b/python/private/common/attributes.bzl @@ -29,6 +29,26 @@ load( _PackageSpecificationInfo = getattr(py_internal, "PackageSpecificationInfo", None) +# Due to how the common exec_properties attribute works, rules must add exec +# groups even if they don't actually use them. This is due to two interactions: +# 1. Rules give an error if users pass an unsupported exec group. +# 2. exec_properties is configurable, so macro-code can't always filter out +# exec group names that aren't supported by the rule. +# The net effect is, if a user passes exec_properties to a macro, and the macro +# invokes two rules, the macro can't always ensure each rule is only passed +# valid exec groups, and is thus liable to cause an error. +# +# NOTE: These are no-op/empty exec groups. If a rule *does* support an exec +# group and needs custom settings, it should merge this dict with one that +# overrides the supported key. +REQUIRED_EXEC_GROUPS = { + # py_binary may invoke C++ linking, or py rules may be used in combination + # with cc rules (e.g. within the same macro), so support that exec group. + # This exec group is defined by rules_cc for the cc rules. + "cpp_link": exec_group(), + "py_precompile": exec_group(), +} + _STAMP_VALUES = [-1, 0, 1] def _precompile_attr_get_effective_value(ctx): @@ -50,7 +70,6 @@ def _precompile_attr_get_effective_value(ctx): if precompile not in ( PrecompileAttr.ENABLED, PrecompileAttr.DISABLED, - PrecompileAttr.IF_GENERATED_SOURCE, ): fail("Unexpected final precompile value: {}".format(repr(precompile))) @@ -60,14 +79,10 @@ def _precompile_attr_get_effective_value(ctx): PrecompileAttr = enum( # Determine the effective value from --precompile INHERIT = "inherit", - # Compile Python source files at build time. Note that - # --precompile_add_to_runfiles affects how the compiled files are included - # into a downstream binary. + # Compile Python source files at build time. ENABLED = "enabled", # Don't compile Python source files at build time. DISABLED = "disabled", - # Compile Python source files, but only if they're a generated file. - IF_GENERATED_SOURCE = "if_generated_source", get_effective_value = _precompile_attr_get_effective_value, ) @@ -90,7 +105,6 @@ def _precompile_source_retention_get_effective_value(ctx): if attr_value not in ( PrecompileSourceRetentionAttr.KEEP_SOURCE, PrecompileSourceRetentionAttr.OMIT_SOURCE, - PrecompileSourceRetentionAttr.OMIT_IF_GENERATED_SOURCE, ): fail("Unexpected final precompile_source_retention value: {}".format(repr(attr_value))) return attr_value @@ -100,14 +114,17 @@ PrecompileSourceRetentionAttr = enum( INHERIT = "inherit", KEEP_SOURCE = "keep_source", OMIT_SOURCE = "omit_source", - OMIT_IF_GENERATED_SOURCE = "omit_if_generated_source", get_effective_value = _precompile_source_retention_get_effective_value, ) def _pyc_collection_attr_is_pyc_collection_enabled(ctx): pyc_collection = ctx.attr.pyc_collection if pyc_collection == PycCollectionAttr.INHERIT: - pyc_collection = ctx.attr._pyc_collection_flag[BuildSettingInfo].value + precompile_flag = PrecompileFlag.get_effective_value(ctx) + if precompile_flag in (PrecompileFlag.ENABLED, PrecompileFlag.FORCE_ENABLED): + pyc_collection = PycCollectionAttr.INCLUDE_PYC + else: + pyc_collection = PycCollectionAttr.DISABLED if pyc_collection not in (PycCollectionAttr.INCLUDE_PYC, PycCollectionAttr.DISABLED): fail("Unexpected final pyc_collection value: {}".format(repr(pyc_collection))) @@ -283,13 +300,9 @@ Whether py source files **for this target** should be precompiled. Values: -* `inherit`: Determine the value from the {flag}`--precompile` flag. -* `enabled`: Compile Python source files at build time. Note that - --precompile_add_to_runfiles affects how the compiled files are included into - a downstream binary. +* `inherit`: Allow the downstream binary decide if precompiled files are used. +* `enabled`: Compile Python source files at build time. * `disabled`: Don't compile Python source files at build time. -* `if_generated_source`: Compile Python source files, but only if they're a - generated file. :::{seealso} @@ -344,8 +357,6 @@ in the resulting output or not. Valid values are: * `inherit`: Inherit the value from the {flag}`--precompile_source_retention` flag. * `keep_source`: Include the original Python source. * `omit_source`: Don't include the original py source. -* `omit_if_generated_source`: Keep the original source if it's a regular source - file, but omit it if it's a generated file. """, ), # Required attribute, but details vary by rule. @@ -357,10 +368,6 @@ in the resulting output or not. Valid values are: # Required attribute, but the details vary by rule. # Use create_srcs_version_attr to create one. "srcs_version": None, - "_precompile_add_to_runfiles_flag": attr.label( - default = "//python/config_settings:precompile_add_to_runfiles", - providers = [BuildSettingInfo], - ), "_precompile_flag": attr.label( default = "//python/config_settings:precompile", providers = [BuildSettingInfo], diff --git a/python/private/common/common.bzl b/python/private/common/common.bzl index 99a632484d..ec46ea8965 100644 --- a/python/private/common/common.bzl +++ b/python/private/common/common.bzl @@ -348,15 +348,29 @@ def collect_runfiles(ctx, files = depset()): collect_default = True, ) -def create_py_info(ctx, *, direct_sources, direct_pyc_files, imports): +def create_py_info( + ctx, + *, + required_py_files, + required_pyc_files, + implicit_pyc_files, + implicit_pyc_source_files, + imports): """Create PyInfo provider. Args: ctx: rule ctx. - direct_sources: depset of Files; the direct, raw `.py` sources for the - target. This should only be Python source files. It should not - include pyc files. - direct_pyc_files: depset of Files; the direct `.pyc` sources for the target. + required_py_files: `depset[File]`; the direct, `.py` sources for the + target that **must** be included by downstream targets. This should + only be Python source files. It should not include pyc files. + required_pyc_files: `depset[File]`; the direct `.pyc` files this target + produces. + implicit_pyc_files: `depset[File]` pyc files that are only used if pyc + collection is enabled. + implicit_pyc_source_files: `depset[File]` source files for implicit pyc + files that are used when the implicit pyc files are not. + implicit_pyc_files: {type}`depset[File]` Implicitly generated pyc files + that a binary can choose to include. imports: depset of strings; the import path values to propagate. Returns: @@ -366,8 +380,10 @@ def create_py_info(ctx, *, direct_sources, direct_pyc_files, imports): """ py_info = PyInfoBuilder() - py_info.direct_pyc_files.add(direct_pyc_files) - py_info.transitive_pyc_files.add(direct_pyc_files) + py_info.direct_pyc_files.add(required_pyc_files) + py_info.transitive_pyc_files.add(required_pyc_files) + py_info.transitive_implicit_pyc_files.add(implicit_pyc_files) + py_info.transitive_implicit_pyc_source_files.add(implicit_pyc_source_files) py_info.imports.add(imports) py_info.merge_has_py2_only_sources(ctx.attr.srcs_version in ("PY2", "PY2ONLY")) py_info.merge_has_py3_only_sources(ctx.attr.srcs_version in ("PY3", "PY3ONLY")) @@ -386,7 +402,7 @@ def create_py_info(ctx, *, direct_sources, direct_pyc_files, imports): py_info.merge_uses_shared_libraries(cc_helper.is_valid_shared_library_artifact(f)) deps_transitive_sources = py_info.transitive_sources.build() - py_info.transitive_sources.add(direct_sources) + py_info.transitive_sources.add(required_py_files) # We only look at data to calculate uses_shared_libraries, if it's already # true, then we don't need to waste time looping over it. diff --git a/python/private/common/common_bazel.bzl b/python/private/common/common_bazel.bzl index c86abd27f0..6148fc2daa 100644 --- a/python/private/common/common_bazel.bzl +++ b/python/private/common/common_bazel.bzl @@ -14,7 +14,9 @@ """Common functions that are specific to Bazel rule implementation""" load("@bazel_skylib//lib:paths.bzl", "paths") +load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load("@rules_cc//cc:defs.bzl", "CcInfo", "cc_common") +load("//python/private:flags.bzl", "PrecompileFlag") load("//python/private:py_interpreter_program.bzl", "PyInterpreterProgramInfo") load("//python/private:toolchain_types.bzl", "EXEC_TOOLS_TOOLCHAIN_TYPE", "TARGET_TOOLCHAIN_TYPE") load(":attributes.bzl", "PrecompileAttr", "PrecompileInvalidationModeAttr", "PrecompileSourceRetentionAttr") @@ -60,7 +62,7 @@ def maybe_precompile(ctx, srcs): Returns: Struct of precompiling results with fields: * `keep_srcs`: list of File; the input sources that should be included - as default outputs and runfiles. + as default outputs. * `pyc_files`: list of File; the precompiled files. * `py_to_pyc_map`: dict of src File input to pyc File output. If a source file wasn't precompiled, it won't be in the dict. @@ -72,9 +74,27 @@ def maybe_precompile(ctx, srcs): if exec_tools_toolchain == None or exec_tools_toolchain.exec_tools.precompiler == None: precompile = PrecompileAttr.DISABLED else: - precompile = PrecompileAttr.get_effective_value(ctx) + precompile_flag = ctx.attr._precompile_flag[BuildSettingInfo].value + + if precompile_flag == PrecompileFlag.FORCE_ENABLED: + precompile = PrecompileAttr.ENABLED + elif precompile_flag == PrecompileFlag.FORCE_DISABLED: + precompile = PrecompileAttr.DISABLED + else: + precompile = ctx.attr.precompile + + # Unless explicitly disabled, we always generate a pyc. This allows + # binaries to decide whether to include them or not later. + if precompile != PrecompileAttr.DISABLED: + should_precompile = True + else: + should_precompile = False source_retention = PrecompileSourceRetentionAttr.get_effective_value(ctx) + keep_source = ( + not should_precompile or + source_retention == PrecompileSourceRetentionAttr.KEEP_SOURCE + ) result = struct( keep_srcs = [], @@ -82,26 +102,17 @@ def maybe_precompile(ctx, srcs): py_to_pyc_map = {}, ) for src in srcs: - # The logic below is a bit convoluted. The gist is: - # * If precompiling isn't done, add the py source to default outputs. - # Otherwise, the source retention flag decides. - # * In order to determine `use_pycache`, we have to know if the source - # is being added to the default outputs. - is_generated_source = not src.is_source - should_precompile = ( - precompile == PrecompileAttr.ENABLED or - (precompile == PrecompileAttr.IF_GENERATED_SOURCE and is_generated_source) - ) - keep_source = ( - not should_precompile or - source_retention == PrecompileSourceRetentionAttr.KEEP_SOURCE or - (source_retention == PrecompileSourceRetentionAttr.OMIT_IF_GENERATED_SOURCE and not is_generated_source) - ) if should_precompile: + # NOTE: _precompile() may return None pyc = _precompile(ctx, src, use_pycache = keep_source) + else: + pyc = None + + if pyc: result.pyc_files.append(pyc) result.py_to_pyc_map[src] = pyc - if keep_source: + + if keep_source or not pyc: result.keep_srcs.append(src) return result @@ -119,6 +130,12 @@ def _precompile(ctx, src, *, use_pycache): Returns: File of the generated pyc file. """ + + # Generating a file in another package is an error, so we have to skip + # such cases. + if ctx.label.package != src.owner.package: + return None + exec_tools_info = ctx.toolchains[EXEC_TOOLS_TOOLCHAIN_TYPE].exec_tools target_toolchain = ctx.toolchains[TARGET_TOOLCHAIN_TYPE].py3_runtime @@ -149,7 +166,11 @@ def _precompile(ctx, src, *, use_pycache): stem = src.basename[:-(len(src.extension) + 1)] if use_pycache: if not target_toolchain.pyc_tag: - fail("Unable to create __pycache__ pyc: pyc_tag is empty") + # This is most likely because of a "runtime toolchain", i.e. the + # autodetecting toolchain, or some equivalent toolchain that can't + # assume to know the runtime Python version at build time. + # Instead of failing, just don't generate any pyc. + return None pyc_path = "__pycache__/{stem}.{tag}.pyc".format( stem = stem, tag = target_toolchain.pyc_tag, diff --git a/python/private/common/py_executable.bzl b/python/private/common/py_executable.bzl index cfd9961606..6c238a2a5d 100644 --- a/python/private/common/py_executable.bzl +++ b/python/private/common/py_executable.bzl @@ -18,10 +18,9 @@ load("@bazel_skylib//lib:structs.bzl", "structs") load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load("@rules_cc//cc:defs.bzl", "cc_common") load("//python/private:builders.bzl", "builders") -load("//python/private:flags.bzl", "PrecompileAddToRunfilesFlag") load("//python/private:py_executable_info.bzl", "PyExecutableInfo") load("//python/private:py_info.bzl", "PyInfo") -load("//python/private:reexports.bzl", "BuiltinPyRuntimeInfo") +load("//python/private:reexports.bzl", "BuiltinPyInfo", "BuiltinPyRuntimeInfo") load( "//python/private:toolchain_types.bzl", "EXEC_TOOLS_TOOLCHAIN_TYPE", @@ -32,7 +31,9 @@ load( "AGNOSTIC_EXECUTABLE_ATTRS", "COMMON_ATTRS", "PY_SRCS_ATTRS", + "PrecompileAttr", "PycCollectionAttr", + "REQUIRED_EXEC_GROUPS", "SRCS_VERSION_ALL_VALUES", "create_srcs_attr", "create_srcs_version_attr", @@ -99,16 +100,13 @@ filename in `srcs`, `main` must be specified. doc = """ Determines whether pyc files from dependencies should be manually included. -NOTE: This setting is only useful with {flag}`--precompile_add_to_runfiles=decided_elsewhere`. - Valid values are: -* `inherit`: Inherit the value from {flag}`--pyc_collection`. -* `include_pyc`: Add pyc files from dependencies in the binary (from - {obj}`PyInfo.transitive_pyc_files`. -* `disabled`: Don't explicitly add pyc files from dependencies. Note that - pyc files may still come from dependencies if a target includes them as - part of their runfiles (such as when {obj}`--precompile_add_to_runfiles=always` - is used). +* `inherit`: Inherit the value from {flag}`--precompile`. +* `include_pyc`: Add implicitly generated pyc files from dependencies. i.e. + pyc files for targets that specify {attr}`precompile="inherit"`. +* `disabled`: Don't add implicitly generated pyc files. Note that + pyc files may still come from dependencies that enable precompiling at the + target level. """, ), # TODO(b/203567235): In Google, this attribute is deprecated, and can @@ -126,10 +124,6 @@ Valid values are: default = "//python/config_settings:bootstrap_impl", providers = [BuildSettingInfo], ), - "_pyc_collection_flag": attr.label( - default = "//python/config_settings:pyc_collection", - providers = [BuildSettingInfo], - ), "_windows_constraints": attr.label_list( default = [ "@platforms//os:windows", @@ -164,11 +158,21 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = direct_sources = filter_to_py_srcs(ctx.files.srcs) precompile_result = semantics.maybe_precompile(ctx, direct_sources) + required_py_files = precompile_result.keep_srcs + required_pyc_files = [] + implicit_pyc_files = [] + implicit_pyc_source_files = direct_sources + + if ctx.attr.precompile == PrecompileAttr.ENABLED: + required_pyc_files.extend(precompile_result.pyc_files) + else: + implicit_pyc_files.extend(precompile_result.pyc_files) + # Sourceless precompiled builds omit the main py file from outputs, so # main has to be pointed to the precompiled main instead. - if main_py not in precompile_result.keep_srcs: + if (main_py not in precompile_result.keep_srcs and + PycCollectionAttr.is_pyc_collection_enabled(ctx)): main_py = precompile_result.py_to_pyc_map[main_py] - direct_pyc_files = depset(precompile_result.pyc_files) executable = _declare_executable_file(ctx) default_outputs = builders.DepsetBuilder() @@ -200,8 +204,10 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = ctx, executable = executable, extra_deps = extra_deps, - main_py_files = depset([main_py] + precompile_result.keep_srcs), - direct_pyc_files = direct_pyc_files, + required_py_files = required_py_files, + required_pyc_files = required_pyc_files, + implicit_pyc_files = implicit_pyc_files, + implicit_pyc_source_files = implicit_pyc_source_files, extra_common_runfiles = [ runtime_details.runfiles, cc_details.extra_runfiles, @@ -241,8 +247,10 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = runfiles_details = runfiles_details, main_py = main_py, imports = imports, - direct_sources = direct_sources, - direct_pyc_files = direct_pyc_files, + required_py_files = required_py_files, + required_pyc_files = required_pyc_files, + implicit_pyc_files = implicit_pyc_files, + implicit_pyc_source_files = implicit_pyc_source_files, default_outputs = default_outputs.build(), runtime_details = runtime_details, cc_info = cc_details.cc_info_for_propagating, @@ -403,8 +411,10 @@ def _get_base_runfiles_for_binary( *, executable, extra_deps, - main_py_files, - direct_pyc_files, + required_py_files, + required_pyc_files, + implicit_pyc_files, + implicit_pyc_source_files, extra_common_runfiles, semantics): """Returns the set of runfiles necessary prior to executable creation. @@ -417,8 +427,15 @@ def _get_base_runfiles_for_binary( executable: The main executable output. extra_deps: List of Targets; additional targets whose runfiles will be added to the common runfiles. - main_py_files: depset of File of the default outputs to add into runfiles. - direct_pyc_files: depset of File of pyc files directly from this target. + required_py_files: `depset[File]` the direct, `.py` sources for the + target that **must** be included by downstream targets. This should + only be Python source files. It should not include pyc files. + required_pyc_files: `depset[File]` the direct `.pyc` files this target + produces. + implicit_pyc_files: `depset[File]` pyc files that are only used if pyc + collection is enabled. + implicit_pyc_source_files: `depset[File]` source files for implicit pyc + files that are used when the implicit pyc files are not. extra_common_runfiles: List of runfiles; additional runfiles that will be added to the common runfiles. semantics: A `BinarySemantics` struct; see `create_binary_semantics_struct`. @@ -433,19 +450,31 @@ def _get_base_runfiles_for_binary( None. """ common_runfiles = builders.RunfilesBuilder() - common_runfiles.add(main_py_files) - - if ctx.attr._precompile_add_to_runfiles_flag[BuildSettingInfo].value == PrecompileAddToRunfilesFlag.ALWAYS: - common_runfiles.add(direct_pyc_files) - elif PycCollectionAttr.is_pyc_collection_enabled(ctx): - common_runfiles.add(direct_pyc_files) - for dep in (ctx.attr.deps + extra_deps): - if PyInfo not in dep: - continue - common_runfiles.add(dep[PyInfo].transitive_pyc_files) - - common_runfiles.add(collect_runfiles(ctx)) - common_runfiles.add(collect_runfiles(ctx)) + common_runfiles.files.add(required_py_files) + common_runfiles.files.add(required_pyc_files) + pyc_collection_enabled = PycCollectionAttr.is_pyc_collection_enabled(ctx) + if pyc_collection_enabled: + common_runfiles.files.add(implicit_pyc_files) + else: + common_runfiles.files.add(implicit_pyc_source_files) + + for dep in (ctx.attr.deps + extra_deps): + if not (PyInfo in dep or BuiltinPyInfo in dep): + continue + info = dep[PyInfo] if PyInfo in dep else dep[BuiltinPyInfo] + common_runfiles.files.add(info.transitive_sources) + + # Everything past this won't work with BuiltinPyInfo + if not hasattr(info, "transitive_pyc_files"): + continue + + common_runfiles.files.add(info.transitive_pyc_files) + if pyc_collection_enabled: + common_runfiles.files.add(info.transitive_implicit_pyc_files) + else: + common_runfiles.files.add(info.transitive_implicit_pyc_source_files) + + common_runfiles.runfiles.append(collect_runfiles(ctx)) if extra_deps: common_runfiles.add_runfiles(targets = extra_deps) common_runfiles.add(extra_common_runfiles) @@ -782,8 +811,10 @@ def _create_providers( ctx, executable, main_py, - direct_sources, - direct_pyc_files, + required_py_files, + required_pyc_files, + implicit_pyc_files, + implicit_pyc_source_files, default_outputs, runfiles_details, imports, @@ -798,10 +829,15 @@ def _create_providers( ctx: The rule ctx. executable: File; the target's executable file. main_py: File; the main .py entry point. - direct_sources: list of Files; the direct, raw `.py` sources for the target. - This should only be Python source files. It should not include pyc - files. - direct_pyc_files: depset of File; the direct pyc files for the target. + required_py_files: `depset[File]` the direct, `.py` sources for the + target that **must** be included by downstream targets. This should + only be Python source files. It should not include pyc files. + required_pyc_files: `depset[File]` the direct `.pyc` files this target + produces. + implicit_pyc_files: `depset[File]` pyc files that are only used if pyc + collection is enabled. + implicit_pyc_source_files: `depset[File]` source files for implicit pyc + files that are used when the implicit pyc files are not. default_outputs: depset of Files; the files for DefaultInfo.files runfiles_details: runfiles that will become the default and data runfiles. imports: depset of strings; the import paths to propagate @@ -876,8 +912,10 @@ def _create_providers( py_info, deps_transitive_sources, builtin_py_info = create_py_info( ctx, - direct_sources = depset(direct_sources), - direct_pyc_files = direct_pyc_files, + required_py_files = required_py_files, + required_pyc_files = required_pyc_files, + implicit_pyc_files = implicit_pyc_files, + implicit_pyc_source_files = implicit_pyc_source_files, imports = imports, ) @@ -931,6 +969,7 @@ def create_base_executable_rule(*, attrs, fragments = [], **kwargs): # The list might be frozen, so use concatentation fragments = fragments + ["py"] kwargs.setdefault("provides", []).append(PyExecutableInfo) + kwargs["exec_groups"] = REQUIRED_EXEC_GROUPS | (kwargs.get("exec_groups") or {}) return rule( # TODO: add ability to remove attrs, i.e. for imports attr attrs = dicts.add(EXECUTABLE_ATTRS, attrs), diff --git a/python/private/common/py_library.bzl b/python/private/common/py_library.bzl index 078626e063..bce18c3132 100644 --- a/python/private/common/py_library.bzl +++ b/python/private/common/py_library.bzl @@ -16,7 +16,7 @@ load("@bazel_skylib//lib:dicts.bzl", "dicts") load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load("//python/private:builders.bzl", "builders") -load("//python/private:flags.bzl", "PrecompileAddToRunfilesFlag") +load("//python/private:flags.bzl", "AddSrcsToRunfilesFlag", "PrecompileFlag") load( "//python/private:toolchain_types.bzl", "EXEC_TOOLS_TOOLCHAIN_TYPE", @@ -26,6 +26,8 @@ load( ":attributes.bzl", "COMMON_ATTRS", "PY_SRCS_ATTRS", + "PrecompileAttr", + "REQUIRED_EXEC_GROUPS", "SRCS_VERSION_ALL_VALUES", "create_srcs_attr", "create_srcs_version_attr", @@ -51,6 +53,11 @@ LIBRARY_ATTRS = union_attrs( PY_SRCS_ATTRS, create_srcs_version_attr(values = SRCS_VERSION_ALL_VALUES), create_srcs_attr(mandatory = False), + { + "_add_srcs_to_runfiles_flag": attr.label( + default = "//python/config_settings:add_srcs_to_runfiles", + ), + }, ) def py_library_impl(ctx, *, semantics): @@ -67,27 +74,39 @@ def py_library_impl(ctx, *, semantics): direct_sources = filter_to_py_srcs(ctx.files.srcs) precompile_result = semantics.maybe_precompile(ctx, direct_sources) - direct_pyc_files = depset(precompile_result.pyc_files) + + required_py_files = precompile_result.keep_srcs + required_pyc_files = [] + implicit_pyc_files = [] + implicit_pyc_source_files = direct_sources + + precompile_attr = ctx.attr.precompile + precompile_flag = ctx.attr._precompile_flag[BuildSettingInfo].value + if (precompile_attr == PrecompileAttr.ENABLED or + precompile_flag == PrecompileFlag.FORCE_ENABLED): + required_pyc_files.extend(precompile_result.pyc_files) + else: + implicit_pyc_files.extend(precompile_result.pyc_files) + default_outputs = builders.DepsetBuilder() default_outputs.add(precompile_result.keep_srcs) - default_outputs.add(direct_pyc_files) + default_outputs.add(required_pyc_files) default_outputs = default_outputs.build() runfiles = builders.RunfilesBuilder() - runfiles.add(precompile_result.keep_srcs) - - if ctx.attr._precompile_add_to_runfiles_flag[BuildSettingInfo].value == PrecompileAddToRunfilesFlag.ALWAYS: - runfiles.add(direct_pyc_files) - + if AddSrcsToRunfilesFlag.is_enabled(ctx): + runfiles.add(required_py_files) runfiles.add(collect_runfiles(ctx)) runfiles = runfiles.build(ctx) cc_info = semantics.get_cc_info_for_library(ctx) py_info, deps_transitive_sources, builtins_py_info = create_py_info( ctx, - direct_sources = depset(direct_sources), + required_py_files = required_py_files, + required_pyc_files = required_pyc_files, + implicit_pyc_files = implicit_pyc_files, + implicit_pyc_source_files = implicit_pyc_source_files, imports = collect_imports(ctx, semantics), - direct_pyc_files = direct_pyc_files, ) # TODO(b/253059598): Remove support for extra actions; https://github.com/bazelbuild/bazel/issues/16455 @@ -119,6 +138,10 @@ Default outputs: NOTE: Precompilation affects which of the default outputs are included in the resulting runfiles. See the precompile-related attributes and flags for more information. + +:::{versionchanged} 0.37.0 +Source files are no longer added to the runfiles directly. +::: """ def create_py_library_rule(*, attrs = {}, **kwargs): @@ -137,6 +160,7 @@ def create_py_library_rule(*, attrs = {}, **kwargs): # TODO: b/253818097 - fragments=py is only necessary so that # RequiredConfigFragmentsTest passes fragments = kwargs.pop("fragments", None) or [] + kwargs["exec_groups"] = REQUIRED_EXEC_GROUPS | (kwargs.get("exec_groups") or {}) return rule( attrs = dicts.add(LIBRARY_ATTRS, attrs), toolchains = [ diff --git a/python/private/enum.bzl b/python/private/enum.bzl index 011d9fbda1..d71442e3b5 100644 --- a/python/private/enum.bzl +++ b/python/private/enum.bzl @@ -17,10 +17,13 @@ This is a separate file to minimize transitive loads. """ -def enum(**kwargs): +def enum(methods = {}, **kwargs): """Creates a struct whose primary purpose is to be like an enum. Args: + methods: {type}`dict[str, callable]` functions that will be + added to the created enum object, but will have the enum object + itself passed as the first positional arg when calling them. **kwargs: The fields of the returned struct. All uppercase names will be treated as enum values and added to `__members__`. @@ -33,4 +36,10 @@ def enum(**kwargs): for key, value in kwargs.items() if key.upper() == key } - return struct(__members__ = members, **kwargs) + + for name, unbound_method in methods.items(): + # buildifier: disable=uninitialized + kwargs[name] = lambda *a, **k: unbound_method(self, *a, **k) + + self = struct(__members__ = members, **kwargs) + return self diff --git a/python/private/flags.bzl b/python/private/flags.bzl index 652e117221..e7643fc1ae 100644 --- a/python/private/flags.bzl +++ b/python/private/flags.bzl @@ -21,6 +21,40 @@ unnecessary files when all that are needed are flag definitions. load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load("//python/private:enum.bzl", "enum") +def _FlagEnum_flag_values(self): + return sorted(self.__members__.values()) + +def FlagEnum(**kwargs): + """Define an enum specialized for flags. + + Args: + **kwargs: members of the enum. + + Returns: + {type}`FlagEnum` struct. This is an enum with the following extras: + * `flag_values`: A function that returns a sorted list of the + flag values (enum `__members__`). Useful for passing to the + `values` attribute for string flags. + """ + return enum( + methods = dict(flag_values = _FlagEnum_flag_values), + **kwargs + ) + +def _AddSrcsToRunfilesFlag_is_enabled(ctx): + value = ctx.attr._add_srcs_to_runfiles_flag[BuildSettingInfo].value + if value == AddSrcsToRunfilesFlag.AUTO: + value = AddSrcsToRunfilesFlag.ENABLED + return value == AddSrcsToRunfilesFlag.ENABLED + +# buildifier: disable=name-conventions +AddSrcsToRunfilesFlag = FlagEnum( + AUTO = "auto", + ENABLED = "enabled", + DISABLED = "disabled", + is_enabled = _AddSrcsToRunfilesFlag_is_enabled, +) + def _bootstrap_impl_flag_get_value(ctx): return ctx.attr._bootstrap_impl_flag[BuildSettingInfo].value @@ -55,17 +89,13 @@ PrecompileFlag = enum( # Automatically decide the effective value based on environment, # target platform, etc. AUTO = "auto", - # Compile Python source files at build time. Note that - # --precompile_add_to_runfiles affects how the compiled files are included - # into a downstream binary. + # Compile Python source files at build time. ENABLED = "enabled", # Don't compile Python source files at build time. DISABLED = "disabled", - # Compile Python source files, but only if they're a generated file. - IF_GENERATED_SOURCE = "if_generated_source", # Like `enabled`, except overrides target-level setting. This is mostly # useful for development, testing enabling precompilation more broadly, or - # as an escape hatch if build-time compiling is not available. + # as an escape hatch to force all transitive deps to precompile. FORCE_ENABLED = "force_enabled", # Like `disabled`, except overrides target-level setting. This is useful # useful for development, testing enabling precompilation more broadly, or @@ -90,32 +120,5 @@ PrecompileSourceRetentionFlag = enum( KEEP_SOURCE = "keep_source", # Don't include the original py source. OMIT_SOURCE = "omit_source", - # Keep the original py source if it's a regular source file, but omit it - # if it's a generated file. - OMIT_IF_GENERATED_SOURCE = "omit_if_generated_source", get_effective_value = _precompile_source_retention_flag_get_effective_value, ) - -# Determines if a target adds its compiled files to its runfiles. When a target -# compiles its files, but doesn't add them to its own runfiles, it relies on -# a downstream target to retrieve them from `PyInfo.transitive_pyc_files` -# buildifier: disable=name-conventions -PrecompileAddToRunfilesFlag = enum( - # Always include the compiled files in the target's runfiles. - ALWAYS = "always", - # Don't include the compiled files in the target's runfiles; they are - # still added to `PyInfo.transitive_pyc_files`. See also: - # `py_binary.pyc_collection` attribute. This is useful for allowing - # incrementally enabling precompilation on a per-binary basis. - DECIDED_ELSEWHERE = "decided_elsewhere", -) - -# Determine if `py_binary` collects transitive pyc files. -# NOTE: This flag is only respect if `py_binary.pyc_collection` is `inherit`. -# buildifier: disable=name-conventions -PycCollectionFlag = enum( - # Include `PyInfo.transitive_pyc_files` as part of the binary. - INCLUDE_PYC = "include_pyc", - # Don't include `PyInfo.transitive_pyc_files` as part of the binary. - DISABLED = "disabled", -) diff --git a/python/private/proto/py_proto_library.bzl b/python/private/proto/py_proto_library.bzl index e123ff8476..ecb0938bcd 100644 --- a/python/private/proto/py_proto_library.bzl +++ b/python/private/proto/py_proto_library.bzl @@ -16,6 +16,7 @@ load("@rules_proto//proto:defs.bzl", "ProtoInfo", "proto_common") load("//python:defs.bzl", "PyInfo") +load("//python/api:api.bzl", _py_common = "py_common") PY_PROTO_TOOLCHAIN = "@rules_python//python/proto:toolchain_type" @@ -25,6 +26,7 @@ _PyProtoInfo = provider( "imports": """ (depset[str]) The field forwarding PyInfo.imports coming from the proto language runtime dependency.""", + "py_info": "PyInfo from proto runtime (or other deps) to propagate.", "runfiles_from_proto_deps": """ (depset[File]) Files from the transitive closure implicit proto dependencies""", @@ -71,6 +73,11 @@ def _py_proto_aspect_impl(target, ctx): else: proto_lang_toolchain_info = getattr(ctx.attr, "_aspect_proto_toolchain")[proto_common.ProtoLangToolchainInfo] + py_common = _py_common.get(ctx) + py_info = py_common.PyInfoBuilder().merge_target( + proto_lang_toolchain_info.runtime, + ).build() + api_deps = [proto_lang_toolchain_info.runtime] generated_sources = [] @@ -127,16 +134,19 @@ def _py_proto_aspect_impl(target, ctx): ), runfiles_from_proto_deps = runfiles_from_proto_deps, transitive_sources = transitive_sources, + py_info = py_info, ), ] _py_proto_aspect = aspect( implementation = _py_proto_aspect_impl, - attrs = {} if _incompatible_toolchains_enabled() else { - "_aspect_proto_toolchain": attr.label( - default = ":python_toolchain", - ), - }, + attrs = _py_common.API_ATTRS | ( + {} if _incompatible_toolchains_enabled() else { + "_aspect_proto_toolchain": attr.label( + default = ":python_toolchain", + ), + } + ), attr_aspects = ["deps"], required_providers = [ProtoInfo], provides = [_PyProtoInfo], @@ -159,6 +169,17 @@ def _py_proto_library_rule(ctx): transitive = [info.transitive_sources for info in pyproto_infos], ) + py_common = _py_common.get(ctx) + + py_info = py_common.PyInfoBuilder() + py_info.set_has_py2_only_sources(False) + py_info.set_has_py3_only_sources(False) + py_info.transitive_sources.add(default_outputs) + py_info.imports.add([info.imports for info in pyproto_infos]) + py_info.merge_all([ + pyproto_info.py_info + for pyproto_info in pyproto_infos + ]) return [ DefaultInfo( files = default_outputs, @@ -171,13 +192,7 @@ def _py_proto_library_rule(ctx): OutputGroupInfo( default = depset(), ), - PyInfo( - transitive_sources = default_outputs, - imports = depset(transitive = [info.imports for info in pyproto_infos]), - # Proto always produces 2- and 3- compatible source files - has_py2_only_sources = False, - has_py3_only_sources = False, - ), + py_info.build(), ] py_proto_library = rule( @@ -218,6 +233,6 @@ proto_library( providers = [ProtoInfo], aspects = [_py_proto_aspect], ), - }, + } | _py_common.API_ATTRS, provides = [PyInfo], ) diff --git a/python/private/py_info.bzl b/python/private/py_info.bzl index ce56e2330a..6c2c3c6499 100644 --- a/python/private/py_info.bzl +++ b/python/private/py_info.bzl @@ -36,7 +36,9 @@ def _PyInfo_init( has_py2_only_sources = False, has_py3_only_sources = False, direct_pyc_files = depset(), - transitive_pyc_files = depset()): + transitive_pyc_files = depset(), + transitive_implicit_pyc_files = depset(), + transitive_implicit_pyc_source_files = depset()): _check_arg_type("transitive_sources", "depset", transitive_sources) # Verify it's postorder compatible, but retain is original ordering. @@ -49,11 +51,15 @@ def _PyInfo_init( _check_arg_type("direct_pyc_files", "depset", direct_pyc_files) _check_arg_type("transitive_pyc_files", "depset", transitive_pyc_files) + _check_arg_type("transitive_implicit_pyc_files", "depset", transitive_pyc_files) + _check_arg_type("transitive_implicit_pyc_source_files", "depset", transitive_pyc_files) return { "direct_pyc_files": direct_pyc_files, "has_py2_only_sources": has_py2_only_sources, "has_py3_only_sources": has_py2_only_sources, "imports": imports, + "transitive_implicit_pyc_files": transitive_implicit_pyc_files, + "transitive_implicit_pyc_source_files": transitive_implicit_pyc_source_files, "transitive_pyc_files": transitive_pyc_files, "transitive_sources": transitive_sources, "uses_shared_libraries": uses_shared_libraries, @@ -90,6 +96,26 @@ A depset of import path strings to be added to the `PYTHONPATH` of executable Python targets. These are accumulated from the transitive `deps`. The order of the depset is not guaranteed and may be changed in the future. It is recommended to use `default` order (the default). +""", + "transitive_implicit_pyc_files": """ +:type: depset[File] + +Automatically generated pyc files that downstream binaries (or equivalent) +can choose to include in their output. If not included, then +{obj}`transitive_implicit_pyc_source_files` should be included instead. + +::::{versionadded} 0.37.0 +:::: +""", + "transitive_implicit_pyc_source_files": """ +:type: depset[File] + +Source `.py` files for {obj}`transitive_implicit_pyc_files` that downstream +binaries (or equivalent) can choose to include in their output. If not included, +then {obj}`transitive_implicit_pyc_files` should be included instead. + +::::{versionadded} 0.37.0 +:::: """, "transitive_pyc_files": """ :type: depset[File] @@ -105,6 +131,14 @@ to always include these files, as the originating target expects them to exist. A (`postorder`-compatible) depset of `.py` files appearing in the target's `srcs` and the `srcs` of the target's transitive `deps`. + +These are `.py` source files that are considered required and downstream +binaries (or equivalent) must include in their outputs. + +::::{versionchanged} 0.37.0 +The files are considered necessary for downstream binaries to function; +previously they were considerd informational and largely unused. +:::: """, "uses_shared_libraries": """ :type: bool @@ -143,6 +177,8 @@ def PyInfoBuilder(): set_has_py2_only_sources = lambda *a, **k: _PyInfoBuilder_set_has_py2_only_sources(self, *a, **k), set_has_py3_only_sources = lambda *a, **k: _PyInfoBuilder_set_has_py3_only_sources(self, *a, **k), set_uses_shared_libraries = lambda *a, **k: _PyInfoBuilder_set_uses_shared_libraries(self, *a, **k), + transitive_implicit_pyc_files = builders.DepsetBuilder(), + transitive_implicit_pyc_source_files = builders.DepsetBuilder(), transitive_pyc_files = builders.DepsetBuilder(), transitive_sources = builders.DepsetBuilder(), ) @@ -199,6 +235,8 @@ def _PyInfoBuilder_merge_all(self, transitive, *, direct = []): # BuiltinPyInfo doesn't have these fields if hasattr(info, "transitive_pyc_files"): + self.transitive_implicit_pyc_files.add(info.transitive_implicit_pyc_files) + self.transitive_implicit_pyc_source_files.add(info.transitive_implicit_pyc_source_files) self.transitive_pyc_files.add(info.transitive_pyc_files) return self @@ -220,6 +258,8 @@ def _PyInfoBuilder_build(self): kwargs = dict( direct_pyc_files = self.direct_pyc_files.build(), transitive_pyc_files = self.transitive_pyc_files.build(), + transitive_implicit_pyc_files = self.transitive_implicit_pyc_files.build(), + transitive_implicit_pyc_source_files = self.transitive_implicit_pyc_source_files.build(), ) else: kwargs = {} diff --git a/python/private/py_package.bzl b/python/private/py_package.bzl index 08f4b0b318..fd8bc2724c 100644 --- a/python/private/py_package.bzl +++ b/python/private/py_package.bzl @@ -11,9 +11,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - "Implementation of py_package rule" +load(":builders.bzl", "builders") +load(":py_info.bzl", "PyInfoBuilder") + def _path_inside_wheel(input_file): # input_file.short_path is sometimes relative ("../${repository_root}/foobar") # which is not a valid path within a zip file. Fix that. @@ -31,10 +33,20 @@ def _path_inside_wheel(input_file): return short_path def _py_package_impl(ctx): - inputs = depset( - transitive = [dep[DefaultInfo].data_runfiles.files for dep in ctx.attr.deps] + - [dep[DefaultInfo].default_runfiles.files for dep in ctx.attr.deps], - ) + inputs = builders.DepsetBuilder() + py_info = PyInfoBuilder() + for dep in ctx.attr.deps: + inputs.add(dep[DefaultInfo].data_runfiles.files) + inputs.add(dep[DefaultInfo].default_runfiles.files) + py_info.merge_target(dep) + py_info = py_info.build() + inputs.add(py_info.transitive_sources) + + # Remove conditional once Bazel 6 support dropped. + if hasattr(py_info, "transitive_pyc_files"): + inputs.add(py_info.transitive_pyc_files) + + inputs = inputs.build() # TODO: '/' is wrong on windows, but the path separator is not available in starlark. # Fix this once ctx.configuration has directory separator information. diff --git a/tests/base_rules/precompile/precompile_tests.bzl b/tests/base_rules/precompile/precompile_tests.bzl index 4c0f936ac5..9d6ac5f7d4 100644 --- a/tests/base_rules/precompile/precompile_tests.bzl +++ b/tests/base_rules/precompile/precompile_tests.bzl @@ -26,11 +26,10 @@ load("//python:py_test.bzl", "py_test") load("//tests/support:py_info_subject.bzl", "py_info_subject") load( "//tests/support:support.bzl", + "ADD_SRCS_TO_RUNFILES", "CC_TOOLCHAIN", "EXEC_TOOLS_TOOLCHAIN", "PRECOMPILE", - "PRECOMPILE_ADD_TO_RUNFILES", - "PRECOMPILE_SOURCE_RETENTION", "PY_TOOLCHAINS", ) @@ -44,7 +43,7 @@ _COMMON_CONFIG_SETTINGS = { _tests = [] -def _test_precompile_enabled_setup(name, py_rule, **kwargs): +def _test_executable_precompile_attr_enabled_setup(name, py_rule, **kwargs): if not rp_config.enable_pystar: rt_util.skip_test(name = name) return @@ -53,31 +52,43 @@ def _test_precompile_enabled_setup(name, py_rule, **kwargs): name = name + "_subject", precompile = "enabled", srcs = ["main.py"], - deps = [name + "_lib"], + deps = [name + "_lib1"], **kwargs ) rt_util.helper_target( py_library, - name = name + "_lib", - srcs = ["lib.py"], + name = name + "_lib1", + srcs = ["lib1.py"], + precompile = "enabled", + deps = [name + "_lib2"], + ) + + # 2nd order target to verify propagation + rt_util.helper_target( + py_library, + name = name + "_lib2", + srcs = ["lib2.py"], precompile = "enabled", ) analysis_test( name = name, - impl = _test_precompile_enabled_impl, + impl = _test_executable_precompile_attr_enabled_impl, target = name + "_subject", config_settings = _COMMON_CONFIG_SETTINGS, ) -def _test_precompile_enabled_impl(env, target): +def _test_executable_precompile_attr_enabled_impl(env, target): target = env.expect.that_target(target) runfiles = target.runfiles() - runfiles.contains_predicate( + runfiles_contains_at_least_predicates(runfiles, [ matching.str_matches("__pycache__/main.fakepy-45.pyc"), - ) - runfiles.contains_predicate( + matching.str_matches("__pycache__/lib1.fakepy-45.pyc"), + matching.str_matches("__pycache__/lib2.fakepy-45.pyc"), matching.str_matches("/main.py"), - ) + matching.str_matches("/lib1.py"), + matching.str_matches("/lib2.py"), + ]) + target.default_outputs().contains_at_least_predicates([ matching.file_path_matches("__pycache__/main.fakepy-45.pyc"), matching.file_path_matches("/main.py"), @@ -88,23 +99,85 @@ def _test_precompile_enabled_impl(env, target): ]) py_info.transitive_pyc_files().contains_exactly([ "{package}/__pycache__/main.fakepy-45.pyc", - "{package}/__pycache__/lib.fakepy-45.pyc", + "{package}/__pycache__/lib1.fakepy-45.pyc", + "{package}/__pycache__/lib2.fakepy-45.pyc", ]) def _test_precompile_enabled_py_binary(name): - _test_precompile_enabled_setup(name = name, py_rule = py_binary, main = "main.py") + _test_executable_precompile_attr_enabled_setup(name = name, py_rule = py_binary, main = "main.py") _tests.append(_test_precompile_enabled_py_binary) def _test_precompile_enabled_py_test(name): - _test_precompile_enabled_setup(name = name, py_rule = py_test, main = "main.py") + _test_executable_precompile_attr_enabled_setup(name = name, py_rule = py_test, main = "main.py") _tests.append(_test_precompile_enabled_py_test) -def _test_precompile_enabled_py_library(name): - _test_precompile_enabled_setup(name = name, py_rule = py_library) +def _test_precompile_enabled_py_library_setup(name, impl, config_settings): + if not rp_config.enable_pystar: + rt_util.skip_test(name = name) + return + rt_util.helper_target( + py_library, + name = name + "_subject", + srcs = ["lib.py"], + precompile = "enabled", + ) + analysis_test( + name = name, + impl = impl, #_test_precompile_enabled_py_library_impl, + target = name + "_subject", + config_settings = _COMMON_CONFIG_SETTINGS | config_settings, + ) + +def _test_precompile_enabled_py_library_common_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"), + ]) + 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", + ]) + +def _test_precompile_enabled_py_library_add_to_runfiles_disabled(name): + _test_precompile_enabled_py_library_setup( + name = name, + impl = _test_precompile_enabled_py_library_add_to_runfiles_disabled_impl, + config_settings = { + ADD_SRCS_TO_RUNFILES: "disabled", + }, + ) + +def _test_precompile_enabled_py_library_add_to_runfiles_disabled_impl(env, target): + _test_precompile_enabled_py_library_common_impl(env, target) + runfiles = env.expect.that_target(target).runfiles() + runfiles.contains_exactly([]) + +_tests.append(_test_precompile_enabled_py_library_add_to_runfiles_disabled) + +def _test_precompile_enabled_py_library_add_to_runfiles_enabled(name): + _test_precompile_enabled_py_library_setup( + name = name, + impl = _test_precompile_enabled_py_library_add_to_runfiles_enabled_impl, + config_settings = { + ADD_SRCS_TO_RUNFILES: "enabled", + }, + ) + +def _test_precompile_enabled_py_library_add_to_runfiles_enabled_impl(env, target): + _test_precompile_enabled_py_library_common_impl(env, target) + runfiles = env.expect.that_target(target).runfiles() + runfiles.contains_exactly([ + "{workspace}/{package}/lib.py", + ]) -_tests.append(_test_precompile_enabled_py_library) +_tests.append(_test_precompile_enabled_py_library_add_to_runfiles_enabled) def _test_pyc_only(name): if not rp_config.enable_pystar: @@ -117,12 +190,19 @@ def _test_pyc_only(name): srcs = ["main.py"], main = "main.py", precompile_source_retention = "omit_source", + pyc_collection = "include_pyc", + deps = [name + "_lib"], + ) + rt_util.helper_target( + py_library, + name = name + "_lib", + srcs = ["lib.py"], + precompile_source_retention = "omit_source", ) analysis_test( name = name, impl = _test_pyc_only_impl, config_settings = _COMMON_CONFIG_SETTINGS | { - ##PRECOMPILE_SOURCE_RETENTION: "omit_source", PRECOMPILE: "enabled", }, target = name + "_subject", @@ -136,9 +216,15 @@ def _test_pyc_only_impl(env, target): runfiles.contains_predicate( matching.str_matches("/main.pyc"), ) + runfiles.contains_predicate( + matching.str_matches("/lib.pyc"), + ) runfiles.not_contains_predicate( matching.str_endswith("/main.py"), ) + runfiles.not_contains_predicate( + matching.str_endswith("/lib.py"), + ) target.default_outputs().contains_at_least_predicates([ matching.file_path_matches("/main.pyc"), ]) @@ -146,165 +232,291 @@ def _test_pyc_only_impl(env, target): matching.file_basename_equals("main.py"), ) -def _test_precompile_if_generated(name): +def _test_precompiler_action(name): if not rp_config.enable_pystar: rt_util.skip_test(name = name) return rt_util.helper_target( py_binary, name = name + "_subject", - srcs = [ - "main.py", - rt_util.empty_file("generated1.py"), - ], - main = "main.py", - precompile = "if_generated_source", + srcs = ["main2.py"], + main = "main2.py", + precompile = "enabled", + precompile_optimize_level = 2, + precompile_invalidation_mode = "unchecked_hash", ) analysis_test( name = name, - impl = _test_precompile_if_generated_impl, + impl = _test_precompiler_action_impl, target = name + "_subject", config_settings = _COMMON_CONFIG_SETTINGS, ) -_tests.append(_test_precompile_if_generated) +_tests.append(_test_precompiler_action) -def _test_precompile_if_generated_impl(env, target): - target = env.expect.that_target(target) - runfiles = target.runfiles() - runfiles.contains_predicate( - matching.str_matches("/__pycache__/generated1.fakepy-45.pyc"), - ) - runfiles.not_contains_predicate( - matching.str_matches("main.*pyc"), - ) - target.default_outputs().contains_at_least_predicates([ - matching.file_path_matches("/__pycache__/generated1.fakepy-45.pyc"), +def _test_precompiler_action_impl(env, target): + action = env.expect.that_target(target).action_named("PyCompile") + action.contains_flag_values([ + ("--optimize", "2"), + ("--python_version", "4.5"), + ("--invalidation_mode", "unchecked_hash"), ]) - target.default_outputs().not_contains_predicate( - matching.file_path_matches("main.*pyc"), - ) + action.has_flags_specified(["--src", "--pyc", "--src_name"]) + action.env().contains_at_least({ + "PYTHONHASHSEED": "0", + "PYTHONNOUSERSITE": "1", + "PYTHONSAFEPATH": "1", + }) -def _test_omit_source_if_generated_source(name): - if not rp_config.enable_pystar: - rt_util.skip_test(name = name) - return +def _setup_precompile_flag_pyc_collection_attr_interaction( + *, + name, + pyc_collection_attr, + precompile_flag, + test_impl): rt_util.helper_target( py_binary, - name = name + "_subject", - srcs = [ - "main.py", - rt_util.empty_file("generated2.py"), + name = name + "_bin", + srcs = ["bin.py"], + main = "bin.py", + precompile = "disabled", + pyc_collection = pyc_collection_attr, + deps = [ + name + "_lib_inherit", + name + "_lib_enabled", + name + "_lib_disabled", ], - main = "main.py", + ) + rt_util.helper_target( + py_library, + name = name + "_lib_inherit", + srcs = ["lib_inherit.py"], + precompile = "inherit", + ) + rt_util.helper_target( + py_library, + name = name + "_lib_enabled", + srcs = ["lib_enabled.py"], precompile = "enabled", ) + rt_util.helper_target( + py_library, + name = name + "_lib_disabled", + srcs = ["lib_disabled.py"], + precompile = "disabled", + ) analysis_test( name = name, - impl = _test_omit_source_if_generated_source_impl, - target = name + "_subject", + impl = test_impl, + target = name + "_bin", config_settings = _COMMON_CONFIG_SETTINGS | { - PRECOMPILE_SOURCE_RETENTION: "omit_if_generated_source", + PRECOMPILE: precompile_flag, }, ) -_tests.append(_test_omit_source_if_generated_source) +def _verify_runfiles(contains_patterns, not_contains_patterns): + def _verify_runfiles_impl(env, target): + runfiles = env.expect.that_target(target).runfiles() + for pattern in contains_patterns: + runfiles.contains_predicate(matching.str_matches(pattern)) + for pattern in not_contains_patterns: + runfiles.not_contains_predicate( + matching.str_matches(pattern), + ) -def _test_omit_source_if_generated_source_impl(env, target): - target = env.expect.that_target(target) - runfiles = target.runfiles() - runfiles.contains_predicate( - matching.str_matches("/generated2.pyc"), + return _verify_runfiles_impl + +def _test_precompile_flag_enabled_pyc_collection_attr_include_pyc(name): + if not rp_config.enable_pystar: + rt_util.skip_test(name = name) + return + _setup_precompile_flag_pyc_collection_attr_interaction( + name = name, + precompile_flag = "enabled", + pyc_collection_attr = "include_pyc", + test_impl = _verify_runfiles( + contains_patterns = [ + "__pycache__/lib_enabled.*.pyc", + "__pycache__/lib_inherit.*.pyc", + ], + not_contains_patterns = [ + "/bin*.pyc", + "/lib_disabled*.pyc", + ], + ), ) - runfiles.contains_predicate( - matching.str_matches("__pycache__/main.fakepy-45.pyc"), + +_tests.append(_test_precompile_flag_enabled_pyc_collection_attr_include_pyc) + +# buildifier: disable=function-docstring-header +def _test_precompile_flag_enabled_pyc_collection_attr_disabled(name): + """Verify that a binary can opt-out of using implicit pycs even when + precompiling is enabled by default. + """ + if not rp_config.enable_pystar: + rt_util.skip_test(name = name) + return + _setup_precompile_flag_pyc_collection_attr_interaction( + name = name, + precompile_flag = "enabled", + pyc_collection_attr = "disabled", + test_impl = _verify_runfiles( + contains_patterns = [ + "__pycache__/lib_enabled.*.pyc", + ], + not_contains_patterns = [ + "/bin*.pyc", + "/lib_disabled*.pyc", + "/lib_inherit.*.pyc", + ], + ), ) - target.default_outputs().contains_at_least_predicates([ - matching.file_path_matches("generated2.pyc"), - ]) - target.default_outputs().contains_predicate( - matching.file_path_matches("__pycache__/main.fakepy-45.pyc"), + +_tests.append(_test_precompile_flag_enabled_pyc_collection_attr_disabled) + +# buildifier: disable=function-docstring-header +def _test_precompile_flag_disabled_pyc_collection_attr_include_pyc(name): + """Verify that a binary can opt-in to using pycs even when precompiling is + disabled by default.""" + if not rp_config.enable_pystar: + rt_util.skip_test(name = name) + return + _setup_precompile_flag_pyc_collection_attr_interaction( + name = name, + precompile_flag = "disabled", + pyc_collection_attr = "include_pyc", + test_impl = _verify_runfiles( + contains_patterns = [ + "__pycache__/lib_enabled.*.pyc", + "__pycache__/lib_inherit.*.pyc", + ], + not_contains_patterns = [ + "/bin*.pyc", + "/lib_disabled*.pyc", + ], + ), + ) + +_tests.append(_test_precompile_flag_disabled_pyc_collection_attr_include_pyc) + +def _test_precompile_flag_disabled_pyc_collection_attr_disabled(name): + if not rp_config.enable_pystar: + rt_util.skip_test(name = name) + return + _setup_precompile_flag_pyc_collection_attr_interaction( + name = name, + precompile_flag = "disabled", + pyc_collection_attr = "disabled", + test_impl = _verify_runfiles( + contains_patterns = [ + "__pycache__/lib_enabled.*.pyc", + ], + not_contains_patterns = [ + "/bin*.pyc", + "/lib_disabled*.pyc", + "/lib_inherit.*.pyc", + ], + ), ) -def _test_precompile_add_to_runfiles_decided_elsewhere(name): +_tests.append(_test_precompile_flag_disabled_pyc_collection_attr_disabled) + +# buildifier: disable=function-docstring-header +def _test_pyc_collection_disabled_library_omit_source(name): + """Verify that, when a binary doesn't include implicit pyc files, libraries + that set omit_source still have the py source file included. + """ if not rp_config.enable_pystar: rt_util.skip_test(name = name) return rt_util.helper_target( py_binary, - name = name + "_binary", + name = name + "_subject", srcs = ["bin.py"], main = "bin.py", deps = [name + "_lib"], - pyc_collection = "include_pyc", + pyc_collection = "disabled", ) rt_util.helper_target( py_library, name = name + "_lib", srcs = ["lib.py"], + precompile = "inherit", + precompile_source_retention = "omit_source", ) analysis_test( name = name, - impl = _test_precompile_add_to_runfiles_decided_elsewhere_impl, - targets = { - "binary": name + "_binary", - "library": name + "_lib", - }, - config_settings = _COMMON_CONFIG_SETTINGS | { - PRECOMPILE_ADD_TO_RUNFILES: "decided_elsewhere", - PRECOMPILE: "enabled", - }, + impl = _test_pyc_collection_disabled_library_omit_source_impl, + target = name + "_subject", + config_settings = _COMMON_CONFIG_SETTINGS, ) -_tests.append(_test_precompile_add_to_runfiles_decided_elsewhere) - -def _test_precompile_add_to_runfiles_decided_elsewhere_impl(env, targets): - env.expect.that_target(targets.binary).runfiles().contains_at_least([ - "{workspace}/{package}/__pycache__/bin.fakepy-45.pyc", - "{workspace}/{package}/__pycache__/lib.fakepy-45.pyc", - "{workspace}/{package}/bin.py", - "{workspace}/{package}/lib.py", - ]) +def _test_pyc_collection_disabled_library_omit_source_impl(env, target): + contains_patterns = [ + "/lib.py", + "/bin.py", + ] + not_contains_patterns = [ + "/lib.*pyc", + "/bin.*pyc", + ] + runfiles = env.expect.that_target(target).runfiles() + for pattern in contains_patterns: + runfiles.contains_predicate(matching.str_matches(pattern)) + for pattern in not_contains_patterns: + runfiles.not_contains_predicate( + matching.str_matches(pattern), + ) - env.expect.that_target(targets.library).runfiles().contains_exactly([ - "{workspace}/{package}/lib.py", - ]) +_tests.append(_test_pyc_collection_disabled_library_omit_source) -def _test_precompiler_action(name): +def _test_pyc_collection_include_dep_omit_source(name): if not rp_config.enable_pystar: rt_util.skip_test(name = name) return rt_util.helper_target( py_binary, name = name + "_subject", - srcs = ["main2.py"], - main = "main2.py", - precompile = "enabled", - precompile_optimize_level = 2, - precompile_invalidation_mode = "unchecked_hash", + srcs = ["bin.py"], + main = "bin.py", + deps = [name + "_lib"], + precompile = "disabled", + pyc_collection = "include_pyc", + ) + rt_util.helper_target( + py_library, + name = name + "_lib", + srcs = ["lib.py"], + precompile = "inherit", + precompile_source_retention = "omit_source", ) analysis_test( name = name, - impl = _test_precompiler_action_impl, + impl = _test_pyc_collection_include_dep_omit_source_impl, target = name + "_subject", config_settings = _COMMON_CONFIG_SETTINGS, ) -_tests.append(_test_precompiler_action) +def _test_pyc_collection_include_dep_omit_source_impl(env, target): + contains_patterns = [ + "/lib.pyc", + ] + not_contains_patterns = [ + "/lib.py", + ] + runfiles = env.expect.that_target(target).runfiles() + for pattern in contains_patterns: + runfiles.contains_predicate(matching.str_endswith(pattern)) + for pattern in not_contains_patterns: + runfiles.not_contains_predicate( + matching.str_endswith(pattern), + ) -def _test_precompiler_action_impl(env, target): - action = env.expect.that_target(target).action_named("PyCompile") - action.contains_flag_values([ - ("--optimize", "2"), - ("--python_version", "4.5"), - ("--invalidation_mode", "unchecked_hash"), - ]) - action.has_flags_specified(["--src", "--pyc", "--src_name"]) - action.env().contains_at_least({ - "PYTHONHASHSEED": "0", - "PYTHONNOUSERSITE": "1", - "PYTHONSAFEPATH": "1", - }) +_tests.append(_test_pyc_collection_include_dep_omit_source) + +def runfiles_contains_at_least_predicates(runfiles, predicates): + for predicate in predicates: + runfiles.contains_predicate(predicate) def precompile_test_suite(name): test_suite( diff --git a/tests/support/support.bzl b/tests/support/support.bzl index 150ca7f4a4..7358a6b1ee 100644 --- a/tests/support/support.bzl +++ b/tests/support/support.bzl @@ -32,9 +32,9 @@ CROSSTOOL_TOP = Label("//tests/support/cc_toolchains:cc_toolchain_suite") # str() around Label() is necessary because rules_testing's config_settings # doesn't accept yet Label objects. +ADD_SRCS_TO_RUNFILES = str(Label("//python/config_settings:add_srcs_to_runfiles")) EXEC_TOOLS_TOOLCHAIN = str(Label("//python/config_settings:exec_tools_toolchain")) PRECOMPILE = str(Label("//python/config_settings:precompile")) -PRECOMPILE_ADD_TO_RUNFILES = str(Label("//python/config_settings:precompile_add_to_runfiles")) PRECOMPILE_SOURCE_RETENTION = str(Label("//python/config_settings:precompile_source_retention")) PYC_COLLECTION = str(Label("//python/config_settings:pyc_collection")) PYTHON_VERSION = str(Label("//python/config_settings:python_version"))