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

Fix visibility for //tcmalloc:llvm #70

Closed
wants to merge 1 commit into from

Conversation

meteorcloudy
Copy link

@meteorcloudy meteorcloudy commented Jan 29, 2021

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

@google-cla google-cla bot added the cla: yes label Jan 29, 2021
@meteorcloudy
Copy link
Author

meteorcloudy commented Jan 29, 2021

/cc @ckennelly

@meteorcloudy
Copy link
Author

The test failure doesn't seem to be caused by this PR?

@ckennelly
Copy link
Collaborator

Since the full context for this isn't immediately apparent, can you describe both in the PR and the commit the problem this is addressing?

Additionally, what circumstances are necessary to reproduce it (to ensure that it doesn't backslide)

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
@meteorcloudy
Copy link
Author

Since the full context for this isn't immediately apparent, can you describe both in the PR and the commit the problem this is addressing?

Done.

Additionally, what circumstances are necessary to reproduce it (to ensure that it doesn't backslide)

To reproduce, you'll need a Bazel binary built at bazelbuild/bazel@cac82cf, you can download a prebuilt one via here, then bazel build //tcmalloc/internal should be able to produce the issue.

@ckennelly
Copy link
Collaborator

This has been resolved by 4c76466.

@ckennelly ckennelly closed this Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants