Consider action_config tools for cc_toolchain make variables
This change makes cc_helper.get_toolchain_global_make_variables consider action_configs for tool paths before looking at tool_paths. This allows toolchains that can't or don't specify tool_paths to have make variables such as $CC and $LD specified correctly.
The mapping from make variable to action name is:
$AR -> ACTION_NAMES.cpp_link_static_library
$CC -> ACTION_NAMES.c_compile
$LD -> ACTION_NAMES.cpp_link_executable
$OBJCOPY -> ACTION_NAMES.objcopy_embed_data
$STRIP -> ACTION_NAMES.strip
All other toolchain make variables are unchanged either because they don't map to tools, or the mapping wasn't obvious to me. I would be happy to change the mappings if there are better ones.
My understanding is that cc_helper is not exposed publicly, so this is not a public API breaking change. It is possibly a change of output for any existing cc_toolchains, but I believe the toolchains in rules_cc won't change as they either don't specify action_configs for these actions, or they don't specify tool_paths at all and these variables just don't work.
Fixes #25887
Just a little bump on this one. What are the next steps?
@armandomontanez for the interaction with rules_cc - this would help to fully migrate to action configs, right?
@comius @trybka @pzembrod just another bump on this PR. Is there anything I can do to help move this along?
@pzembrod @comius just another bump for review of this PR. I have been using a locally built Bazel with this change for quite a while now with no issue, and I would love to not have to continue using a fork :)
Just another friendly bump to try and get this PR moving again.
@pzembrod Friendly ping
Just another friendly bump to try and get this PR moving again. Is there something blocking this from being reviewed and/or merged?
Just another friendly bump to try and get this PR moving again. Is there something blocking this from being reviewed and/or merged?
There's nothing in particular blocking this; working on my backlog now. :-)
I have added a commit that moves the tool names in to constants, and inlines calls to _tool_path() since it is a single line function that doesn't really add more clarity.
I'm not sure why theres so many failures in CI. It looks like there are C++ errors, and it can't find python? Perhaps there is a temporary issue with CI unrelated to this change?
I tried the failing tests locally, and they do not fail for me. I think CI just had a blip, but I can't rerun the jobs myself to find out.
The failure should go away if you rebase onto master.
Just a friendly bump to try and get this PR going again.