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

Remove ctx.resolve_tools in favor of ctx.actions.run(executable = …) and ctx.actions.run(tools = ...). #2438

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

tjgq
Copy link
Contributor

@tjgq tjgq commented Mar 26, 2024

By passing the files_to_run for a tool into the executable or tools argument to ctx.actions.run or ctx.actions.run_shell, it's no longer necessary to pass the return values of ctx.resolve_tools into the inputs and input_manifests arguments.

Each struct field of the AppleMacToolsToolchainInfo and AppleXPlatToolsToolchainInfo providers previously populated with the return values of ctx.resolve_tools is now populated with the files_to_run for the respective tool.

The resolved_ prefix is removed from field and variable names; in addition to it no longer making sense, this change provides proof that all usages have been audited.

This lets us retire the ctx.resolve_tools API in a future Bazel version.

@tjgq tjgq force-pushed the resolve-tools branch 2 times, most recently from ed3ee93 to 02390d6 Compare March 26, 2024 20:43
@tjgq tjgq marked this pull request as ready for review March 26, 2024 21:34
Copy link
Collaborator

@brentleyjones brentleyjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for this @tjgq!

apple/internal/codesigning_support.bzl Outdated Show resolved Hide resolved
apple/internal/partials/support/resources_support.bzl Outdated Show resolved Hide resolved
brentleyjones referenced this pull request Mar 27, 2024
PiperOrigin-RevId: 610380908
…...)` and `ctx.actions.run(tools = ...)`.

By passing the files_to_run for a tool into the `executable` or `tools` argument to `ctx.actions.run` or `ctx.actions.run_shell`, it's no longer necessary to pass the return values of `ctx.resolve_tools` into the `inputs` and `input_manifests` arguments.

Each struct field of the AppleMacToolsToolchainInfo and AppleXPlatToolsToolchainInfo providers previously populated with the return values of `ctx.resolve_tools` is now populated with the files_to_run for the respective tool.

The `resolved_` prefix is removed from field and variable names; in addition to it no longer making sense, this change provides proof that all usages have been audited.

This lets us retire the `ctx.resolve_tools` API in a future Bazel version.
@mattrobmattrob mattrobmattrob merged commit 8cbbef1 into bazelbuild:master Mar 28, 2024
8 checks passed
luispadron added a commit to bazel-ios/rules_ios that referenced this pull request Apr 5, 2024
rules_apple 3.5.0 broke `resolved_*` tools from `AppleMacToolchainInfo` (bazelbuild/rules_apple#2438)
This sets the new minimum supported version for the next rules_ios release to 3.5.0.
luispadron added a commit to bazel-ios/rules_ios that referenced this pull request Apr 5, 2024
rules_apple 3.5.0 broke `resolved_*` tools from `AppleMacToolchainInfo` (bazelbuild/rules_apple#2438)
This sets the new minimum supported version for the next rules_ios release to 3.5.0.
@luispadron
Copy link
Contributor

luispadron commented Apr 5, 2024

We should list this as a breaking change in the releases i think, it broke some downstream rules since the AppleMacToolchainInfo provider fields have changed (and some of the resource action arguments)

luispadron added a commit to bazel-ios/rules_ios that referenced this pull request Apr 5, 2024
rules_apple 3.5.0 broke `resolved_*` tools from `AppleMacToolchainInfo` (bazelbuild/rules_apple#2438)
This sets the new minimum supported version for the next rules_ios release to 3.5.0.
luispadron added a commit to bazel-ios/rules_ios that referenced this pull request Apr 5, 2024
rules_apple 3.5.0 broke `resolved_*` tools from `AppleMacToolchainInfo` (bazelbuild/rules_apple#2438)
This sets the new minimum supported version for the next rules_ios release to 3.5.0.
luispadron added a commit to bazel-ios/rules_ios that referenced this pull request Apr 5, 2024
rules_apple 3.5.0 broke `resolved_*` tools from `AppleMacToolchainInfo` (bazelbuild/rules_apple#2438)
This sets the new minimum supported version for the next rules_ios release to 3.5.0.
luispadron added a commit to bazel-ios/rules_ios that referenced this pull request Apr 5, 2024
rules_apple 3.5.0 broke `resolved_*` tools from `AppleMacToolchainInfo` (bazelbuild/rules_apple#2438)
This sets the new minimum supported version for the next rules_ios release to 3.5.0.
@brentleyjones
Copy link
Collaborator

That's not public API, right? I think we are free to modify that without any mention.

@luispadron
Copy link
Contributor

Yeah you're right. Thought it was in providers.bzl but doesn't look like it

acecilia pushed a commit to revolut-mobile/rules_apple that referenced this pull request Apr 12, 2024
……)` and `ctx.actions.run(tools = ...)`. (bazelbuild#2438)

By passing the files_to_run for a tool into the `executable` or `tools`
argument to `ctx.actions.run` or `ctx.actions.run_shell`, it's no longer
necessary to pass the return values of `ctx.resolve_tools` into the
`inputs` and `input_manifests` arguments.

Each struct field of the AppleMacToolsToolchainInfo and
AppleXPlatToolsToolchainInfo providers previously populated with the
return values of `ctx.resolve_tools` is now populated with the
files_to_run for the respective tool.

The `resolved_` prefix is removed from field and variable names; in
addition to it no longer making sense, this change provides proof that
all usages have been audited.

This lets us retire the `ctx.resolve_tools` API in a future Bazel
version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants