Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builtins exports.bzl file is called after the WORKSPACE #17713

Closed
keith opened this issue Mar 9, 2023 · 24 comments
Closed

Builtins exports.bzl file is called after the WORKSPACE #17713

keith opened this issue Mar 9, 2023 · 24 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: bug

Comments

@keith
Copy link
Member

keith commented Mar 9, 2023

Description of the bug:

After this commit a3f0bfa, using rules_swift in some cases results in this error:

ERROR: Traceback (most recent call last):
        File "/private/var/tmp/_bazel_ksmiley/1c823ac8225acd936431a04c304d3fec/external/rules_proto/proto/private/rules/proto_descriptor_set.bzl", line 53, column 32, in <toplevel>
                "deps": attr.label_list(
Error in label_list: Illegal argument: element in 'providers' is of unexpected type. Either all elements should be providers, or all elements should be lists of providers, but got an element of type NoneType.
ERROR: Error computing the main repository mapping: at /private/var/tmp/_bazel_ksmiley/1c823ac8225acd936431a04c304d3fec/external/build_bazel_rules_apple/apple/apple.bzl:41:5: at /private/var/tmp/_bazel_ksmiley/1c823ac8225acd936431a04c304d3fec/external/build_bazel_rules_apple/apple/internal/experimental_mixed_language_library.bzl:4:5: at /private/var/tmp/_bazel_ksmiley/1c823ac8225acd936431a04c304d3fec/external/build_bazel_rules_swift/swift/swift.bzl:79:5: at /private/var/tmp/_bazel_ksmiley/1c823ac8225acd936431a04c304d3fec/external/build_bazel_rules_swift/swift/internal/swift_proto_library.bzl:17:6: at /private/var/tmp/_bazel_ksmiley/1c823ac8225acd936431a04c304d3fec/external/rules_proto/proto/defs.bzl:18:6: initialization of module 'proto/private/rules/proto_descriptor_set.bzl' failed

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Here's a repro project protorepro.zip. Run USE_BAZEL_VERSION=last_green bazelisk build .... Testing with the commit before that does not have the issue. Using rules_proto @ the current HEAD (71c4fc69900946093ac5c82d81efd19fa522d060) does not fix the issue.

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@keith
Copy link
Member Author

keith commented Mar 9, 2023

cc @comius

@ShreeM01 ShreeM01 added type: bug untriaged team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Mar 9, 2023
@comius comius added team-Rules-Server Issues for serverside rules included with Bazel P2 We'll consider working on this in future. (Assignee optional) and removed team-Remote-Exec Issues and PRs for the Execution (Remote) team untriaged labels Mar 10, 2023
@comius comius self-assigned this Mar 10, 2023
@comius
Copy link
Contributor

comius commented Mar 10, 2023

cc @brandjon @Wyverald @meteorcloudy
The root cause of this is, that builtins exports.bzl file is called after the WORKSPACE evaluation.

Reproducer:

WORKSPACE:
load(":proto.bzl", "X")

BUILD: 
--empty--

proto.bzl:
print(ProtoInfo)
X = "foo"

bazel build ... will print None

@comius comius changed the title Starlark ProtoInfo used in rule is None Builtins exports.bzl file is called after the WORKSPACE Mar 10, 2023
@comius comius assigned brandjon and unassigned comius Mar 10, 2023
@comius
Copy link
Contributor

comius commented Mar 10, 2023

Possible workaround might be to split bzl files containing repository rules in Apple repo, so that they don't load rules_proto (or anything that uses ProtoInfo).

@comius comius added team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob and removed team-Rules-Server Issues for serverside rules included with Bazel labels Mar 10, 2023
@keith
Copy link
Member Author

keith commented Mar 10, 2023

Yea that could work for this case since it's unlikely folks need that specific rule at this stage

@keith
Copy link
Member Author

keith commented Mar 10, 2023

looks like rules_clojure has the same issue through a different codepath https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2903#0186cce4-f095-461b-a896-2efe15924483

@PatriosTheGreat
Copy link

Hey everyone.

I'm also getting this error trying to build Tensorflow from github on Windows using the latest bazel binaries: https://github.com/bazelbuild/bazel/releases/tag/7.0.0-pre.20230302.1.

Steps to reproduce:

git clone https://github.com/tensorflow/tensorflow.git
cd tensorflow
bazel build //tensorflow/...

Error:

ERROR: Traceback (most recent call last):
        File "kg2r3nzx/external/io_bazel_rules_closure/closure/protobuf/closure_proto_library.bzl", line 146, column 32, in <toplevel>
                "deps": attr.label_list(
Error in label_list: Illegal argument: element in 'providers' is of unexpected type. Either all elements should be providers, or all elements should be lists of providers, but got an element of type NoneType.
ERROR: Error computing the main repository mapping: at tensorflow/workspace2.bzl:51:6: at kg2r3nzx/external/io_bazel_rules_closure/closure/defs.bzl:23:6: initialization of module 'closure/protobuf/closure_proto_library.bzl' failed

@Wyverald
Copy link
Member

Could we assign some dummy Provider value to these symbols before exports.bzl is eval'd? I'm not too sure what is expected of provider-type values, but this might be easier than asking all downstream users to migrate.

@brandjon
Copy link
Member

brandjon commented Mar 17, 2023

The root cause of this is, that builtins exports.bzl file is called after the WORKSPACE evaluation.

It's not that it's called "after", it's that its result is not at all used for WORKSPACE-loaded .bzl files. Builtins injection only applies to BUILD-loaded .bzl files.

See BazelStarlarkEnvironment -- createBuildBzlEnvUsingInjection starts from the uninjected environment and applies the given changes. There's no such logic for WORKSPACE-loaded .bzls.

(The None comes from the native-defined value intended to be overridden by builtins injection.)

@meteorcloudy
Copy link
Member

I think this should be fixed from Bazel side since it's an incompatible change causing many downstream breakages, but is probably avoidable.

copybara-service bot pushed a commit that referenced this issue Apr 11, 2023
This is to workaround #17713 (something about WORKSPACE loaded bzl files
not having builtins-bzl overrides applied).

PiperOrigin-RevId: 523468866
Change-Id: Ia1484a03b958d149255f0b5c37df211ef8dfc46b
copybara-service bot pushed a commit that referenced this issue Apr 19, 2023
#17713

Closes #18022.

PiperOrigin-RevId: 525350732
Change-Id: I869653fcd6d4a82ccca49f14e66245c182062731
@meteorcloudy
Copy link
Member

This should be fixed by b1341be

@keith
Copy link
Member Author

keith commented May 4, 2023

Not fixed at 8b60d6d, cc @benjaminp

@keith keith reopened this May 4, 2023
@meteorcloudy
Copy link
Member

@keith Can you provide more information on what's still broken?

@keith
Copy link
Member Author

keith commented May 5, 2023

Here's a new repro: protorepro.zip

Seems to be the exact same error as before you just have to be using bzlmod and not the WORKSPACE

fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
Returning None fails type-check, when a rule with ProtoInfo is loaded from WORKSPACE context (builtins are not applied).

Worksaround: bazelbuild#17713
PiperOrigin-RevId: 523387385
Change-Id: I08408a6209de9c266bf10f4584f76f0341029fd5
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
This is to workaround bazelbuild#17713 (something about WORKSPACE loaded bzl files
not having builtins-bzl overrides applied).

