Fix input root creation for path mapped workers#23991
Fix input root creation for path mapped workers#23991fmeum wants to merge 2 commits intobazelbuild:masterfrom
Conversation
github-actions
bot
added
team-Local-Exec
|
@coeuvre Can you take a look at this one? |
|
How urgent is it? Otherwise I would like to let Tiago review it because he has all the knowledge around this area. |
|
It's not urgent, take your time. |
| // We consider `prepareExecution` to be also part of setup. | ||
| Stopwatch prepareExecutionStopwatch = Stopwatch.createStarted(); | ||
| worker.prepareExecution(inputFiles, outputs, key.getWorkerFilesWithDigests().keySet()); | ||
| Set<PathFragment> workerFiles = key.getWorkerFilesWithDigests().keySet(); |
There was a problem hiding this comment.
I am a little confused; shouldn't the path mapping transformation be applied before computing the worker key, not after? In other words, if two actions have the same set of tool inputs after path mapping (but not before), shouldn't they share the same set of persistent workers?
This seems desirable because currently --worker_max_instances=Foo=N doesn't actually mean "at most N workers will be spawned", but rather "at most N*M where M is the number of distinct configurations", and normalizing the key would at least mitigate it. (But maybe there's a subtle reason we can't do this and maintain correctness?)
It would also remove the asymmetry between tool inputs and other inputs in this file (it looks like inputFiles have already been path-mapped elsewhere?) which would help readability.
There was a problem hiding this comment.
Thanks for the suggestion, looks much better now.
The set of worker files wasn't path mapped, which resulted in build failures when using a generated multiplex worker.
fmeum
force-pushed
the
path-mapping-generated-worker
branch
from
c782a62 to
8c6a0fb
Compare
|
@bazel-io fork 8.0.0 |
tjgq
added
awaiting-PR-merge
github-actions
bot
removed
the
awaiting-PR-merge
The set of worker files wasn't path mapped, which resulted in build failures when using a generated multiplex worker. Fixes bazelbuild#23990 Closes bazelbuild#23991. PiperOrigin-RevId: 691114049 Change-Id: I50cc2daa1c9fe7102ba99893aa878fb4c6b1c3d3
The set of worker files wasn't path mapped, which resulted in build failures when using a generated multiplex worker. Fixes #23990 Closes #23991. PiperOrigin-RevId: 691114049 Change-Id: I50cc2daa1c9fe7102ba99893aa878fb4c6b1c3d3 Commit 9d51ede Co-authored-by: Fabian Meumertzheim <[email protected]>
The set of worker files wasn't path mapped, which resulted in build failures when using a generated multiplex worker. Fixes bazelbuild#23990 Closes bazelbuild#23991. PiperOrigin-RevId: 691114049 Change-Id: I50cc2daa1c9fe7102ba99893aa878fb4c6b1c3d3
4 participants
The set of worker files wasn't path mapped, which resulted in build failures when using a generated multiplex worker.
Fixes #23990