-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Sidecar taskhost #12071
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?
Sidecar taskhost #12071
Conversation
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 adds plumbing to support long‐lived TaskHost processes by distinguishing between build‐lived and long‐lived task hosts. The changes refactor handshake serialization to use a structured HandshakeComponents type, update constants and environment retrieval for MSBuild paths, and adjust logging and process‐launch logic accordingly.
- Introduce the _isNetRuntime flag in TaskHostConfiguration and update its constructor API.
- Replace raw int arrays with a structured HandshakeComponents type and update related handshake loops.
- Update constants, environment variable use, and process name references across the codebase for improved consistency.
Reviewed Changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/UnitTests.Shared/RunnerUtilities.cs | Use Constants.MSBuildAssemblyName for MSBuild assembly lookup. |
src/Shared/TaskHostConfiguration.cs | Introduce _isNetRuntime field and update constructor comments and parameters. |
src/Shared/NodePipeServer.cs | Switch from a for loop to a foreach loop (with manual index) when processing handshake components. |
src/Shared/NodePipeClient.cs | Update handshake serialization to use component enumeration. |
src/Shared/NodePipeBase.cs | Change HandshakeComponents property type from int[] to a structured type. |
src/Shared/NodeEndpointOutOfProcBase.cs | Add IsHandshakePartValid and related handshake improvements. |
src/Shared/CommunicationsUtilities.cs | Refactor handshake construction and update constants handling. |
src/Shared/BuildEnvironmentHelper.cs | Add NET-specific directory properties for tools and assemblies. |
src/MSBuild/MSBuild/Microsoft.Build.Core.xsd | Update runtime attribute documentation and enumeration to include NET. |
src/Build/Resources/Constants.cs | Introduce new constants for dotnet process names and MSBuild executable/assembly names. |
src/Build/Instance/TaskFactories/TaskHostTask.cs | Update TaskHost spawning logic and error logging naming. |
src/Build/Evaluation/IntrinsicFunctions.cs | Improve dictionary initialization and task host existence check. |
src/Build/BackEnd/** | Various updates addressing node launching, handshake logic, and process name consistency. |
.editorconfig | Minor header/BOM change. |
Comments suppressed due to low confidence (2)
src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs:340
- The typo 'realTaskAssemblyLoaction' has been corrected to 'realTaskAssemblyLocation' for improved clarity.
string realTaskAssemblyLocation = TaskInstance.GetType().Assembly.Location;
.editorconfig:1
- [nitpick] Ensure that the file encoding (with BOM) is intentional and consistent with the project’s standards.
# editorconfig.org
Fixes #11335
Context
We already had TaskHost live alongside the build. It would get cleaned up when the build is done.
This is some additional plumbing to allow for TaskHost to live even after the build is done.
Now we have two cases:
before merging this one, we need to merge Yuliias one first (since there is overlap, I based it on hers instead of on main)
#11393
Changes Made
Added plumbing to allow for long lived TaskHost spawning.
Testing
TODO
Notes