PiperOrigin-RevId: 523468866
Change-Id: Ia1484a03b958d149255f0b5c37df211ef8dfc46b
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
bazelbuild#17713

Closes bazelbuild#18022.

PiperOrigin-RevId: 525350732
Change-Id: I869653fcd6d4a82ccca49f14e66245c182062731
@brentleyjones
Copy link
Contributor

bzlmod is still broken in the latest rolling release:

ERROR: Traceback (most recent call last):
        File "/Users/brentley/Developer/rules_xcodeproj/examples/integration/bazel-output-base/rules_xcodeproj.noindex/build_output_base/external/rules_proto~5.3.0-21.7/proto/private/rules/proto_descriptor_set.bzl", line 53, column 32, in <toplevel>
                "deps": attr.label_list(
Error in label_list: Illegal argument: element in 'providers' is of unexpected type. Either all elements should be providers, or all elements should be lists of providers, but got an element of type Provider.

@meteorcloudy
Copy link
Member

@comius @hvadehra Any idea?

@benjaminp
Copy link
Collaborator

b1341be disclaimed efforts to fix bzlmod. So, this state is not surprising.

@meteorcloudy
Copy link
Member

/cc @Wyverald for awareness

@keith
Copy link
Member Author

keith commented Jun 23, 2023

I can confirm the new commit fixes the real world case of this. thanks all!

@meteorcloudy
Copy link
Member

Let's try backport both b1341be and c2553cc to Bazel 6.3.0

@meteorcloudy
Copy link
Member

@bazel-io fork 6.3.0

@iancha1992
Copy link
Member

iancha1992 commented Jun 27, 2023

Let's try backport both b1341be and c2553cc to Bazel 6.3.0

@meteorcloudy looks like b1341be is good to cherry-pick. However, when we try to cherry-pick c2553cc, we seem to need another PR or commit because of conflicts from 2 files ("src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java" and "src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java").

For example, createBzlToplevelsWithoutNative function should be in the "src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java" file already in the release-6.3.0.

The conflict from the "src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java" file is from the changed code for the function, "getAndDigestPredeclaredEnvironment"

@meteorcloudy
Copy link
Member

@iancha1992 Thanks for the info!

@hvadehra Do you think we need those fixes in 6.3.0 at all? Did starlarkification of those symbols only happen after Bazel 6?

@hvadehra
Copy link
Member

hvadehra commented Jun 29, 2023

Some rules/symbols might have moved to _builtins before 6.0 was cut. But I don't know if there are any references to those in workspace files. We can cherry pick b1341be out of an abundance of caution. We only need c2553cc for bzlmod, so I'll defer to you if that's something you want to support in 6.3.0.

@meteorcloudy
Copy link
Member

@iancha1992 OK, then let's only cherry pick b1341be for now.

iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Jun 29, 2023
bazelbuild#17713

Closes bazelbuild#18022.

PiperOrigin-RevId: 525350732
Change-Id: I869653fcd6d4a82ccca49f14e66245c182062731
iancha1992 added a commit that referenced this issue Jun 30, 2023
#17713

Closes #18022.

PiperOrigin-RevId: 525350732
Change-Id: I869653fcd6d4a82ccca49f14e66245c182062731

Co-authored-by: Benjamin Peterson <benjamin@engflow.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: bug
Projects
None yet
Development

No branches or pull requests