-
Notifications
You must be signed in to change notification settings - Fork 1.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 launching net taskhost - initial implementation #11393
base: main
Are you sure you want to change the base?
Support launching net taskhost - initial implementation #11393
Conversation
2d1cd86
to
9b892e9
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.
Some general questions as I was idly looking at the work so far :) Happy to see this start!
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs
Outdated
Show resolved
Hide resolved
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs
Outdated
Show resolved
Hide resolved
{ | ||
// Start the new process. We pass in a node mode with a node number of 2, to indicate that we | ||
// want to start up an MSBuild task host node. | ||
commandLineArgs = $"/nologo /nodemode:2 /nodereuse:{ComponentHost.BuildParameters.EnableNodeReuse} /low:{ComponentHost.BuildParameters.LowPriority}"; |
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 rest of these arguments (node reuse/priority) should be applied to the .NET-host command line too, right?
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.
not sure
@rainersigwald could you clarify this point, please?
if (!string.IsNullOrWhiteSpace(realTaskAssemblyLoaction) && | ||
realTaskAssemblyLoaction != _taskFactoryWrapper.TaskFactoryLoadedType.Path) | ||
// TODO ask why for net task host it returns false net472\MSBuild\Current\Bin\Microsoft.Build.dll instead of path to a custom task. | ||
// Interestingly TaskInstance._taskType contains the correct path. |
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.
@rainersigwald , it's a question for you when you will review :)
ad14fb9
to
65691df
Compare
Partially Fixes #11331
To test the feature, setup these env variables:
MSBuildToolsDirectoryNET
= e.g. "C:\msbuild\msbuild_yk\msbuild\artifacts\bin\bootstrap\core"MSBuildAssemblyDirectory
= e.g. "C:\msbuild\msbuild_yk\msbuild\artifacts\bin\bootstrap\core\sdk\9.0.200-preview.0.24603.3"Keep in mind, due to current handshake mechanism, only matching version of sdk can be launched.
The tests are commented because we don't expect them to run now for NET runtime. There is a separate task to cover this functionality.