Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix builds for filegroup targets with incompatible dependencies #12601

Closed
wants to merge 1 commit into from

Conversation

philsc
Copy link
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
Contributor Author

philsc commented Dec 2, 2020

@katre, for your consideration.

Copy link
Member

@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