Skip to content

Commit

Permalink
Add basic incompatible target skipping support
Browse files Browse the repository at this point in the history
This patch aims to implement a basic version of incompatible target
skipping outlined here:
https://docs.google.com/document/d/12n5QNHmFSkuh5yAbdEex64ot4hRgR-moL1zRimU7wHQ/edit?usp=sharing

The implementation in this patch supports target skipping based
on the target platform. In a `BUILD` file you can now add constraints
that the target platform must satisfy in order for the target to be
built and/or tested. For example, use the following snippet to declare
a target to be compatible with Windows platforms only:

  cc_binary(
      name = "bin",
      srcs = ["bin.cc"],
      target_compatible_with = [
          "@platforms//os:windows",
      ],
  )

Builds triggered with `:all` or `...` on a non-Windows platform will
simply skip the incompatible target. An appropriate note is shown on
the command line if the `--show_result` threshold is high enough.
Targets that transitively depend on incompatible targets are
themselves considered incompatible and will also be skipped.

Explicitly requesting an incompatible target on the command line is an
error and will cause the build to fail. Bazel will print out an
appropriate error message and inform the user what constraint could
not be satisfied.

See the new documentation in this patch for more information. In
particular, https://docs.bazel.build/versions/master/platforms.html
should be a good bit more informative.

This implementation does not make any effort to support expressing
compatibility with toolchains. It is possible that using `select()`
(commented on below) already makes this possible, but it's not
validated or explicitly supported in this patch.

During implementation we noticed that `select()` can be quite powerful
in combination with `target_compatible_with`. A basic summary of this
is also documented on the Platforms page.

It may be useful to create helper functions in, say, skylib to help
make complex `select()` statements more readable. For example, we
could replace the following:

  target_compatible_with = select({
      "@platforms//os:linux": [],
      "@platforms//os:macos": [],
      "//conditions:default": [":not_compatible"],
  })

with something like:

  target_compatible_with = constraints.any_of([
      "@platforms//os:linux",
      "@platforms//os:macos",
  ])

That, however, is work for follow-up patches.

Many thanks to Austin Schuh (@AustinSchuh) and Greg Estren
(@gregestren) for working on the proposal and helping a ton on this
patch itself. Also thanks to many others who provided feedback on the
implementation.

RELNOTES: Bazel skips incompatible targets based on target platform
and `target_compatible_with` contents. See
https://docs.bazel.build/versions/master/platforms.html for more
details.
  • Loading branch information
