bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Ability to silence visibility warnings (or filter warnings in general)

Open GMNGeoffrey opened this issue 3 years ago • 6 comments

Description of the feature request:

I would like the ability to silence warnings like:

WARNING: /really/long/path/BUILD:26:25: in cc_library rule //path/to/package:target: Target '//path/to/package/again:target' violates visibility of target '//some/other/target'. Continuing because --nocheck_visibility is active

These only occur when someone has set --nocheck_visibility, so it seems pretty likely that they are not interested in the warning. Perhaps they are interested in collecting all visibility warnings, but it seems like they could just pass --keep_going and look at the errors.

Some ideas for how to solve this:

  1. Don't show these warnings when --nocheck_visibility is set. Presumably the person who set this does not want to check visibility, so warning them about it is not helpful.
  2. Add a flag for filtering warnings by content (current flags appear to only do so my target).
  3. Add a separate flag for turning off visibility warnings. This one seems a bit weird, but would be backward compatible, unlike 1.
  4. Deprecate --[no]check_visibility as a boolean flag and create a new flag --visibility (or something) with options "error", "warning", "off"
  5. Somehow make --[no]check_visibility still accept its current options but also allow an argument to be passed turning off the warnings. This seems unlikely to work with your flag parsing library, but who knows.

What underlying problem are you trying to solve with this feature?

Really noisy Bazel build logs. Visibility just doesn't function very well in a multi-repo world and so we frequently turn it off. When doing so, it's not very helpful to get a page full of warnings. Here are some example logs, though I've experienced much worse than this in other cases: https://github.com/iree-org/iree/actions/runs/3391208935/jobs/5636191652.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 5.1.0

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 ?

`git remote get-url upstream; git rev-parse main; git rev-parse HEAD`

https://github.com/iree-org/iree.git
03190c689ae29bca4bd5ea71d5793811c47a90a2
4080c2062c46fe8dd1582f10223aaddbdae91c2e

Have you found anything relevant by searching the web?

Not really. There are output filter options, but I don't think any of them achieve what I want.

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

No response

GMNGeoffrey avatar Nov 14 '22 22:11 GMNGeoffrey

Dupe of https://github.com/bazelbuild/bazel/issues/12358

sgowroji avatar Nov 15 '22 04:11 sgowroji

I feel like most people solve this today by just aggressively marking everything as public

keith avatar Nov 15 '22 05:11 keith

Dupe of #12358

I don't think that's accurate. In fact the --output_filter and in particular --auto_output_filter seems to cover that use case (I haven't checked, since no one ever responded to that issue).

I feel like most people solve this today by just aggressively marking everything as public

Indeed, that's what I meant about visibility being basically non-functional in a multi-repo world. This doesn't work for dependencies on an external project that has failed to do this though (cough tensorflow cough), which is what I'm dealing with

GMNGeoffrey avatar Nov 15 '22 18:11 GMNGeoffrey

Some gut reactions:

I am ideologically opposed to warning spam. But that doesn't mean I want to go on a campaign to eliminate them from the console right now, or that others on the team are of like mind. If we don't take a hard line on minimizing or eliminating warning spam, then we should mitigate it with a generalized --output_filter style feature. (Edit: filed #17030)

I am also ideologically opposed to disabling visibility checks, and feel like warning spam is a suitable punishment for this heresy. I'm disturbed that people are opting out of visibility checking in production. If upstream repos are using visibility incorrectly, they should be fixed. But if that's not happening in practice, then users need an escape hatch that doesn't simply disable visibility for the entire workspace. Otherwise we have a contagion effect.

@Wyverald, @meteorcloudy: Perhaps there should be a workspace/bzlmod feature to ignore visibilities for a given repo?

brandjon avatar Dec 14 '22 16:12 brandjon

I am also ideologically opposed to disabling visibility checks, and feel like warning spam is a suitable punishment for this heresy. I'm disturbed that people are opting out of visibility checking in production. If upstream repos are using visibility incorrectly, they should be fixed.

I think you're overestimating the usability of the current visibility system outside of a monorepo. How would a project declare that some other project is allowed to use something? For instance, our project has visibility into some TF stuff inside of Google's monorepo. In OSS though, that stuff isn't //visibility:public, so we can't use it. The philosophy of restricting someone from using something is also very different in OSS than it is within Google. At Google, if you give someone visibility into your project, you're on the hook for making sure they don't break. In OSS, that same thing does not apply, so you don't need to be as worried about locking down visibility and the idea that someone would need to ask you to add them to a source file in your repository before you could use their "open source" software is absolutely wild. I've thought before about how Bazel could make visibility actually useful in OSS. I think it would basically require that each project set visibilities to some named categories and then downstream projects could map that to visibility groups within their own build. Also required, I think, would be for visibility groups to be more composable. Right now, there's weird interactions between package_group the named "special" visibilities that pretend to be located in the main repo, and things like __subpackages__. For now though, the thing that works and is actually implemented is --nocheck_visibility and aggressively marking everything public.

GMNGeoffrey avatar Dec 14 '22 21:12 GMNGeoffrey

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

github-actions[bot] avatar Feb 18 '24 01:02 github-actions[bot]

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please post @bazelbuild/triage in a comment here and we'll take a look. Thanks!

github-actions[bot] avatar May 18 '24 01:05 github-actions[bot]