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.

Sign up for GitHub

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

Jump to bottom

Bazel CI: downstream projects broken due to visibility check on select() keys #12925

Closed
meteorcloudy opened this issue Jan 29, 2021 · 10 comments
Closed

Bazel CI: downstream projects broken due to visibility check on select() keys #12925

meteorcloudy opened this issue Jan 29, 2021 · 10 comments
Assignees
@meteorcloudy
Labels
breakage P1 I'll work on this now. (Assignee required) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: bug

Comments

@meteorcloudy
Copy link
Member

https://buildkite.com/bazel/bazel-auto-sheriff-face-with-cowboy-hat/builds/413

Caused by: cac82cf

TensorFlow:

(02:11:27) ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/6ba24013571fc441abf8cb89e73a856e/external/curl/BUILD.bazel:34:11: in cc_library rule @curl//:curl: alias '//tensorflow:macos' referring to target '//tensorflow:macos_arm64' is not visible from target '@curl//:curl'. Check the visibility declaration of the former target if you think the dependency is legitimate

Envoy:

(02:11:05) ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/08eac9c5b682112ef6dbfa42e405abe6/external/com_github_google_tcmalloc/tcmalloc/internal/BUILD:217:11: in cc_library rule @com_github_google_tcmalloc//tcmalloc/internal:mincore: target '@com_github_google_tcmalloc//tcmalloc:llvm' is not visible from target '@com_github_google_tcmalloc//tcmalloc/internal:mincore'. Check the visibility declaration of the former target if you think the dependency is legitimate

I'll try to send PRs to downstream projects to fix this.
@meteorcloudy meteorcloudy self-assigned this Jan 29, 2021
@meteorcloudy meteorcloudy added breakage P1 I'll work on this now. (Assignee required) labels Jan 29, 2021
@meteorcloudy
Copy link
Member Author

FYI @gregestren

@meteorcloudy
Copy link
Member Author

The Envoy breakage is due to com_google_tcmalloc, so I have to fix tcmalloc then update its version in Envoy.

meteorcloudy added a commit to meteorcloudy/tcmalloc that referenced this issue Jan 29, 2021
@meteorcloudy
Fix visibility for //tcmalloc:llvm
53b9004 
Related: bazelbuild/bazel#12925
@meteorcloudy meteorcloudy mentioned this issue Jan 29, 2021
Closed
@meteorcloudy
Copy link
Member Author

FYI @htuch

@gregestren
Copy link
Contributor

Thanks for the info, @meteorcloudy. This is a delicate one. If it comes to it we may have to roll back the PR. But fixing the depot is exactly what we ultimately want to do. I'm not completely clear yet if the first option is necessary.

Keep me updated from your end.

@meteorcloudy
Copy link
Member Author

Not too many downstream projects are broken, and it's an easy fix, so I don't think it should block you from making progress ;)

copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this issue Jan 29, 2021
@tensorflower-gardener
Add missing visibility for //tensorflow:macos
16613a7 
bazel-skylib needs to be updated to include the visibility support for config_setting_group

Related: bazelbuild/bazel#12925
PiperOrigin-RevId: 354532132
Change-Id: I2362b074bbbd78f42daf3a17b7e929901c289aa6
meteorcloudy added a commit to meteorcloudy/tcmalloc that referenced this issue Jan 29, 2021
@meteorcloudy
Fix visibility for //tcmalloc:llvm
8a6623a 
Due to bazelbuild/bazel@cac82cf, future Bazel version will enforce
visibility check on keys of select(). This PR fixes visibilty of
//tcmalloc:llvm so that projects depending on tcmalloc can still be able
to build with Bazel@HEAD.

Related: bazelbuild/bazel#12925
copybara-service bot pushed a commit to google/tcmalloc that referenced this issue Jan 29, 2021
@copybara-github
Due to bazelbuild/bazel@cac82cf, future Bazel version will enforce
4c76466 
visibility check on keys of select(). This PR fixes visibilty of
//tcmalloc:llvm so that projects depending on tcmalloc can still be able
to build with Bazel@HEAD.

Related: bazelbuild/bazel#12925
PiperOrigin-RevId: 354571704
Change-Id: I3414fefc543996ce4090fb5be4385d6e69d0c4b7
@htuch
Copy link

htuch commented Jan 29, 2021

@yanavlasov for visibility; @meteorcloudy Yan is the new Envoy - Bazel relationship owner :)

@meteorcloudy
Copy link
Member Author

@htuch Thanks for the introduce.
@yanavlasov Can you help upgrading tcmalloc to the latest commit to fix this breakage?
google/tcmalloc@4c76466

@sventiffe sventiffe added team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: bug labels Feb 10, 2021
@sventiffe
Copy link
Contributor

Any update on this issue?

@meteorcloudy
Copy link
Member Author

It looks like Envoy has been green for a while. I believe they have fixed the breakage. Closing.

@gregestren
Copy link
Contributor

Note this is getting reintroduced behind two incompatible flags:

But thank you for the effort you'd already put in to clean up ahead of time.

suiliangchu added a commit to suiliangchu/tcmalloc that referenced this issue Aug 29, 2022
@suiliangchu @copybara-github
Due to bazelbuild/bazel@cac82cf, future Bazel version will enforce
8f509e8 
visibility check on keys of select(). This PR fixes visibilty of
//tcmalloc:llvm so that projects depending on tcmalloc can still be able
to build with Bazel@HEAD.

Related: bazelbuild/bazel#12925
PiperOrigin-RevId: 354571704
Change-Id: I3414fefc543996ce4090fb5be4385d6e69d0c4b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@meteorcloudy meteorcloudy

Labels
breakage P1 I'll work on this now. (Assignee required) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@meteorcloudy @gregestren @htuch @sventiffe