Skip to content

Fix bazel crash when passing config_setting to target_compatible_with#13254

Closed
philsc wants to merge 1 commit intobazelbuild:masterfrom
philsc:unreviewed/restore-error-checking-publish
Closed

Fix bazel crash when passing config_setting to target_compatible_with#13254
philsc wants to merge 1 commit intobazelbuild:masterfrom
philsc:unreviewed/restore-error-checking-publish

Conversation

@philsc
Copy link
Copy Markdown
Contributor

@philsc philsc commented Mar 21, 2021

When you pass a config_setting into a target's
target_compatible_with attribute, we see the following crash:

FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=//:hello, config=BuildConfigurationValue.Key[86e57527e1c8bb10edc853e734cb858f8159d8f3e0a4df9ceb16f80aad784b93]}' (requested by nodes )
        at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:563)
        at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:398)
        at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.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.NullPointerException
        at com.google.devtools.build.lib.analysis.platform.ConstraintCollection.hasConstraintValue(ConstraintCollection.java:216)
        at com.google.devtools.build.lib.analysis.RuleContext.targetPlatformHasConstraint(RuleContext.java:1226)
        at com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics.lambda$incompatibleConfiguredTarget$1(RuleContextConstraintSemantics.java:904)
        at java.base/java.util.stream.ReferencePipeline$2$1.accept(Unknown Source)
        at com.google.common.collect.CollectSpliterators$1.lambda$forEachRemaining$1(CollectSpliterators.java:120)
        at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(Unknown Source)
        at com.google.common.collect.CollectSpliterators$1.forEachRemaining(CollectSpliterators.java:120)
        at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source)
        at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source)
        at java.base/java.util.stream.ReferencePipeline.collect(Unknown Source)
        at com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics.incompatibleConfiguredTarget(RuleContextConstraintSemantics.java:905)
        at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:327)
        at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:194)
        at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:938)
        at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:1013)
        at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:371)
        at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:477)
        ... 7 more

This happens because we discard various errors generated in the
RuleContext builder when the target_compatible_with attribute is
specified. This is not the desired behaviour.

This patch changes the code so that the RuleContext errors are
checked before evaluating the target_compatible_with attribute. That
way Bazel can tell the user something's wrong without crashing. In the
above example we now see this error message:

ERROR: /bazel-cache/phil/bazel/_bazel_phil/c2296ba7b90d1074a9fe6e8d3b871222/sandbox/linux-sandbox/129/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:101:10: in target_compatible_with attribute of sh_binary rule //target_skipping:problematic_foo3_target: '//target_skipping:foo3_config_setting' does not have mandatory providers: 'ConstraintValueInfo'

That should be sufficient to let the user know that they need to
specify constraint_value targets instead.

Fixes #13250

When you pass a `config_setting` into a target's
`target_compatible_with` attribute, we see the following crash:

    FATAL: bazel crashed due to an internal error. Printing stack trace:
    java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=//:hello, config=BuildConfigurationValue.Key[86e57527e1c8bb10edc853e734cb858f8159d8f3e0a4df9ceb16f80aad784b93]}' (requested by nodes )
            at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:563)
            at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:398)
            at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.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.NullPointerException
            at com.google.devtools.build.lib.analysis.platform.ConstraintCollection.hasConstraintValue(ConstraintCollection.java:216)
            at com.google.devtools.build.lib.analysis.RuleContext.targetPlatformHasConstraint(RuleContext.java:1226)
            at com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics.lambda$incompatibleConfiguredTarget$1(RuleContextConstraintSemantics.java:904)
            at java.base/java.util.stream.ReferencePipeline$2$1.accept(Unknown Source)
            at com.google.common.collect.CollectSpliterators$1.lambda$forEachRemaining$1(CollectSpliterators.java:120)
            at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(Unknown Source)
            at com.google.common.collect.CollectSpliterators$1.forEachRemaining(CollectSpliterators.java:120)
            at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source)
            at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
            at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source)
            at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source)
            at java.base/java.util.stream.ReferencePipeline.collect(Unknown Source)
            at com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics.incompatibleConfiguredTarget(RuleContextConstraintSemantics.java:905)
            at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:327)
            at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:194)
            at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:938)
            at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:1013)
            at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:371)
            at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:477)
            ... 7 more

