Skip to content

Fix builds for filegroup targets with incompatible dependencies#12601

Closed
philsc wants to merge 1 commit intobazelbuild:masterfrom
philsc:fix12582
Closed

Fix builds for filegroup targets with incompatible dependencies#12601
philsc wants to merge 1 commit intobazelbuild:masterfrom
philsc:fix12582

Conversation

@philsc
Copy link
Copy Markdown
Contributor

@philsc philsc commented Dec 2, 2020

When building a filegroup with incompatible dependencies, the logic
was such that the incompatible dependencies would actually be built by
bazel. These dependencies only expose FailActions so the build
failed.

This is caused by bazel skipping the incompatibility check for any
rules that don't participate in toolchain resolution.

For example, without the fix in RuleContextConstraintSemantics.java,
the added unit test fails with:

ERROR: /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10: Reporting failed target //target_skipping:binary located at /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10 failed: Can't build this. This target is incompatible. Please file a bug upstream. caused by //target_skipping:binary
Target //target_skipping:filegroup failed to build

This patch makes it so targets that don't use toolchain resolution
skip the check for the target_compatible_with attribute, but still
check for incompatible dependencies.

When building a `filegroup` with incompatible dependencies, the logic
was such that the incompatible dependencies would actually be built by
bazel. These dependencies only expose `FailAction`s so the build
failed.

This is caused by bazel skipping the incompatibility check for any
rules that don't participate in toolchain resolution.

For example, without the fix in `RuleContextConstraintSemantics.java`,
the added unit test fails with:

    ERROR: /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10: Reporting failed target //target_skipping:binary located at /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10 failed: Can't build this. This target is incompatible. Please file a bug upstream. caused by //target_skipping:binary
    Target //target_skipping:filegroup failed to build

This patch makes it so targets that don't use toolchain resolution
skip the check for the `target_compatible_with` attribute, but still
check for incompatible dependencies.
@google-cla google-cla Bot added the cla: yes label Dec 2, 2020
@philsc
Copy link
Copy Markdown
Contributor Author

philsc commented Dec 2, 2020

@katre, for your consideration.

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.

Thanks, I'll merge this today.

@bazel-io bazel-io closed this in 84fadcf Dec 2, 2020
@philsc philsc deleted the fix12582 branch December 2, 2020 19:23
frc971-automation pushed a commit to frc971/971-Robot-Code that referenced this pull request Dec 5, 2020
This is the same version as 4.0.0rc2 except with 2 cherry-picks:
a3c94ec2ed Fix builds for filegroup targets with incompatible dependencies
b3c9d7381c Fix a couple of bugs with Incompatible Target Skipping

The patch for the filegroup fix has already merged on master:
bazelbuild/bazel#12601

The other patch is still under review:
bazelbuild/bazel#12560

This should enable us to convert the code base to use platforms.

Change-Id: I35f078c2576f5268d5e3fb33e9bd86732529f744
philwo pushed a commit that referenced this pull request Dec 9, 2020
When building a `filegroup` with incompatible dependencies, the logic
was such that the incompatible dependencies would actually be built by
bazel. These dependencies only expose `FailAction`s so the build
failed.

This is caused by bazel skipping the incompatibility check for any
rules that don't participate in toolchain resolution.

For example, without the fix in `RuleContextConstraintSemantics.java`,
the added unit test fails with:

    ERROR: /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10: Reporting failed target //target_skipping:binary located at /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10 failed: Can't build this. This target is incompatible. Please file a bug upstream. caused by //target_skipping:binary
    Target //target_skipping:filegroup failed to build

This patch makes it so targets that don't use toolchain resolution
skip the check for the `target_compatible_with` attribute, but still
check for incompatible dependencies.

Closes #12601.

PiperOrigin-RevId: 345263391
philwo pushed a commit that referenced this pull request Dec 10, 2020
When building a `filegroup` with incompatible dependencies, the logic
was such that the incompatible dependencies would actually be built by
bazel. These dependencies only expose `FailAction`s so the build
failed.

This is caused by bazel skipping the incompatibility check for any
rules that don't participate in toolchain resolution.

For example, without the fix in `RuleContextConstraintSemantics.java`,
the added unit test fails with:

    ERROR: /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10: Reporting failed target //target_skipping:binary located at /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10 failed: Can't build this. This target is incompatible. Please file a bug upstream. caused by //target_skipping:binary
    Target //target_skipping:filegroup failed to build

This patch makes it so targets that don't use toolchain resolution
skip the check for the `target_compatible_with` attribute, but still
check for incompatible dependencies.

Closes #12601.

PiperOrigin-RevId: 345263391
ulfjack pushed a commit to EngFlow/bazel that referenced this pull request Mar 5, 2021
When building a `filegroup` with incompatible dependencies, the logic
was such that the incompatible dependencies would actually be built by
bazel. These dependencies only expose `FailAction`s so the build
failed.

This is caused by bazel skipping the incompatibility check for any
rules that don't participate in toolchain resolution.

For example, without the fix in `RuleContextConstraintSemantics.java`,
the added unit test fails with:

    ERROR: /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10: Reporting failed target //target_skipping:binary located at /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10 failed: Can't build this. This target is incompatible. Please file a bug upstream. caused by //target_skipping:binary
    Target //target_skipping:filegroup failed to build

This patch makes it so targets that don't use toolchain resolution
skip the check for the `target_compatible_with` attribute, but still
check for incompatible dependencies.

Closes bazelbuild#12601.

PiperOrigin-RevId: 345263391
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.

2 participants