diff --git a/site/en/docs/platforms.md b/site/en/docs/platforms.md index 256db39d6e49d0..9284b34c939752 100644 --- a/site/en/docs/platforms.md +++ b/site/en/docs/platforms.md @@ -245,3 +245,8 @@ def format(target): $ bazel cquery //... --output=starlark --starlark:file=example.cquery ``` + +### Known Issues + +Incompatible targets [ignore visibility +restrictions](https://github.com/bazelbuild/bazel/issues/16044). diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 5f115fbcacb940..33dc8f06a97a2b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -2208,6 +2208,41 @@ java_library( ], ) +java_library( + name = "constraints/incompatible_target_checker", + srcs = ["constraints/IncompatibleTargetChecker.java"], + deps = [ + ":analysis_cluster", + ":file_provider", + ":incompatible_platform_provider", + ":test/test_configuration", + ":transitive_info_provider_map_builder", + "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/analysis/platform", + "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils", + "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", + "//src/main/java/com/google/devtools/build/lib/analysis:config/config_conditions", + "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", + "//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value", + "//src/main/java/com/google/devtools/build/lib/analysis:dependency", + "//src/main/java/com/google/devtools/build/lib/analysis:dependency_kind", + "//src/main/java/com/google/devtools/build/lib/analysis:target_and_configuration", + "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection", + "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/collect/nestedset", + "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages:configured_attribute_mapper", + "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data", + "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key", + "//src/main/java/com/google/devtools/build/lib/skyframe:rule_configured_target_value", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/main/java/com/google/devtools/build/lib/util", + "//src/main/java/com/google/devtools/build/skyframe", + "//third_party:auto_value", + "//third_party:guava", + ], +) + # TODO(b/144899336): This should be analysis/starlark/BUILD java_library( name = "starlark/args", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index fbbaec9cc72358..bc14130e9cb3fb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -403,8 +403,8 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) .add( attr("distribs", DISTRIBUTIONS) .nonconfigurable("Used in core loading phase logic with no access to configs")) - // Any rule that has provides its own meaning for the "target_compatible_with" attribute - // has to be excluded in `RuleContextConstraintSemantics.incompatibleConfiguredTarget()`. + // Any rule that provides its own meaning for the "target_compatible_with" attribute + // has to be excluded in `IncompatibleTargetChecker`. .add( attr(RuleClass.TARGET_COMPATIBLE_WITH_ATTR, LABEL_LIST) .mandatoryProviders(ConstraintValueInfo.PROVIDER.id()) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 390092c3623c04..977a65b29c9081 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget; import com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; -import com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics; import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleConfiguredTargetUtil; import com.google.devtools.build.lib.analysis.test.AnalysisFailure; import com.google.devtools.build.lib.analysis.test.AnalysisFailureInfo; @@ -325,12 +324,6 @@ private ConfiguredTarget createRule( return erroredConfiguredTarget(ruleContext); } - ConfiguredTarget incompatibleTarget = - RuleContextConstraintSemantics.incompatibleConfiguredTarget(ruleContext, prerequisiteMap); - if (incompatibleTarget != null) { - return incompatibleTarget; - } - try { Class missingFragmentClass = null; for (Class fragmentClass : diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 98eab47cd14c8a..4ce591a69f5318 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -53,7 +53,6 @@ import com.google.devtools.build.lib.analysis.config.FragmentCollection; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; -import com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics; import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext; @@ -2180,14 +2179,6 @@ private static boolean checkRuleDependencyMandatoryProviders( private void validateDirectPrerequisite( Attribute attribute, ConfiguredTargetAndData prerequisite) { - if (RuleContextConstraintSemantics.checkForIncompatibility(prerequisite.getConfiguredTarget()) - .isIncompatible()) { - // If the prerequisite is incompatible (e.g. has an incompatible provider), we pretend that - // there is no further validation needed. Otherwise, it would be difficult to make the - // incompatible target satisfy things like required providers and file extensions. - return; - } - validateDirectPrerequisiteType(prerequisite, attribute); validateDirectPrerequisiteFileTypes(prerequisite, attribute); if (attribute.performPrereqValidatorCheck()) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index df040755f46277..ef668a7974a8be 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -448,22 +448,6 @@ public static RunfilesSupport withExecutable( computeActionEnvironment(ruleContext)); } - /** - * Creates and returns a {@link RunfilesSupport} object for the given rule and executable. This - * version discards all arguments. Only use this for Incompatible Target - * Skipping. - */ - public static RunfilesSupport withExecutableButNoArgs( - RuleContext ruleContext, Runfiles runfiles, Artifact executable) { - return RunfilesSupport.create( - ruleContext, - executable, - runfiles, - CommandLine.EMPTY, - computeActionEnvironment(ruleContext)); - } - private static CommandLine computeArgs(RuleContext ruleContext, CommandLine additionalArgs) { if (!ruleContext.getRule().isAttrDefined("args", Type.STRING_LIST)) { // Some non-_binary rules create RunfilesSupport instances; it is fine to not have an args diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java index eb6b28f72c3e6e..c52131d3a536a4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.FilesToRunProvider; +import com.google.devtools.build.lib.analysis.IncompatiblePlatformProvider; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; @@ -155,6 +156,28 @@ public RuleConfiguredTarget( } } + /** Use this constructor for creating incompatible ConfiguredTarget instances. */ + public RuleConfiguredTarget( + Label label, + BuildConfigurationKey configurationKey, + NestedSet visibility, + TransitiveInfoProviderMap providers, + ImmutableMap configConditions, + String ruleClassString) { + this( + label, + configurationKey, + visibility, + providers, + configConditions, + ImmutableSet.of(), + ruleClassString, + ImmutableList.of(), + ImmutableMap.of()); + + Preconditions.checkState(providers.get(IncompatiblePlatformProvider.PROVIDER) != null, label); + } + /** The configuration conditions that trigger this rule's configurable attributes. */ @Override public ImmutableMap getConfigConditions() { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java new file mode 100644 index 00000000000000..5f5a306575b8d3 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java @@ -0,0 +1,272 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.analysis.constraints; + +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.ConfiguredTargetValue; +import com.google.devtools.build.lib.analysis.Dependency; +import com.google.devtools.build.lib.analysis.DependencyKind; +import com.google.devtools.build.lib.analysis.FileProvider; +import com.google.devtools.build.lib.analysis.FilesToRunProvider; +import com.google.devtools.build.lib.analysis.IncompatiblePlatformProvider; +import com.google.devtools.build.lib.analysis.Runfiles; +import com.google.devtools.build.lib.analysis.RunfilesProvider; +import com.google.devtools.build.lib.analysis.TargetAndConfiguration; +import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMapBuilder; +import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; +import com.google.devtools.build.lib.analysis.config.ConfigConditions; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; +import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo; +import com.google.devtools.build.lib.analysis.platform.PlatformInfo; +import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils; +import com.google.devtools.build.lib.analysis.test.TestActionBuilder; +import com.google.devtools.build.lib.analysis.test.TestConfiguration; +import com.google.devtools.build.lib.analysis.test.TestProvider; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.packages.BuildType; +import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper; +import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.packages.PackageSpecification; +import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents; +import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; +import com.google.devtools.build.lib.skyframe.RuleConfiguredTargetValue; +import com.google.devtools.build.lib.util.OrderedSetMultimap; +import com.google.devtools.build.skyframe.SkyFunction.Environment; +import com.google.devtools.build.skyframe.SkyframeLookupResult; +import java.util.List; +import java.util.Optional; +import javax.annotation.Nullable; + +/** + * Helpers for creating configured targets that are incompatible. + * + *

