python: Add `provides = [PyInfo]` to py_library/py_binary/py_test #20436

stagnation · 2023-12-05T09:18:26Z

This is required for aspects to propagate along python libraries (and binaries) with required_providers set.
Otherwise the aspect terminates on the top-level target. This follows from/was inspired by #19609 where the CcInfo was added to cc_binary targets.

The cc_library does provide CcInfo so an aspect can successfully propagate the cc_library tree.
Also related: #17214

Reproduction example to show the problem with py targets

#!/bin/sh

set -eu

dir=${1:-$(mktemp -d)}

# "$dir"/BUILD.bazel {{{
cat > "$dir"/BUILD.bazel <<'EOF'
# stack a high dependency tree
py_binary(
    name = "base_py",
    srcs = ["base.py"],
    main = "base.py",
)

py_library(
    name = "stack_0_py",
    srcs = ["extra.py"],
    deps = ["//:base_py"],
)

[
    py_library(
        name = "stack_{}_py".format(index),
        srcs = ["extra.py"],
        deps = [":stack_{}_py".format(index - 1)],
    )
    for index in range(1, 2)
]
cc_library(
    name = "base_cc",
    srcs = ["base.c"],
)

cc_library(
    name = "stack_0_cc",
    srcs = ["extra.c"],
    deps = ["//:base_cc"],
)

[
    cc_library(
        name = "stack_{}_cc".format(index),
        srcs = ["extra.c"],
        deps = [":stack_{}_cc".format(index - 1)],
    )
    for index in range(1, 2)
]
EOF
# }}}

# "$dir"/file_count.bzl {{{
cat > "$dir"/file_count.bzl <<'EOF'
FileCountInfo = provider(
    'count',
    fields = {
        'count' : 'number of files'
    }
)

def _file_count_aspect_impl(_, ctx):
    name = ctx.rule.attr.name
    count = 0
    # Make sure the rule has a srcs attribute.
    if hasattr(ctx.rule.attr, 'srcs'):
        # Iterate through the sources counting files
        for src in ctx.rule.attr.srcs:
            for _ in src.files.to_list():
                count = count + 1
    # Get the counts from our dependencies.
    for dep in ctx.rule.attr.deps:
        if FileCountInfo in dep:
            count = count + dep[FileCountInfo].count

    print(name, count)
    return [FileCountInfo(count = count)]

ok = aspect(
    implementation = _file_count_aspect_impl,
    attr_aspects = ['deps'],
)

required = aspect(
    implementation = _file_count_aspect_impl,
    required_providers = [
        [PyInfo],
        [CcInfo],
    ],
    attr_aspects = ['deps'],
)
EOF
# }}}

touch "$dir"/WORKSPACE
touch "$dir"/base.py
touch "$dir"/extra.py
touch "$dir"/base.c
touch "$dir"/extra.c

echo Reproducing in "$dir"
cd "$dir" || exit 1

bazelisk --version

echo "[Python] First print the succesfull traversal with transient target, and accumulating count"
(
    set -x
    bazelisk build \
        --ui_event_filters=-info --noshow_progress --noshow_loading_progress \
        --show_result=0 \
        --aspects=//:file_count.bzl%ok //:stack_1_py
    set +x
)
echo "[Python] Now add 'required_providers', and the transitive dependencies disappear."
(
    set -x
    bazelisk build \
        --ui_event_filters=-info --noshow_progress --noshow_loading_progress \
        --show_result=0 \
        --aspects=//:file_count.bzl%required //:stack_1_py
    set +x
)

echo "[CC] Both works for cc_library"
(
    set -x
    bazelisk build \
        --ui_event_filters=-info --noshow_progress --noshow_loading_progress \
        --show_result=0 \
        --aspects=//:file_count.bzl%ok //:stack_1_cc
    bazelisk build \
        --ui_event_filters=-info --noshow_progress --noshow_loading_progress \
        --show_result=0 \
        --aspects=//:file_count.bzl%required //:stack_1_cc
    set +x
)

Output:

