LLVMProfDataAction doesn't respect environment variables set by cc_toolchain_config

[kmARC]

When LLVMProfDataAction runs llvm-profdata, the environment configuration is only forwarded from --action_env, but not from the toolchain configuration. ctx.actions.run() doesn't use the env keyword argument at all:

rules_cc/cc/private/rules_impl/fdo/fdo_context.bzl

Lines 306 to 315 in cdf9913

	ctx.actions.run(
	mnemonic = "LLVMProfDataAction",
	executable = llvm_profdata,
	tools = [all_files],
	arguments = [ctx.actions.args().add("merge").add("-o").add(profile_artifact).add(raw_profile_artifact)],
	inputs = [raw_profile_artifact],
	outputs = [profile_artifact],
	use_default_shell_env = True,
	progress_message = "LLVMProfDataAction: Generating %{output}",
	)

This causes the action fail in our case, where llvm-profdata is part of the fully hermetic C++ toolchain and therefore requires the hermetic location of its libraries to be added to LD_LIBRARY_PATH.

A naive implementation (which works in our case) is shown on the WIP PR #639.

[kmARC]

(The same implementation is baked in to pre-9.0.0 versions and IIUC there I can't even override this behavior with simply patching rules_cc: code snippet in the bazel repository)

[keith]

another potential fix: #666

[keith]

if you could test with mine that would be great

[kmARC]

This doesn't work for our toolchain out-of-the-box, even adding the new action name.

First, I think you need to add this action config to the toolchain autodetection, too. See my patch on top of your branch here: 4a15e1b

After this, I would expect that this condition evaluates to true, which never does:

 if cc_common.action_is_enabled(
        feature_configuration = feature_configuration,
        action_name = ACTION_NAMES.llvm_profdata,
    ):
        llvm_profdata = cc_common.get_tool_for_action(
            feature_configuration = feature_configuration,
            action_name = ACTION_NAMES.llvm_profdata,
        )
        llvm_profdata_env = cc_common.get_environment_variables(
            feature_configuration = feature_configuration,
            action_name = ACTION_NAMES.llvm_profdata,
            variables = cc_common.empty_variables(),
        )

Simply removing that condition and forcefully set the llvm_profdata env works for the profile-fedback build (but of course fails for anything else). Not sure what I am missing.

[keith]

I updated my branch with your change but also enabled = True, does it work for you now?

[kmARC]

Yes, this works perfectly! Thanks!

Can you explain why the enabled = True needed? I see the llvm-cov action above which doesn't have that, and I guess therefore when it's needed it is enabled somewhere else; I would expect that the feature eventually makes sure that this action config is enabled...

[keith]

it all depends on how the action is used. bazel automatically enables the features like c++-compile etc. it looks to me like the llvm-cov action isn't actually ever read from bazelbuild/bazel@21b5eb6 so comparing that one might not be useful.

in this case we only need it because of calling cc_common.action_is_enabled. that feels a bit wrong since that means other users with custom toolchains also have to know to add enabled=True, we'll see if we can improve that in PR review.

[kmARC]

Thanks for the explanation (This made me realize that indeed our clang toolchain generates an empty coverage.dat file :-/ )