Skip to content

Add new exec_cfg and non_exec_cfg features#649

Open
keith wants to merge 1 commit intobazelbuild:mainfrom
keith:ks/add-new-exec_cfg-and-target_cfg-features
Open

Add new exec_cfg and non_exec_cfg features#649
keith wants to merge 1 commit intobazelbuild:mainfrom
keith:ks/add-new-exec_cfg-and-target_cfg-features

Conversation

@keith
Copy link
Copy Markdown
Member

@keith keith commented Mar 22, 2026

This allows adding custom copts when building for the exec config.
Previously this was done implicitly by bazel's exec transition, but
bazel wants to stop doing that because that is brittle and dependent on
crosstool configuration (for example it requires knowing what compiler
is being used on windows). This is added to the "legacy" features to
best effort maintain backwards compatibility

Fixes bazelbuild/bazel#13308
Fixes bazelbuild/bazel#24545

keith added a commit to keith/bazel that referenced this pull request Mar 22, 2026
Must be paired with a rules_cc version bump that includes bazelbuild/rules_cc#649

Fixes bazelbuild#13308
Fixes bazelbuild#24545
@keith
Copy link
Copy Markdown
Member Author

keith commented Mar 22, 2026

meant to be paired with bazelbuild/bazel#29048

@keith
Copy link
Copy Markdown
Member Author

keith commented Mar 22, 2026

cc @gregestren

@armandomontanez armandomontanez added P2 We'll consider working on this in future. (Assignee optional) category: toolchains type: internal cleanup Does not directly address a feature request or a bug report, but improves project hygiene labels Mar 23, 2026
@keith
Copy link
Copy Markdown
Member Author

keith commented Mar 23, 2026

I don't have a strong preference around naming here. offline i got the recommendation to use is_exec or is_exec_cfg. also we could have only that feature and toss the target_cfg one. People could still derive that with with(not_features=["exec_cfg"], but then it wouldn't be possible to have an unrepresentable state. any thoughts?

@trybka
Copy link
Copy Markdown
Collaborator

trybka commented Mar 23, 2026

Interested if @gregestren or @katre have any thoughts here.
ISTR they had principled reasons to not want to expose this, but it feels like that ship may have sailed some time ago.

FWIW cc_toolchain._is_tool_configuration has a big ol' warning on it: "INTERNAL API, DO NOT USE!"

Should we be using ctx.configuration.is_tool_configuration() instead?

Functionally, we already have host/nonhost (though that is defaulted to off). It seems like we're just replicating that, but with better names?

@gregestren @katre was the main objection to the old "host / nonhost" the naming / inaccuracy?

@katre
Copy link
Copy Markdown
Contributor

katre commented Mar 23, 2026

My recollection is that the original goal was to reduce the difference between the target and exec configurations to nothing and enable cache sharing between them.

This is clearly not happening and not something we are working towards (and having tools build with optimizations so they finish faster is a worthwhile goal.

This change is narrowly targeted and makes a lot of sense to me.

if cc_toolchain._is_tool_configuration:
all_features.append("exec_cfg")
else:
all_features.append("target_cfg")
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.

Repeating my concerns from a private conversation: target is meaningful only as a cfg on a particular edge where it refers to the current configuration's target platform, not so much on a global level (it's possible for the top-level target platform to show up as an exec platform). That's why I would prefer a name that negates exec but doesn't mention target, e.g. non_exec_cfg.

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.

+1 to the naming, and it matches the existing host/nonhost which will make it very clear when migrating.

I think it's reasonable to have both exec/nonexec (sometimes it can be difficult to match not-features in certain states so having that explicitly vs having to check "not_features = [exec]" is reasonable).

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 non_exec_cfg but if anyone can think of something more clear that would be appreciated. I don't think users have the level of nuance we're talking about here, and IMO non_exec_cfg leaves more room for ambiguity. It reads to me like we're implying there's yet another potential configuration, when there really are only these 2.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about tool_cfg and non_tool_cfg? It's a little clearer about the context. (And I wouldn't mind a wholesale "exec"->"tool" changeover but that's a big lift.)

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.

i like how that reads more but i do wonder about splitting the terminology more. seems to me like nearly all places today use exec and off the top of my head i can only think of --tool_deps using tool

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Valid point, I'd say leave as exec_cfg and non_exec_cfg, and we'll deal with an overhaul of the entire "exec" name later.

This allows adding custom copts when building for the exec config.
Previously this was done implicitly by bazel's exec transition, but
bazel wants to stop doing that because that is brittle and dependent on
crosstool configuration (for example it requires knowing what compiler
is being used on windows). This is added to the "legacy" features to
best effort maintain backwards compatibility

Fixes bazelbuild/bazel#13308
Fixes bazelbuild/bazel#24545
@keith keith force-pushed the ks/add-new-exec_cfg-and-target_cfg-features branch from 9643eaf to effd218 Compare March 23, 2026 23:04
@keith
Copy link
Copy Markdown
Member Author

keith commented Mar 23, 2026

Updated to ctx.configuration.is_tool_configuration(), note that API is also currently restricted to the internal starlark allowlist.

@keith keith changed the title Add new exec_cfg and target_cfg features Add new exec_cfg and non_exec_cfg features Mar 23, 2026
@katre
Copy link
Copy Markdown
Contributor

katre commented Mar 24, 2026

Updated to ctx.configuration.is_tool_configuration(), note that API is also currently restricted to the internal starlark allowlist.

We should open that up. I don't think the original goals of keeping it private make sense any more. @gregestren, what do you think?

@keith
Copy link
Copy Markdown
Member Author

keith commented Mar 24, 2026

i have this pr for that bazelbuild/bazel#29056, but I do wonder again about the exec vs tool naming being discussed above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: toolchains P2 We'll consider working on this in future. (Assignee optional) type: internal cleanup Does not directly address a feature request or a bug report, but improves project hygiene

Projects

None yet

5 participants