A target is considered incompatible if any of the following applies: + * + *

    + *
  1. The target's target_compatible_with attribute specifies a constraint that is + * not present in the target platform. The target is said to be "directly incompatible". + *
  2. One or more of the target's dependencies is incompatible. The target is said to be + * "indirectly incompatible." + *
+ * + * The intent of these helpers is that they get called as early in the analysis phase as possible. + * That's why there are two helpers instead of just one. The first helper determines direct + * incompatibility very early in the analysis phase. If a target is not directly incompatible, the + * dependencies need to be analysed and then we can check for indirect incompatibility. Doing these + * checks as early as possible allows us to skip analysing unused dependencies and ignore unused + * toolchains. + * + *

See https://bazel.build/docs/platforms#skipping-incompatible-targets for more information on + * incompatible target skipping. + */ +public class IncompatibleTargetChecker { + /** + * Creates an incompatible configured target if it is "directly incompatible". + * + *

In other words, this function checks if a target is incompatible because of its + * "target_compatible_with" attribute. + * + *

This function returns a nullable {@code Optional} of a {@link RuleConfiguredTargetValue}. + * This provides three states of return values: + * + *

    + *
  • {@code null}: A skyframe restart is required. + *
  • {@code Optional.empty()}: The target is not directly incompatible. Analysis can continue. + *
  • {@code !Optional.empty()}: The target is directly incompatible. Analysis should not + * continue. + *
+ */ + @Nullable + public static Optional createDirectlyIncompatibleTarget( + TargetAndConfiguration targetAndConfiguration, + ConfigConditions configConditions, + Environment env, + @Nullable PlatformInfo platformInfo, + NestedSetBuilder transitivePackagesForPackageRootResolution) + throws InterruptedException { + Target target = targetAndConfiguration.getTarget(); + Rule rule = target.getAssociatedRule(); + + if (rule == null || rule.getRuleClass().equals("toolchain") || platformInfo == null) { + return Optional.empty(); + } + + // Retrieve the label list for the target_compatible_with attribute. + BuildConfigurationValue configuration = targetAndConfiguration.getConfiguration(); + ConfiguredAttributeMapper attrs = + ConfiguredAttributeMapper.of(rule, configConditions.asProviders(), configuration); + if (!attrs.has("target_compatible_with", BuildType.LABEL_LIST)) { + return Optional.empty(); + } + + // Resolve the constraint labels. + List