WIP resolve_tools #849
WIP resolve_tools #849
Conversation
| @@ -763,7 +766,8 @@ InputFiles = provider( | |||
| "into $EXT_BUILD_DEPS/include." | |||
| ), | |||
| libs = "Library files built by Bazel. Will be copied into $EXT_BUILD_DEPS/lib.", | |||
| tools_files = ( | |||
| tools_files = "All files (including runfiles needed for the tools", | |||
There was a problem hiding this comment.
I think that we probably also want a tools_manifests entry here and we should merge this with any additional manifests when we call ctx.actions.run_shell. I've not fully groked what the manifests input does to an action as its not well documented.
| @@ -458,16 +458,19 @@ def cc_external_rule_impl(ctx, attrs): | |||
| # TODO: `additional_tools` is deprecated, remove. | |||
| legacy_tools = ctx.files.additional_tools + ctx.files.tools_deps | |||
|
|
|||
| inputs_from_tools, manifests_from_tools = ctx.resolve_tools(tools = attrs.tools_deps) | |||
There was a problem hiding this comment.
Possibly should be legacy_tools here?
There was a problem hiding this comment.
Also need to be aware of what this looks like once deprecated attributes are removed. build_data is not included in legacy_tools at the moment.
| lines.append("##mkdirs## $$EXT_BUILD_DEPS$$/bin") | ||
| for tool in files.tools_files: | ||
| for tool in files.tools_roots: | ||
| lines.append("##symlink_to_dir## $$EXT_BUILD_ROOT$$/{} $$EXT_BUILD_DEPS$$/bin/".format(tool)) |
There was a problem hiding this comment.
Does this symlinking break finding runfiles for the tool? It doesn't break it for MAKE but we don't find make via the path so aren't using this mechanism in that case.
|
This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days. Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_foreign_cc! |
|
This PR was automatically closed because it went 30 days without a reply since it was labeled "Can Close?" |
Initial attempt at reviving some old code that I put together that calls
ctx.resolve_toolson the tools to set the manifest and runfiles for the tools correctly.This patch is sufficient to resolve the errors seen in using #848 but isn't yet in a state that is ready to merge.