philsc authored and AustinSchuh committed Oct 23, 2020
1 parent c58f966 commit 397f3bc
Show file tree
Hide file tree
Showing 31 changed files with 1,653 additions and 35 deletions.
2 changes: 2 additions & 0 deletions site/docs/platforms-intro.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ or opt in early depends on your specific value / cost needs:
flags on the command line.
* Simpler language design. All languages share a common API for defining
toolchains, using toolchains, and selecting the right toolchain for a platform.
* Targets can be [skipped](platforms.html#skipping-incompatible-targets) in the
build and test phase if they are incompatible with the target platform.

### Costs
* Dependent projects that don't yet support platforms might not automatically work
Expand Down
114 changes: 114 additions & 0 deletions site/docs/platforms.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,117 @@ command-line flags:

* `--host_platform` - defaults to `@bazel_tools//platforms:host_platform`
* `--platforms` - defaults to `@bazel_tools//platforms:target_platform`

## Skipping incompatible targets

When building for a specific target platform it is often desirable to skip
targets that will never work on that platform. For example, your Windows device
driver is likely going to generate lots of compiler errors when building on a
Linux machine with `//...`. Use the
[`target_compatible_with`](be/common-definitions.html#common.target_compatible_with)
attribute to tell Bazel what target platform constraints your code has.

The simplest use of this attribute restricts a target to a single platform.
The target will not be built for any platform that doesn't satisfy all of the
constraints. The following example restricts `win_driver_lib.cc` to 64-bit
Windows.

```python
cc_library(
name = "win_driver_lib",
srcs = "win_driver_lib.cc",
target_compatible_with = [
"@platforms//cpu:x86_64",
"@platforms//os:windows",
],
)
```

When building for anything but 64-bit Windows we say that `:win_driver_lib` is
incompatible. Incompatibility is transitive. Any targets that transitively
depend on an incompatible target are themselves considered incompatible.

### When are targets skipped?

Targets are skipped when they are considered incompatible and included in the
build as part of a target pattern expansion. For example, the following two
invocations skip any incompatible targets found in a target pattern expansion.

```console
$ bazel build --platforms=//:myplatform //...`
```

```console
$ bazel build --platforms=//:myplatform //:all`
```

Explicitly specifying an incompatible target on the command line results in an
error message and a failed build.

```console
$ bazel build --platforms=//:myplatform //:target_incompatible_with_myplatform
...
ERROR: Target //:target_incompatible_with_myplatform is incompatible and cannot be built, but was explicitly requested.
...
FAILED: Build did NOT complete successfully
```

### More expressive constraints

For more flexibility in expressing constraints, create a user-defined
[`constraint_value`](platform.html#constraint_value) that no platform
satisfies. For example, Put the following somewhere in your project and change
`//:not_compatible` in the subsequent examples to match your location.

```python
constraint_setting(name = "not_compatible_setting")

constraint_value(
name = "not_compatible",
constraint_setting = ":not_compatible_setting",
)
```

Use [`select()`](functions.html#select) in combination with `:not_compatible`
to express more complicated restrictions. For example, use it to implement
basic OR logic. The following marks a library compatible with macOS and Linux,
but no other platforms. Note that an empty constraints list is equivalent to
"compatible with everything".

```python
cc_library(
name = "unixish_lib",
srcs = "unixish_lib.cc",
target_compatible_with = select({
"@platforms//os:osx": [],
"@platforms//os:linux": [],
"//conditions:default": ["//:not_compatible"],
],
)
```

The above can be interpreted as follows:

1. If we are targeting macOS, then this target has no constraints.
2. If we are targeting Linux, then this target has no constraints.
3. Otherwise the target has the `:not_compatible` constraint. Because
`:not_compatible` is not part of any platforms, the target is deemed
incompatible.

To make your constraints more readable, use
[skylib](https://github.com/bazelbuild/bazel-skylib)'s
[`selects.with_or()`](https://github.com/bazelbuild/bazel-skylib/blob/master/docs/selects_doc.md#selectswith_or).

You can express inverse compatibility in a similar way. The following example
describes a library that is compatible with everything _except_ for ARM.

```python
cc_library(
name = "non_arm_lib",
srcs = "non_arm_lib.cc",
target_compatible_with = select({
"@platforms//cpu:arm": ["//:not_compatible"],
"//conditions:default": [],
],
)
```
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class PredefinedAttributes {
"templates/attributes/common/licenses.html",
"templates/attributes/common/restricted_to.html",
"templates/attributes/common/tags.html",
"templates/attributes/common/target_compatible_with.html",
"templates/attributes/common/testonly.html",
"templates/attributes/common/toolchains.html",
"templates/attributes/common/visibility.html");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<p>
<code>List of <a href="../build-ref.html#labels">labels</a>; optional; default
is the empty list</code>
</p>

<p>
A list of
<code><a href="platform.html#constraint_value">constraint_value</a></code>s
that must be present in the target platform for this target to be considered
"compatible". This is in addition to any constraints already set by the rule
type. If the target platform does not satisfy all listed constraints then the
target is considered "incompatible". Incompatible targets are skipped for
building and testing when the target pattern is expanded
(e.g. `//...`, `:all`). When explicitly specified on the command line,
incompatible targets cause Bazel to print an error and cause a build or test
failure.
</p>

<p>
Targets that transitively depend on incompatible targets are themselves
considered incompatible. They are also skipped for building and testing.
</p>

<p>
An empty list (which is the default) signifies that the target is compatible
with all platforms.
<p>

<p>
See the <a href="../platforms.html#skipping-incompatible-targets">Platforms</a>
page for more information about incompatible target skipping.
</p>
30 changes: 30 additions & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ java_library(
":extra/extra_action_info_file_write_action",
":extra_action_artifacts_provider",
":file_provider",
":incompatible_platform_provider",
":inconsistent_aspect_order_exception",
":label_and_location",
":label_expander",
Expand Down Expand Up @@ -377,6 +378,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink",
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
"//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/stringtemplate",
"//src/main/java/com/google/devtools/build/lib/bugreport",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
Expand Down Expand Up @@ -600,6 +602,7 @@ java_library(
":config/build_options",
":config/invalid_configuration_exception",
":configured_target",
":constraints/platform_restrictions_result",
":constraints/top_level_constraint_semantics",
":extra_action_artifacts_provider",
":make_environment_event",
Expand Down Expand Up @@ -775,6 +778,18 @@ java_library(
],
)

java_library(
name = "incompatible_platform_provider",
srcs = ["IncompatiblePlatformProvider.java"],
deps = [
":configured_target",
":transitive_info_provider",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//third_party:guava",
],
)

java_library(
name = "inconsistent_aspect_order_exception",
srcs = ["InconsistentAspectOrderException.java"],
Expand Down Expand Up @@ -1981,6 +1996,17 @@ java_library(
],
)

java_library(
name = "constraints/platform_restrictions_result",
srcs = ["constraints/PlatformRestrictionsResult.java"],
deps = [
":configured_target",
"//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
],
)

java_library(
name = "constraints/top_level_constraint_semantics",
srcs = ["constraints/TopLevelConstraintSemantics.java"],
Expand All @@ -1990,15 +2016,19 @@ java_library(
":configured_target",
":constraints/constraint_semantics",
":constraints/environment_collection",
":constraints/platform_restrictions_result",
":constraints/supported_environments_provider",
":incompatible_platform_provider",
":transitive_info_collection",
":view_creation_failed_exception",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.analysis.constraints.ConstraintConstants;
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
Expand Down Expand Up @@ -334,6 +335,10 @@ public static RuleClass.Builder commonCoreAndStarlarkAttributes(RuleClass.Builde
.dontCheckConstraints()
.nonconfigurable(
"special logic for constraints and select: see ConstraintSemantics"))
.add(
attr("target_compatible_with", LABEL_LIST)
.mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
.allowedFileTypes(FileTypeSet.NO_FILE))
.add(
attr(RuleClass.CONFIG_SETTING_DEPS_ATTRIBUTE, LABEL_LIST)
.nonconfigurable("stores configurability keys"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigurationResolver.TopLevelTargetsAndConfigsResult;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.constraints.PlatformRestrictionsResult;
import com.google.devtools.build.lib.analysis.constraints.TopLevelConstraintSemantics;
import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory;
import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory.CoverageReportActionsWrapper;
Expand Down Expand Up @@ -212,6 +213,7 @@ public AnalysisResult update(
TargetPatternPhaseValue loadingResult,
BuildOptions targetOptions,
Set<String> multiCpu,
ImmutableSet<String> explicitTargetPatterns,
List<String> aspects,
AnalysisOptions viewOptions,
boolean keepGoing,
Expand Down Expand Up @@ -430,6 +432,24 @@ public AnalysisResult update(
skyframeBuildView.clearInvalidatedConfiguredTargets();
}

TopLevelConstraintSemantics topLevelConstraintSemantics =
new TopLevelConstraintSemantics(
skyframeExecutor.getPackageManager(),
input -> skyframeExecutor.getConfiguration(eventHandler, input),
eventHandler);

PlatformRestrictionsResult platformRestrictions =
topLevelConstraintSemantics.checkPlatformRestrictions(
skyframeAnalysisResult.getConfiguredTargets(), explicitTargetPatterns, keepGoing);

if (!platformRestrictions.targetsWithErrors().isEmpty()) {
// If there are any errored targets (e.g. incompatible targets that are explicitly specified
// on the command line), remove them from the list of targets to be built.
skyframeAnalysisResult =
skyframeAnalysisResult.withAdditionalErroredTargets(
ImmutableSet.copyOf(platformRestrictions.targetsWithErrors()));
}

int numTargetsToAnalyze = topLevelTargetsWithConfigs.size();
int numSuccessful = skyframeAnalysisResult.getConfiguredTargets().size();
if (0 < numSuccessful && numSuccessful < numTargetsToAnalyze) {
Expand All @@ -440,11 +460,11 @@ public AnalysisResult update(
}

Set<ConfiguredTarget> targetsToSkip =
new TopLevelConstraintSemantics(
skyframeExecutor.getPackageManager(),
input -> skyframeExecutor.getConfiguration(eventHandler, input),
eventHandler)
.checkTargetEnvironmentRestrictions(skyframeAnalysisResult.getConfiguredTargets());
Sets.union(
topLevelConstraintSemantics.checkTargetEnvironmentRestrictions(
skyframeAnalysisResult.getConfiguredTargets()),
platformRestrictions.targetsToSkip())
.immutableCopy();

AnalysisResult result =
createResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
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;
Expand Down Expand Up @@ -322,6 +323,12 @@ private ConfiguredTarget createRule(
prerequisiteMap.values()))
.build();

ConfiguredTarget incompatibleTarget =
RuleContextConstraintSemantics.incompatibleConfiguredTarget(ruleContext, prerequisiteMap);
if (incompatibleTarget != null) {
return incompatibleTarget;
}

List<NestedSet<AnalysisFailure>> analysisFailures = depAnalysisFailures(ruleContext);
if (!analysisFailures.isEmpty()) {
return erroredConfiguredTargetWithFailures(ruleContext, analysisFailures);
Expand Down
Loading

0 comments on commit 397f3bc

Please sign in to comment.