-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Support layering_check
with C++ path mapping
#22957
Conversation
a69252a
to
e63f761
Compare
BiFunction<ImmutableMap<String, String>, String, ImmutableMap<String, String>> | ||
modifyExecutionInfo) { |
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.
This design of passing in a BiFunction, checking if it would result in one key SUPPORTS_PATH_MAPPING and then modifying the execution info by adding that key, looks fishy.
I'd be more comfortable passing in ImmutableMap<String, String> executionInfo
and following SpawnAction example. Interning it and returning it in getExecutionInfo.
It will probably require a bit more internal testing, but it's also more future proof.
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.
I am now passing in the execution info map directly. I still need some extra logic to save a field (which would result in a larger instance size), PTAL.
src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
Show resolved
Hide resolved
@bazel-io flag |
@bazel-io fork 7.4.0 |
1cd4ca6
to
4cf6a54
Compare
@iancha1992 I resolved the merge conflict. |
4cf6a54
to
b57b931
Compare
@iancha1992 I fixed the build failure. |
@comius Friendly ping so that this doesn't get lost |
Users can opt into path mapping for C++ module map actions via `--modify_execution_info=CppModuleMap=+supports-path-mapping`. This is achieved by mapping paths in the module map files as well as converting the sequence variable for module map paths to a new structured `ArtifactSequenceVariable`. Also makes it so that `ExecutionServer` gracefully handles failing commands instead of crashing. Closes bazelbuild#22957. PiperOrigin-RevId: 675073116 Change-Id: I13835c7fb01354a89ec5fd141cf892c6b733efe4
Users can opt into path mapping for C++ module map actions via `--modify_execution_info=CppModuleMap=+supports-path-mapping`. This is achieved by mapping paths in the module map files as well as converting the sequence variable for module map paths to a new structured `ArtifactSequenceVariable`. Also makes it so that `ExecutionServer` gracefully handles failing commands instead of crashing. Closes #22957. PiperOrigin-RevId: 675073116 Change-Id: I13835c7fb01354a89ec5fd141cf892c6b733efe4 Fixes #23178
Users can opt into path mapping for C++ module map actions via
--modify_execution_info=CppModuleMap=+supports-path-mapping
. This is achieved by mapping paths in the module map files as well as converting the sequence variable for module map paths to a new structuredArtifactSequenceVariable
.Also makes it so that
ExecutionServer
gracefully handles failing commands instead of crashing.