$ env USE_BAZEL_VERSION=7.0.0rc5 sh reproduction-aspect-required-provider.
sh aspect-required-provider/
Reproducing in aspect-required-provider/
bazel 7.0.0rc5
[Python] First print the succesfull traversal with transient target, and accumulating count
+ bazelisk build --ui_event_filters=-info --noshow_progress --noshow_loading_progress --aspects=//:file_count.bzl%ok //:stack_1_py
Starting local Bazel server and connecting to it...
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: base_py 1
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_0_py 2
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_1_py 3
+ set +x
[Python] Now add 'required_providers', and the transitive dependencies disappear.
+ bazelisk build --ui_event_filters=-info --noshow_progress --noshow_loading_progress --aspects=//:file_count.bzl%required //:stack_1_py
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_1_py 1
+ set +x
[CC] Both works for cc_library
+ bazelisk build --ui_event_filters=-info --noshow_progress --noshow_loading_progress --aspects=//:file_count.bzl%ok //:stack_1_cc
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: base_cc 1
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_0_cc 2
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_1_cc 3
+ bazelisk build --ui_event_filters=-info --noshow_progress --noshow_loading_progress --aspects=//:file_count.bzl%required //:stack_1_cc
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: base_cc 1
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_0_cc 2
DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_1_cc 3

stagnation · 2023-12-05T11:32:44Z

How do I rerun the checks? It seems to fail for a flaky infrastructure error:

09:20:39) ERROR: /workdir/src/BUILD:569:10: //src:test_repos depends on 
@_main~bazel_build_deps~bazel_tools_repo_cache//:files in repository 
@_main~bazel_build_deps~bazel_tools_repo_cache which failed to fetch. no such package 
'@_main~bazel_build_deps~bazel_tools_repo_cache//': java.io.IOException: Error downloading 
[https://mirror.bazel.build/github.com/bazelbuild/rules_jvm_external/archive/refs/tags/4.4.2.zip, https://github.com
/bazelbuild/rules_jvm_external/archive/refs/tags/4.4.2.zip] to /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-
agent/ec321eb2cc2d0f8f91b676b6d4c66c29/external/_main~bazel_build_deps~bazel_tools_repo_cache/tempfile: 
Unknown host: codeload.github.com
(09:20:39) ERROR: Analysis of target '//src:test_repos' failed; build aborted: Analysis failed

keertk · 2023-12-05T15:19:28Z

@stagnation reran it - looks good now.

stagnation · 2023-12-06T11:26:55Z

Thank you!

stagnation · 2023-12-18T10:39:30Z

Is there something else I can help with here? I want the commit to be symmetric to this:
b1d3441 from @mai93 where the CcInfo was added to cc_binary.

Add provides = [CcInfo] to cc_binary

Fixes: #19609
PiperOrigin-RevId: 576924920
Change-Id: I7ab0c13539f729a80a08e264d35aed7dafe78cc0

rickeylev

Huh. I'm not too surprised py_binary/py_test weren't advertising PyInfo, but I don't know how py_library managed to not advertise that.

src/main/starlark/builtins_bzl/common/python/py_binary_bazel.bzl

stagnation · 2024-02-23T12:47:35Z

I think the tests are flaky again, can we retrigger them?

It is harder for me to understand what the error is this time, but I guess that it can't download deps, and then some tests fails

$ ansi2txt <raw_log | grep failed
bk;t=1707211754581(09:29:14) WARNING: Download from https://mirror.bazel.build/github.com/bazelbuild/rules_python/releases/download/0.28.0/rules_python-0.28.0.tar.gz failed: class java.io.FileNotFoundException GET returned 404 Not
Found
bk;t=1707211755628(09:29:15) WARNING: Download from https://mirror.bazel.build/github.com/bazelbuild/rules_kotlin/releases/download/v1.9.0/rules_kotlin-v1.9.0.tar.gz failed: class java.io.FileNotFoundException GET returned 404 Not
Found
bk;t=1707211755750(09:29:15) WARNING: Download from https://mirror.bazel.build/github.com/bazelbuild/rules_jvm_external/releases/download/6.0/rules_jvm_external-6.0.tar.gz failed: class java.io.FileNotFoundException GET returned 40
4 Not Found
bk;t=1707211756186(09:29:16) WARNING: Download from https://mirror.bazel.build/maven.google.com/com/android/tools/r8/8.2.42/r8-8.2.42.jar failed: class java.io.FileNotFoundException GET returned 404 Not Found
bk;t=1707211756817(09:29:16) WARNING: Download from https://mirror.bazel.build/github.com/JetBrains/kotlin/releases/download/v1.9.10/kotlin-compiler-1.9.10.zip failed: class java.io.FileNotFoundException GET returned 404 Not Found

stagnation · 2024-03-28T10:27:58Z

Can I have another review round please?

stagnation · 2024-04-08T09:47:09Z

