Skip to content

Add llvm-profdata action, support env#666

Open
keith wants to merge 2 commits into
bazelbuild:mainfrom
keith:ks/add-llvm-profdata-action-support-env
Open

Add llvm-profdata action, support env#666
keith wants to merge 2 commits into
bazelbuild:mainfrom
keith:ks/add-llvm-profdata-action-support-env

Conversation

@keith

@keith keith commented Mar 25, 2026

Copy link
Copy Markdown
Member

Having a real action name for llvm-profdata is helpful for fdo and for
coverage.

This change makes sure if a toolchain provides env for llvm-profdata, it
is correctly used in fdo actions.

Fixes #640

@keith

keith commented Mar 25, 2026

Copy link
Copy Markdown
Member Author

i am also adding this new action name in #659, so i'll rebase whichever merges

@keith keith force-pushed the ks/add-llvm-profdata-action-support-env branch from 5b60312 to 2ff7937 Compare April 3, 2026 17:07
Comment on lines +77 to +80
if cc_common.action_is_enabled(
feature_configuration = feature_configuration,
action_name = ACTION_NAMES.llvm_profdata,
):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

is there a different API I can use for this without having to force it to be enabled = True? without this calling these other functions fails ofc

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

updated to automatically request that feature automatically instead

@keith keith force-pushed the ks/add-llvm-profdata-action-support-env branch from 2ff7937 to a9b3259 Compare April 4, 2026 15:45
@hvadehra hvadehra removed their request for review April 21, 2026 04:54
@keith keith force-pushed the ks/add-llvm-profdata-action-support-env branch 2 times, most recently from ad5393d to 59d2d07 Compare April 30, 2026 22:19
trybka
trybka previously approved these changes May 15, 2026

@trybka trybka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please rebase and resolve merge conflicts

@keith keith force-pushed the ks/add-llvm-profdata-action-support-env branch from 59d2d07 to 7c201d5 Compare May 15, 2026 15:07
@keith

keith commented May 15, 2026

Copy link
Copy Markdown
Member Author

done!

@trybka trybka self-requested a review May 15, 2026 15:35
trybka
trybka previously approved these changes May 15, 2026
@cerisier

cerisier commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Is there anything missing for this to get merged ?

lilygorsheneva
lilygorsheneva previously approved these changes Jun 8, 2026
Comment thread tests/cc/common/cc_fdo_env_test.bzl Outdated
@@ -0,0 +1,119 @@
"""Tests for FDO action environment variables."""

load("@bazel_features//private:util.bzl", _bazel_version_ge = "ge")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we avoid the private parts of bazel_features?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sorry fixed

keith added 2 commits June 8, 2026 11:24
Having a real action name for llvm-profdata is helpful for fdo and for
coverage.

This change makes sure if a toolchain provides env for llvm-profdata, it
is correctly used in fdo actions.

Fixes bazelbuild#640
@keith keith force-pushed the ks/add-llvm-profdata-action-support-env branch from 46912da to 2ce1d9a Compare June 8, 2026 18:25
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.

LLVMProfDataAction doesn't respect environment variables set by cc_toolchain_config

4 participants