incompatible_config_setting_private_default_visibility #12933

gregestren · 2021-01-29T22:44:49Z

Visibility on config_setting isn't historically enforced. This is purely for legacy reasons. There's no philosophical reason to distinguish them.

This flag, in conjunction with --incompatible_enforce_config_setting_visibility (#12932), removes that distinction.

Values:

--incompatible_config_setting_private_default_visibility=off: if --incompatible_enforce_config_setting_visibility=off, every config_setting is visible to every target, regardless of visibility settings. Else, every config_setting without an explicit visibility setting is //visibility:public (ignoring package visibility defaults)
--incompatible_config_setting_private_default_visibility=on: if --incompatible_enforce_config_setting_visibility=off, every config_setting is visible to every target, regardless of visibility settings. Else, config_setting follows the same visibility rules as all other targets.

Incompatibility error:

ERROR: myapp/BUILD:4:1: in config_setting rule //myapp:my_config: target 'myapp:my_config' is not visible from target '//some:other_target. Check the visibility declaration of the former target if you think the dependency is legitimate

Migration:

Treat all config_settings as if they follow standard visibility logic at https://docs.bazel.build/versions/master/visibility.html: have them set visibility explicitly if they'll be used anywhere outside their own package. The ultimate goal of this migration is to fully enforce that expectation.

The text was updated successfully, but these errors were encountered:

This was rolled back in 36d228b because of depot breakages. This version adds incompatible flags to safely introduce enforcement. See #12932 and #12933 for flag settings. *** Original change description *** Roll back #12877. *** Fixes #12669. RELNOTES: enforce config_setting visibility. See #12932 for details. PiperOrigin-RevId: 354930807

This was rolled back in bazelbuild@36d228b because of depot breakages. This version adds incompatible flags to safely introduce enforcement. See bazelbuild#12932 and bazelbuild#12933 for flag settings. *** Original change description *** Roll back bazelbuild#12877. *** Fixes bazelbuild#12669. RELNOTES: enforce config_setting visibility. See bazelbuild#12932 for details. PiperOrigin-RevId: 354930807

This was rolled back in 36d228b because of depot breakages. This version adds incompatible flags to safely introduce enforcement. See #12932 and #12933 for flag settings. *** Original change description *** Roll back #12877. *** Fixes #12669. RELNOTES: enforce config_setting visibility. See #12932 for details. PiperOrigin-RevId: 354930807

meteorcloudy · 2022-09-14T17:15:13Z

I assume this flag is migration ready? Should we flip it before 6.0 cut?

gregestren · 2022-09-14T20:21:06Z

What was the risk calculation for flipping? I can't guarantee repos won't break. If that's okay, I support pushing this forward.

More specifically, if we're not worried about initial breakages, I'd flip both this and #12932. If we want to be more careful I'd just flip #12932. If we don't want any change of any breakages I wouldn't flip.

I'm hoping the answer is "flip both and adjust whatever breakages we see."

The reason this isn't trivial is repos can already set visibility on config_setting. So if a repo has config_setting, visibility = '//visibility:private'), today it's actually public and may be depended on by targets in other packages. If we flip these flags then suddenly we give that declaration meaning and, while technically correct, existing dependencies may no longer be valid.

So it's definitely a strict improvement but we have to get mislabeled repo code fixed up for it to finally stick.

meteorcloudy · 2022-09-15T09:14:04Z

I'm hoping the answer is "flip both and adjust whatever breakages we see."

We should do it in the reverse order to make sure downstream is green, otherwise we'll lose the ability to catch other CI breakages.

I recently updated our incompatible change process: https://bazel.build/contribute/breaking-changes#update-repos

With the incompatible flags pipeline, we can detect those breakages before flipping the flag, check this doc

I just added the "migration-ready" label for both flag, fixed the issue name and triggered a rerun, you can see the result here https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/1256

meteorcloudy · 2022-09-15T11:55:51Z

The result looks good. You can ignore the rules_nodejs failure. But except that,

--incompatible_config_setting_private_default_visibility didn't break any downstream project, so you can flip it.

--incompatible_enforce_config_setting_visibility is breaking Envoy and TensorFlow, if you can file issue or send PR to fix them before flipping the flag, it will be great!

gregestren · 2022-09-15T20:10:08Z

Great! What's my deadline for fixing Envoy and TensorFlow and flipping --incompatible_enforce_config_setting_visibility? That's the more important one to set first.

meteorcloudy · 2022-09-16T08:46:53Z

6.0 is currently scheduled to be cut on Sep 26, see #16159

But there are still many release blockers, see https://github.com/bazelbuild/bazel/issues?q=is%3Aopen+is%3Aissue+milestone%3A%226.0.0+release+blockers%22, and it's likely the cut day will be postponed again.

We can add the milestone to both tracking issues, so we won't forget ;)

meteorcloudy · 2022-09-20T09:20:18Z

Bumping to P1 since it's blocking 6.0 release

keertk · 2022-11-23T22:15:41Z

For tracking:
cc @meteorcloudy

Failures: https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/1341

To fix:

…bazelrc to prevent regression Imported from GitHub PR tensorflow/tensorflow#58832 Related: bazelbuild/bazel#12932 and bazelbuild/bazel#12933 Copybara import of the project: -- b4edfd47bd9a0c0363a9feb1eb1b48d067e4644e by Yun Peng <[email protected]>: Flip two incompatible flags in .bazelrc to prevent regression -- c33c0dd6e5744393fe2b1909efc91dc19bd6dda1 by Yun Peng <[email protected]>: Add visibility to more targets -- 34d1bfe1564f0905993c26a891f2385b83480e3d by Yun Peng <[email protected]>: --incompatible_config_setting_private_default_visibility cannot be flipped yet Merging this change closes #58832 PiperOrigin-RevId: 494895621

gregestren · 2023-02-01T18:41:24Z

Thanks for recent updates, @keertk . I'm excited to see you making this 7.0 compatible. Is there an appropriate 7.0 Github label for this issue? Or does migration-ready already cover it?

keertk · 2023-02-01T18:48:21Z

Hey @gregestren, just created/added breaking-change-7.0. Thanks!

…g_private_default_visibility to true This was rolled back in bazelbuild@d4fc6db due to failing Bazel downstream tests. I fixed as many as I was aware of: see latest comments on bazelbuild#12933. Fixes bazelbuild#12933. PiperOrigin-RevId: 485694298 Change-Id: Ie38ce2f6cdbf952a208de37a96658770004b2bf8

@Head

*** Reason for rollback *** Incompatible pipeline still showing failures: https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/1320 Need to investigate why some fixes @Head seem not to work. *** Original change description *** Roll forward bazelbuild@1fdc628: default --incompatible_config_setting_private_default_visibility to true This was rolled back in bazelbuild@d4fc6db due to failing Bazel downstream tests. I fixed as many as I was aware of: see latest comments on bazelbuild#12933. Fixes bazelbuild#12933. PiperOrigin-RevId: 485901094 Change-Id: Ifed7424c482d4feda76811838209e3e10607471b

gregestren mentioned this issue Jan 29, 2021

incompatible_enforce_config_setting_visibility #12932

Closed

gregestren added incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions labels Jan 29, 2021

gregestren self-assigned this Jan 29, 2021

gregestren changed the title ~~Incompatible change: --incompatible_config_setting_public_default_visibility~~ Incompatible change: --incompatible_config_setting_private_default_visibility Jan 29, 2021

gregestren mentioned this issue Feb 10, 2021

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

Closed

gregestren added the migration-ready label Feb 12, 2021

gregestren mentioned this issue Jul 16, 2021

git cherry-pick for 4.2.0: flag-gated config_setting visibility #13701

Closed

gregestren added the type: process label Feb 1, 2022

alexeagle mentioned this issue Mar 22, 2022

Flip default for --incompatible_config_setting_private_default_visibility bazel-contrib/SIG-rules-authors#34

Closed

sgowroji removed the migration-ready label Apr 21, 2022

meteorcloudy added the migration-ready Incompatible flag is ready for migration with Bazel rolling releases or Bazel@last_green label Sep 14, 2022

meteorcloudy changed the title ~~Incompatible change: --incompatible_config_setting_private_default_visibility~~ incompatible_config_setting_private_default_visibility Sep 15, 2022

meteorcloudy added this to the 6.0.0 release blockers milestone Sep 16, 2022

meteorcloudy added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Sep 20, 2022

meteorcloudy added the breaking-change-6.0 Incompatible flags to be flipped in Bazel 6.0 label Sep 22, 2022

lberki added P2 We'll consider working on this in future. (Assignee optional) and removed P1 I'll work on this now. (Assignee required) labels Oct 4, 2022

meteorcloudy mentioned this issue Nov 30, 2022

incompatible_strict_conflict_checks #16729

Closed

meteorcloudy removed the breaking-change-6.0 Incompatible flags to be flipped in Bazel 6.0 label Dec 2, 2022

keertk mentioned this issue Dec 7, 2022

Flag --incompatible_config_setting_private_default_visibility will break rules_go in Bazel 7.0 bazel-contrib/rules_go#3386

Closed

meteorcloudy removed this from the 6.0.0 nice to have milestone Jan 4, 2023

keertk self-assigned this Jan 30, 2023

gregestren mentioned this issue Feb 1, 2023

Unexpected visibility limitations with config_setting visibility bazelbuild/bazel-skylib#404

Open

keertk added the breaking-change-7.0 Incompatible flags to be flipped in Bazel 7.0 label Feb 1, 2023

gregestren mentioned this issue Feb 15, 2023

test_suite visibility not checked by test or aquery commands #14053

Closed

cpsauer mentioned this issue May 11, 2023

Flip default for --incompatible_enforce_config_setting_visibility bazel-contrib/SIG-rules-authors#39

Open

gregestren mentioned this issue Aug 1, 2023

Always check $config_dependencies visibility at use #19139

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

incompatible_config_setting_private_default_visibility #12933

incompatible_config_setting_private_default_visibility #12933

gregestren commented Jan 29, 2021 •

edited

Loading

meteorcloudy commented Sep 14, 2022

gregestren commented Sep 14, 2022 •

edited

Loading

meteorcloudy commented Sep 15, 2022 •

edited

Loading

meteorcloudy commented Sep 15, 2022

gregestren commented Sep 15, 2022

meteorcloudy commented Sep 16, 2022

meteorcloudy commented Sep 20, 2022

keertk commented Nov 23, 2022 •

edited by sgowroji

Loading

gregestren commented Feb 1, 2023

keertk commented Feb 1, 2023

incompatible_config_setting_private_default_visibility #12933

incompatible_config_setting_private_default_visibility #12933

Comments

gregestren commented Jan 29, 2021 • edited Loading

meteorcloudy commented Sep 14, 2022

gregestren commented Sep 14, 2022 • edited Loading

meteorcloudy commented Sep 15, 2022 • edited Loading

meteorcloudy commented Sep 15, 2022

gregestren commented Sep 15, 2022

meteorcloudy commented Sep 16, 2022

meteorcloudy commented Sep 20, 2022

keertk commented Nov 23, 2022 • edited by sgowroji Loading

gregestren commented Feb 1, 2023

keertk commented Feb 1, 2023

gregestren commented Jan 29, 2021 •

edited

Loading

gregestren commented Sep 14, 2022 •

edited

Loading

meteorcloudy commented Sep 15, 2022 •

edited

Loading

keertk commented Nov 23, 2022 •

edited by sgowroji

Loading