@rickeylev: I cannot seem to remove the "Awaiting User Response" label

stagnation · 2024-07-01T09:37:47Z

Friendly ping :)

stagnation · 2024-09-12T10:54:01Z

Rebased

rickeylev

LGTM, but note that python rules are in rules_python. This is only going to matter if you're still using the builtin rules, which are deprecated and due to be removed (replaced by rules_python)

This is required for aspects to propagate along python libraries (and binaries) with `required_providers` set. Otherwise the aspect terminates on the top-level target. This follows from/was inspired by #19609 where the `CcInfo` was added to `cc_binary` targets. The `cc_library` does provide `CcInfo` so an aspect can successfully propagate the `cc_library` tree. Also related: #17214 <details> <summary>Reproduction example to show the problem with py targets</summary> ``` #!/bin/sh set -eu dir=${1:-$(mktemp -d)} # "$dir"/BUILD.bazel {{{ cat > "$dir"/BUILD.bazel <<'EOF' # stack a high dependency tree py_binary( name = "base_py", srcs = ["base.py"], main = "base.py", ) py_library( name = "stack_0_py", srcs = ["extra.py"], deps = ["//:base_py"], ) [ py_library( name = "stack_{}_py".format(index), srcs = ["extra.py"], deps = [":stack_{}_py".format(index - 1)], ) for index in range(1, 2) ] cc_library( name = "base_cc", srcs = ["base.c"], ) cc_library( name = "stack_0_cc", srcs = ["extra.c"], deps = ["//:base_cc"], ) [ cc_library( name = "stack_{}_cc".format(index), srcs = ["extra.c"], deps = [":stack_{}_cc".format(index - 1)], ) for index in range(1, 2) ] EOF # }}} # "$dir"/file_count.bzl {{{ cat > "$dir"/file_count.bzl <<'EOF' FileCountInfo = provider( 'count', fields = { 'count' : 'number of files' } ) def _file_count_aspect_impl(_, ctx): name = ctx.rule.attr.name count = 0 # Make sure the rule has a srcs attribute. if hasattr(ctx.rule.attr, 'srcs'): # Iterate through the sources counting files for src in ctx.rule.attr.srcs: for _ in src.files.to_list(): count = count + 1 # Get the counts from our dependencies. for dep in ctx.rule.attr.deps: if FileCountInfo in dep: count = count + dep[FileCountInfo].count print(name, count) return [FileCountInfo(count = count)] ok = aspect( implementation = _file_count_aspect_impl, attr_aspects = ['deps'], ) required = aspect( implementation = _file_count_aspect_impl, required_providers = [ [PyInfo], [CcInfo], ], attr_aspects = ['deps'], ) EOF # }}} touch "$dir"/WORKSPACE touch "$dir"/base.py touch "$dir"/extra.py touch "$dir"/base.c touch "$dir"/extra.c echo Reproducing in "$dir" cd "$dir" || exit 1 bazelisk --version echo "[Python] First print the succesfull traversal with transient target, and accumulating count" ( set -x bazelisk build \ --ui_event_filters=-info --noshow_progress --noshow_loading_progress \ --show_result=0 \ --aspects=//:file_count.bzl%ok //:stack_1_py set +x ) echo "[Python] Now add 'required_providers', and the transitive dependencies disappear." ( set -x bazelisk build \ --ui_event_filters=-info --noshow_progress --noshow_loading_progress \ --show_result=0 \ --aspects=//:file_count.bzl%required //:stack_1_py set +x ) echo "[CC] Both works for cc_library" ( set -x bazelisk build \ --ui_event_filters=-info --noshow_progress --noshow_loading_progress \ --show_result=0 \ --aspects=//:file_count.bzl%ok //:stack_1_cc bazelisk build \ --ui_event_filters=-info --noshow_progress --noshow_loading_progress \ --show_result=0 \ --aspects=//:file_count.bzl%required //:stack_1_cc set +x ) ``` Output: ``` $ env USE_BAZEL_VERSION=7.0.0rc5 sh reproduction-aspect-required-provider. sh aspect-required-provider/ Reproducing in aspect-required-provider/ bazel 7.0.0rc5 [Python] First print the succesfull traversal with transient target, and accumulating count + bazelisk build --ui_event_filters=-info --noshow_progress --noshow_loading_progress --aspects=//:file_count.bzl%ok //:stack_1_py Starting local Bazel server and connecting to it... DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: base_py 1 DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_0_py 2 DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_1_py 3 + set +x [Python] Now add 'required_providers', and the transitive dependencies disappear. + bazelisk build --ui_event_filters=-info --noshow_progress --noshow_loading_progress --aspects=//:file_count.bzl%required //:stack_1_py DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_1_py 1 + set +x [CC] Both works for cc_library + bazelisk build --ui_event_filters=-info --noshow_progress --noshow_loading_progress --aspects=//:file_count.bzl%ok //:stack_1_cc DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: base_cc 1 DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_0_cc 2 DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_1_cc 3 + bazelisk build --ui_event_filters=-info --noshow_progress --noshow_loading_progress --aspects=//:file_count.bzl%required //:stack_1_cc DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: base_cc 1 DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_0_cc 2 DEBUG: /home/nils/task/reproductions/aspect-required-provider/file_count.bzl:22:10: stack_1_cc 3 ``` </details> Closes #20436. PiperOrigin-RevId: 677997144 Change-Id: Id2f75db10447b334a9d5ec9872e15f490ae44927

