-
Notifications
You must be signed in to change notification settings - Fork 259
Integrate bazel resource_sets into the build actions #1457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This bumps bazel_lib to 3.1.0, and then syncs the prod dependencies between bzlmod and workspaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR integrates Bazel resource_sets into build actions to enable safer parallelization. By specifying resource consumption, Bazel can properly schedule builds instead of over-scheduling when build systems use parallel compilation (e.g., CMAKE_BUILD_PARALLEL_LEVEL).
Changes:
- Adds new resource_sets module that defines size classes (tiny/small/medium/large/enormous) with CPU and memory allocations
- Updates framework.bzl and built_tools_framework.bzl to set resource_set and environment variables for parallel builds
- Adds bazel_lib dependency (with temporary archive_override) for resource_set_for function
- Updates dependency versions for platforms, bazel_features, bazel_skylib, rules_cc, rules_shell, and aspect_rules_lint
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| foreign_cc/private/resource_sets.bzl | New module defining size classes, settings creation, and resource set computation logic |
| foreign_cc/settings/BUILD.bazel | New file that creates build settings for resource size configuration |
| foreign_cc/private/framework.bzl | Integrates SIZE_ATTRIBUTES and sets resource_set/env vars for cc_external_rule actions |
| foreign_cc/built_tools/private/built_tools_framework.bzl | Integrates SIZE_ATTRIBUTES and sets resource_set/env vars for built tool actions |
| toolchains/private/BUILD.bazel | Example usage: sets cmake_tool to "medium" resource size |
| foreign_cc/repositories.bzl | Adds bazel_lib dependency and updates versions for platforms, bazel_features, bazel_skylib, rules_cc, rules_shell |
| MODULE.bazel | Updates dependency versions and adds temporary archive_override for bazel_lib pending upstream PR |
| WORKSPACE.bazel | Updates aspect_rules_lint and switches from aspect_bazel_lib to bazel_lib |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
796305a to
24c447e
Compare
24c447e to
b04bc68
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Set the approximate size of this build. This does two things:" + | ||
| "1. Sets the environment variables to tell the underlying build system " + | ||
| " the requested parallelization; examples are CMAKE_BUILD_PARALLEL_LEVEL " + | ||
| " for cmake or MAKEFLAGS for autotools. " + | ||
| "2. Sets the resource_set attribute on the action to tell bazel how many cores " + | ||
| " are being used, so it schedules appropriately." + | ||
| "The sizes map to labels, which can be used to override the meaning of the sizes. See " + | ||
| "@rules_foreign_cc//foreign_cc/settings:size_{size}_{cpu|mem}" |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation string could be improved for clarity. Consider adding line breaks or formatting to make the two main points more distinct. The reference to settings labels at the end could also be more specific about how to use them.
| "Set the approximate size of this build. This does two things:" + | |
| "1. Sets the environment variables to tell the underlying build system " + | |
| " the requested parallelization; examples are CMAKE_BUILD_PARALLEL_LEVEL " + | |
| " for cmake or MAKEFLAGS for autotools. " + | |
| "2. Sets the resource_set attribute on the action to tell bazel how many cores " + | |
| " are being used, so it schedules appropriately." + | |
| "The sizes map to labels, which can be used to override the meaning of the sizes. See " + | |
| "@rules_foreign_cc//foreign_cc/settings:size_{size}_{cpu|mem}" | |
| "Set the approximate size of this build. This does two things:\n\n" | |
| "1. Sets environment variables to tell the underlying build system the\n" | |
| " requested level of parallelism; examples are CMAKE_BUILD_PARALLEL_LEVEL\n" | |
| " for CMake or MAKEFLAGS for autotools.\n" | |
| "2. Sets the resource_set attribute on the action to tell Bazel how many\n" | |
| " cores and how much memory are being used, so it can schedule\n" | |
| " appropriately.\n\n" | |
| "Each size value maps to build-setting labels that control the CPU and\n" | |
| "memory assigned to that size. You can override the meaning of a size by\n" | |
| "changing the values of the labels\n" | |
| "@rules_foreign_cc//foreign_cc/settings:size_<size>_cpu and\n" | |
| "@rules_foreign_cc//foreign_cc/settings:size_<size>_mem." |
| resource_size = "medium", | ||
| tags = ["manual"], |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the cmake_tool has resource_size set to "medium" while other built tools (make_tool on line 23, ninja_tool on line 86, meson_tool on line 113, and pkgconfig_tool on line 159) don't have resource_size specified. For consistency and to fully realize the benefits of this feature, consider whether these other build tools should also have appropriate resource_size values set. For example, building make, ninja, or pkgconfig from source could also benefit from resource allocation hints.
This is an attempt to make parallelization of builds safer by telling bazel what resources an action will consume. By default, Bazel allocates 1 CPU and 250M of RAM, and this makes the current methods of parallelization (i.e.
--action_env=CMAKE_BUILD_PARALLEL_LEVEL=16) pretty unpleasant, since bazel won't know and will happily schedule 160 cores worth of actions on a 10-core machine.This is currently up to get early feedback; some things that are missing are:
Related to #329
A description of the intended workflow here is:
resource_size=tiny|small|medium|large|enormousas they see fit.CMAKE_BUILD_PARALLEL_LEVEL)@rules_foreign_cc//foreign_cc/settings:size_$size_(cpu|mem)@rules_foreign_cc//foreign_cc/settings:size_defaultCurrently, changing the values of these settings does invalidate the cache.