Don't override NUM_JOBS if already set #3258
Conversation
UebelAndre
left a comment
UebelAndre
left a comment
There was a problem hiding this comment.
Thanks! Just had one request
| }) | ||
|
|
||
| # Allow the user to override the number of jobs for the build script. | ||
| if "NUM_JOBS" not in env: |
There was a problem hiding this comment.
If this is any value larger than 1 then it would be good to have an associated resource set
There was a problem hiding this comment.
I don't know what that means, what's an "associated resource"?
There was a problem hiding this comment.
Basically a change similar to #3112
which uses ctx.actions.run.resource_set
There was a problem hiding this comment.
You'll find the big ugly file rust/private/rustc_resource_set.bzl and it's use in rust/private/rustc.bzl
resource_set = get_rustc_resource_set(toolchain)There was a problem hiding this comment.
@UebelAndre I can't call get_rustc_resource_set as it stands without refactoring it to take an int instead of a toolchain, do you want me to do that, or duplicate rust/private/rustc_resource_set.bzl with an int variant?
There was a problem hiding this comment.
You could just add a new function. get_cargo_build_script_resource_set or something that does what you need. When I wrote that all I was thinking about was Rustc actions but I don't think we should duplicate these functions. So I'd just make a new function for the use here.
| }) | ||
|
|
||
| # Allow the user to override the number of jobs for the build script. | ||
| if "NUM_JOBS" not in env: |
There was a problem hiding this comment.
You could just add a new function. get_cargo_build_script_resource_set or something that does what you need. When I wrote that all I was thinking about was Rustc actions but I don't think we should duplicate these functions. So I'd just make a new function for the use here.
3 participants
Building large src crates (e.g. proj_sys) take a long time because NUM_JOBS was always 1. This allows the user to override (e.g. with --actiona_env) the value of NUM_JOBS