github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Dec 5, 2023

keertk removed the awaiting-user-response Awaiting a response from the author label Dec 5, 2023

comius requested a review from rickeylev December 27, 2023 20:26

rickeylev changed the title ~~Add provides = [PyInfo] to py_library and py_binary~~ python: Add provides = [PyInfo] to py_library/py_binary/py_test Jan 25, 2024

rickeylev requested changes Jan 25, 2024

View reviewed changes

src/main/starlark/builtins_bzl/common/python/py_binary_bazel.bzl Outdated Show resolved Hide resolved

rickeylev added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jan 25, 2024

stagnation force-pushed the feature/provide-PyInfo-from-py-rules branch from 2230b8c to 6a8c7e9 Compare January 26, 2024 10:50

stagnation requested a review from rickeylev February 2, 2024 09:40

stagnation force-pushed the feature/provide-PyInfo-from-py-rules branch from 6a8c7e9 to 59e8fb4 Compare February 6, 2024 09:28

stagnation force-pushed the feature/provide-PyInfo-from-py-rules branch 2 times, most recently from 5a94bea to dbbcd8e Compare March 8, 2024 08:24

Add provides = [PyInfo] to py_library and py_binary

48d1788

stagnation force-pushed the feature/provide-PyInfo-from-py-rules branch from dbbcd8e to 48d1788 Compare September 12, 2024 10:53

rickeylev approved these changes Sep 12, 2024

View reviewed changes

comius added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 23, 2024

comius approved these changes Sep 23, 2024

View reviewed changes

copybara-service bot closed this in 910e576 Sep 24, 2024

github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

python: Add `provides = [PyInfo]` to py_library/py_binary/py_test #20436

python: Add `provides = [PyInfo]` to py_library/py_binary/py_test #20436

stagnation commented Dec 5, 2023 •

edited

Loading

stagnation commented Dec 5, 2023 •

edited

Loading

keertk commented Dec 5, 2023

stagnation commented Dec 6, 2023

stagnation commented Dec 18, 2023

rickeylev left a comment

stagnation commented Feb 23, 2024 •

edited

Loading

stagnation commented Mar 28, 2024

stagnation commented Apr 8, 2024

stagnation commented Jul 1, 2024

stagnation commented Sep 12, 2024

rickeylev left a comment

python: Add provides = [PyInfo] to py_library/py_binary/py_test #20436

python: Add provides = [PyInfo] to py_library/py_binary/py_test #20436

Conversation

stagnation commented Dec 5, 2023 • edited Loading

stagnation commented Dec 5, 2023 • edited Loading

keertk commented Dec 5, 2023

stagnation commented Dec 6, 2023

stagnation commented Dec 18, 2023

rickeylev left a comment

Choose a reason for hiding this comment

stagnation commented Feb 23, 2024 • edited Loading

stagnation commented Mar 28, 2024

stagnation commented Apr 8, 2024

stagnation commented Jul 1, 2024

stagnation commented Sep 12, 2024

rickeylev left a comment

Choose a reason for hiding this comment

python: Add `provides = [PyInfo]` to py_library/py_binary/py_test #20436

python: Add `provides = [PyInfo]` to py_library/py_binary/py_test #20436

stagnation commented Dec 5, 2023 •

edited

Loading

stagnation commented Dec 5, 2023 •

edited

Loading

stagnation commented Feb 23, 2024 •

edited

Loading