This happens because we discard various errors generated in the
`RuleContext` builder when the `target_compatible_with` attribute is
specified. This is not the desired behaviour.

This patch changes the code so that the `RuleContext` errors are
checked before evaluating the `target_compatible_with` attribute. That
way Bazel can tell the user something's wrong without crashing. In the
above example we now see this error message:

    ERROR: /bazel-cache/phil/bazel/_bazel_phil/c2296ba7b90d1074a9fe6e8d3b871222/sandbox/linux-sandbox/129/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:101:10: in target_compatible_with attribute of sh_binary rule //target_skipping:problematic_foo3_target: '//target_skipping:foo3_config_setting' does not have mandatory providers: 'ConstraintValueInfo'

That should be sufficient to let the user know that they need to
specify `constraint_value` targets instead.
@google-cla google-cla Bot added the cla: yes label Mar 21, 2021
@philsc
Copy link
Copy Markdown
Contributor Author

philsc commented Mar 21, 2021

@gregestren , @katre , FYI

Copy link
Copy Markdown
Collaborator

@katre katre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'll merge this today.

@bazel-io bazel-io closed this in 706f5ac Mar 22, 2021
@philsc philsc deleted the unreviewed/restore-error-checking-publish branch March 22, 2021 15:34
philwo pushed a commit that referenced this pull request Apr 19, 2021
When you pass a `config_setting` into a target's
`target_compatible_with` attribute, we see the following crash:

    FATAL: bazel crashed due to an internal error. Printing stack trace:
    java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=//:hello, config=BuildConfigurationValue.Key[86e57527e1c8bb10edc853e734cb858f8159d8f3e0a4df9ceb16f80aad784b93]}' (requested by nodes )
            at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:563)
            at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:398)
            at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.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.NullPointerException
            at com.google.devtools.build.lib.analysis.platform.ConstraintCollection.hasConstraintValue(ConstraintCollection.java:216)
            at com.google.devtools.build.lib.analysis.RuleContext.targetPlatformHasConstraint(RuleContext.java:1226)
            at com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics.lambda$incompatibleConfiguredTarget$1(RuleContextConstraintSemantics.java:904)
            at java.base/java.util.stream.ReferencePipeline$2$1.accept(Unknown Source)
            at com.google.common.collect.CollectSpliterators$1.lambda$forEachRemaining$1(CollectSpliterators.java:120)
            at java.base/java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(Unknown Source)
            at com.google.common.collect.CollectSpliterators$1.forEachRemaining(CollectSpliterators.java:120)
            at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source)
            at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
            at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source)
            at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source)
            at java.base/java.util.stream.ReferencePipeline.collect(Unknown Source)
            at com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics.incompatibleConfiguredTarget(RuleContextConstraintSemantics.java:905)
            at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:327)
            at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:194)
            at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:938)
            at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:1013)
            at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:371)
            at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:477)
            ... 7 more

This happens because we discard various errors generated in the
`RuleContext` builder when the `target_compatible_with` attribute is
specified. This is not the desired behaviour.

This patch changes the code so that the `RuleContext` errors are
checked before evaluating the `target_compatible_with` attribute. That
way Bazel can tell the user something's wrong without crashing. In the
above example we now see this error message:

    ERROR: /bazel-cache/phil/bazel/_bazel_phil/c2296ba7b90d1074a9fe6e8d3b871222/sandbox/linux-sandbox/129/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:101:10: in target_compatible_with attribute of sh_binary rule //target_skipping:problematic_foo3_target: '//target_skipping:foo3_config_setting' does not have mandatory providers: 'ConstraintValueInfo'

That should be sufficient to let the user know that they need to
specify `constraint_value` targets instead.

Fixes #13250

Closes #13254.

PiperOrigin-RevId: 364308810
@aaron-michaux
Copy link
Copy Markdown

The documentation says:

target_settings  |  List of labels ; optional
A list of config_settings that must be satisfied by the target configuration in order for this toolchain to be selected during toolchain resolution.

But it should say config_values?????

@katre
Copy link
Copy Markdown
Collaborator

katre commented Nov 7, 2022

No, the target_settings attribute requires values of config_setting, not constraint_values. They are used differently and have different semantics.

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.

Bazel crashes when specifying target_compatible_with to rust_binary

3 participants