incompatible_enforce_config_setting_visibility

[gregestren]

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

This flag starts the process of removing the distinction.

Values:

• --incompatible_enforce_config_setting_visibility=off: every config_setting is visible to every target, regardless of visibility settings
• --incompatible_enforce_config_setting_visibility=on: config_setting follows the policy set by --incompatible_config_setting_private_default_visibility (https://github.com/bazelbuild/bazel/issues/12933).

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.

[meteorcloudy]

Bumping to P1 since it's blocking 6.0 release

[lberki]

Bumping this down to P2 to indicate our collective desire to get this into Bazel 6.0, but also that we won't postpone the release cut for this, should this and other P2s be the only things that remain.

[meteorcloudy]

I believe it's due to https://github.com/bazelbuild/bazel/commit/a3a4cf8c486d1d27e72d9e595c9d718efb21071a, some projects are failing with

(01:17:57) FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=//app:force_build_apks_test, config=BuildConfigurationKey[678d252511456a6024fd604054b0aaecc42f4b9f9497e55372fe2a819c340ec3]}' (requested by nodes )
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:642)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:382)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.lang.IllegalStateException: Configurations don't match: BuildConfigurationKey[678d252511456a6024fd604054b0aaecc42f4b9f9497e55372fe2a819c340ec3] BuildConfigurationKey[4e5d16177afbc1a0a7486bfbd8ea79ab6c17a06b75acfedecdc9fee2ac3e1976] 678d252511456a6024fd604054b0aaecc42f4b9f9497e55372fe2a819c340ec3 (ConfiguredTarget(@bazel_tools//src/conditions:host_windows_x64_constraint, 4e5d16177afbc1a0a7486bfbd8ea79ab6c17a06b75acfedecdc9fee2ac3e1976) config_setting rule @bazel_tools//src/conditions:host_windows_x64_constraint)
	at com.google.common.base.Preconditions.checkState(Preconditions.java:534)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData.<init>(ConfiguredTargetAndData.java:72)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData.fromConfiguredTarget(ConfiguredTargetAndData.java:124)
	at com.google.devtools.build.lib.analysis.RuleContext$Builder.build(RuleContext.java:1759)
	at com.google.devtools.build.lib.analysis.RuleContext$Builder.build(RuleContext.java:1704)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:324)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:192)
	at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:1231)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:1197)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:414)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:571)
	... 7 more

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

[gregestren]

Thanks @meteorcloudy . Looking...

[gregestren]

Where is this failure in https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/1294?

[gregestren]

https://bazel-review.googlesource.com/c/bazel/+/205341 will fix.

[meteorcloudy]

Where is this failure in https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/1294?

If you click any of those jobs:

[cpsauer]

Hey, uh, guys...I definitely have config_setttings with unset visibility that aren't erroring when used from selects() outside their packages. That's unexpected, right?

(True with --incompatible_enforce_config_setting_visibility and on bazel rolling 7.0.0-pre.20230426.1.)

[gregestren]

This migration is confusing (sorry) because there are two flags.

--incompatible_enforce_config_setting_visibility is already set to default at head. With that flag every config_setting is public unless it specifically overrides.

https://github.com/bazelbuild/bazel/issues/12933 changes the default to private. This is not yet flipped at head. As I understand from the comment thread @keertk is working through CI fixups.

[cpsauer]

Sorry--on a second read, I'm realizing you'd said above. Sorry for the false alarm, Greg, and thanks as always for being patient, informative, and great.

In recompense, I'll work to make sure our deps (like rules_boost, where I have a commit bit) all work with it and update the issue that brought me here.