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

Remove references to io_bazel repository #16559

Closed
wants to merge 1 commit into from

Conversation

comius
Copy link
Contributor

@comius comius commented Oct 26, 2022

Users of @bazel_tools and bzlmod are broken when referring to the package that uses io_bazel.

@comius
Copy link
Contributor Author

comius commented Oct 26, 2022

cc @Wyverald

"@io_bazel//third_party:jsr305",
"@io_bazel//third_party/grpc-java:grpc-jar",
"@io_bazel//third_party:guava",
"@//third_party:javax_annotations",
Copy link
Member

@meteorcloudy meteorcloudy Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think @// will work correctly for Bzlmod, maybe we have to use Label("//third_party:javax_annotations") here? @Wyverald Can confirm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this manually with rules_kotlin and it works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you test with rules_kotlin? Does rules_kotlin somehow uses this macro?

Can you test if bazel build --enable_bzlmod //src:bazel_nojdk still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you test with rules_kotlin? Does rules_kotlin somehow uses this macro?

I have a modded version of it. There's a dep in rules_kotlin, here https://github.com/bazelbuild/rules_kotlin/blob/master/src/main/protobuf/BUILD#L6-L14, that causes to load @bazel_tools//src/main/protobuf/BUILD, that causes to load @bazel_tools//third_party/grpc/build_defs.bzl.

Can you test if bazel build --enable_bzlmod //src:bazel_nojdk still work?

Argh, it fails:

ERROR: /usr/local/google/home/ilist/.cache/bazel/_bazel_ilist/e9eba874c4fa4205502c6d2768696dfb/external/googleapis~override/BUILD.bazel:76:18: no such package '@[unknown repo '' requested from @googleapis~override]//third_party/grpc-java': The repository '@[unknown repo '' requested from @googleapis~override]' could not be resolved: No repository visible as '@' from repository '@googleapis~override' and referenced by '@googleapis~override//:google_devtools_build_v1_publish_build_event_java_grpc'
ERROR: Analysis of target '//src:bazel_nojdk' failed; build aborted: 
INFO: Elapsed time: 18.611s

I'll check if I can fix it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I believe Label("//third_party:javax_annotations") will fix it since the label is resolved to the repo where the bzl file is located which is the same no matter where this macro is used.

Copy link
Contributor Author

@comius comius Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main branch already fails with (maybe I'm doing something wrong):

$ bazel build  //src:bazel
$ bazel-bin/src/bazel build --enable_bzlmod //src:bazel_nojdk --keep_going
File "/usr/local/google/home/ilist/.cache/bazel/_bazel_ilist/e9eba874c4fa4205502c6d2768696dfb/external/grpc~1.47.0~grpc_repo_deps_ext~com_envoyproxy_protoc_gen_validate/bazel/protobuf.bzl", line 225, column 22, in <toplevel>
		Label("@com_google_re2j//jar"),

If I use Label this branch fails with the same errors building bazel_nojdk.

rules_kotlin work as before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main branch is getting fixed by #16578

@sgowroji sgowroji added team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website awaiting-review PR is awaiting review from an assigned reviewer labels Oct 26, 2022
@comius comius added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Oct 26, 2022
@fmeum
Copy link
Collaborator

fmeum commented Oct 26, 2022

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Oct 26, 2022
@comius comius added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Oct 26, 2022
@Wyverald Wyverald 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 Oct 27, 2022
@Wyverald
Copy link
Member

@bazel-io fork 6.0.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Oct 27, 2022
@sgowroji
Copy link
Member

sgowroji commented Oct 27, 2022

Hello @comius @Wyverald For Merging above Thirdparty change could you please squash the commits into one.

Users of @bazel_tools and bzlmod are broken when referring to the package that uses io_bazel.
@comius
Copy link
Contributor Author

comius commented Oct 31, 2022

Hello @comius @Wyverald For Merging above Thirdparty change could you please squash the commits into one.

@sgowroji done

copybara-service bot pushed a commit that referenced this pull request Oct 31, 2022
Users of @bazel_tools and bzlmod are broken when referring to the package that uses io_bazel.

Partial commit for third_party/*, see #16559.

Signed-off-by: Sunil Gowroji <[email protected]>
@sgowroji
Copy link
Member

Merged at 1d514ab

@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Oct 31, 2022
@sgowroji sgowroji closed this Oct 31, 2022
ShreeM01 added a commit that referenced this pull request Nov 2, 2022
Users of @bazel_tools and bzlmod are broken when referring to the package that uses io_bazel.

Partial commit for third_party/*, see #16559.

Signed-off-by: Sunil Gowroji <[email protected]>

Signed-off-by: Sunil Gowroji <[email protected]>
Co-authored-by: Ivo List <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants