Skip to content

Prepare to make bazel-toolchains compatible with Incompatible Target Skipping #12336

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.

Sign up for GitHub

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

Jump to bottom
Closed
philsc wants to merge 1 commit into from
Closed

Prepare to make bazel-toolchains compatible with Incompatible Target Skipping #12336

philsc wants to merge 1 commit into from

Conversation

@philsc
Copy link
Contributor

@philsc philsc commented Oct 23, 2020 •
edited
Loading

The upcoming Incompatible Target Skipping patch (#10945) introduces a
new target_compatible_with attribute which clashes with one of the
custom attributes in the bazel-toolchains repo. This patch adds a .patch
file in third_party so that we can fix the clash in a followup patch.

Once the clash is fixed we can merge the Incompatible Target
Skipping patch.

This is essentially a backport of bazelbuild/bazel-toolchains#913.

@google-cla google-cla bot added the cla: yes label Oct 23, 2020
@philsc
Copy link
Contributor Author

philsc commented Oct 23, 2020

@gregestren , this is the separate third-party PR.

@philsc
Copy link
Contributor Author

philsc commented Oct 23, 2020

@katre , this is the separate third-party PR.

@philsc philsc mentioned this pull request Oct 23, 2020
Closed
@AustinSchuh
Copy link
Contributor

AustinSchuh commented Oct 23, 2020

The failure looks like something is flaky. It downloaded https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu14.28.21-ca-jdk14.0.1-win_x64.zip and got the wrong checksum. When I download that by hand, I get the correct checksum.

katre
katre suggested changes Oct 23, 2020
View reviewed changes
Copy link
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.

The WORKSPACE change needs to be in the main PR, and only the file under third_party/bazel_toolchains should be here. This is a limitation of our internal/external process and is fairly confusing, so I'm sorry I have to ask you to change this yet again.

This actually makes it much safer, although to simplify the main PR you could make three:

  1. Add the patch under third_party (that I merge manually)
  2. Use the patch in WORKSPACE (and we use the normal merge process)
  3. The rest of the incompatible target skipping changes (which I have internally and will review today).

@philsc
Copy link
Contributor Author

philsc commented Oct 23, 2020

Okay, sounds good. Let me get on that. Thanks @katre !

@philsc
Make bazel-toolchains compatible with Incompatible Target Skipping
36f1539 
The upcoming Incompatible Target Skipping patch (bazelbuild#10945) introduces a
new `target_compatible_with` attribute which clashes with one of the
custom attributes in the bazel-toolchains repo. This patch fixes the
clash so we can merge the Incompatible Target Skipping path.

This is essentially a backport of bazelbuild/bazel-toolchains#913.
@philsc philsc force-pushed the bazel-toolchains-incompatible-target-skipping-fix branch from f81f6ac to 36f1539 Compare October 23, 2020 12:26
@philsc philsc changed the title Make bazel-toolchains compatible with Incompatible Target Skipping Prepare to make bazel-toolchains compatible with Incompatible Target Skipping Oct 23, 2020
katre
katre approved these changes Oct 23, 2020
View reviewed changes
Copy link
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.

I'll merge this shortly.

@philsc
Copy link
Contributor Author

philsc commented Oct 23, 2020

I'll merge this shortly.

Thank you. I'll get a WORKSPACE-only PR ready.

@katre
Copy link
Collaborator

katre commented Oct 23, 2020

Merged as 4381dbf.

@katre katre closed this Oct 23, 2020
philsc added a commit to philsc/bazel that referenced this pull request Oct 23, 2020
@philsc
Make bazel-toolchains compatible with Incompatible Target Skipping
db13131 
This is a follow-up to bazelbuild#12336. It makes use of the .patch file to make
the 3.1 version of bazel-toolchains compatible with Incompatible
Target Skipping.
@philsc philsc mentioned this pull request Oct 23, 2020
Closed
bazel-io pushed a commit that referenced this pull request Oct 23, 2020
@philsc @copybara-github
Make bazel-toolchains compatible with Incompatible Target Skipping (p…
8946cc1 
…art 2)

This is a follow-up to #12336. It makes use of the .patch file to make
the 3.1 version of bazel-toolchains compatible with Incompatible
Target Skipping.

Closes #12341.

PiperOrigin-RevId: 338674186
@philsc philsc deleted the bazel-toolchains-incompatible-target-skipping-fix branch February 13, 2021 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@katre katre katre approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@philsc @AustinSchuh @katre