-
Notifications
You must be signed in to change notification settings - Fork 446
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
Don't use the MSBuild server process for init-build.proj. #14643
Don't use the MSBuild server process for init-build.proj. #14643
Conversation
This appears to fix an issue where our SdkResolver is not loaded because there are still lingering dotnet processes when we try to binplace the resolver DLL.
You should also be able to shutdown any persistent servers (MSBuild or compiler) with |
@baronfel, Do you know if there was any behavior change in this space? This code hasn't changed since the beginning of 7.0 and recently broke. Is it expected to have to explicitly shutdown the build server to get new SdkResolvers to load? |
Yeah - in 7.0 rc1 MSBuild server was enabled by default, which is why I think you're seeing this behavior now. From that time we've squashed a few bug in it, but we are still deciding if the feature should be on for the release. I'm not sure if I'd classify this specific interaction as a bug - for that I'll tag @rokonec for his opinion. |
I think using the shutdown command is better as it's more clear in the intent. I've changed the PR to use that and added a comment about what we're trying to do. |
I would very like to understand why is lingering dotnet processes of MSBuild server an issue for you? Can you please explain... |
@crummel - you mentioned the MSBuild server is going to be disabled by default for 7.0 and this change would not be required. Can you link to the corresponding msbuild PR issue discussing this? |
Saving some time since I saw this pop up: dotnet/sdk#28369 is the PR to change the default behavior to be an opt-in instead of an opt out. we got tactics approval yesterday so should merge today. |
rokonec - Can you answer the question if it is expected to have to explicitly shutdown the build server to get new SdkResolvers to load? We are trying to make an informed decision if we should proceed with this PR even though the default behavior is being changed to opt-in? |
@MichaelSimons I believe |
* Adding ppc64le arch for source build (#14631) * Remove llvm-project from source-build as we have removed this dependency in runtime. (#14697) * Remove llvm-project from source-build as we have removed this dependency in runtime. * Add patch for the backport PR to get this to build for now. * Remove unneeded copy of this patch. * [release/7.0.1xx] Update dependencies from dotnet/sdk (#14705) [release/7.0.1xx] Update dependencies from dotnet/sdk - Remove backported command-line-api patch * Don't use the MSBuild server process for init-build.proj. (#14643) * Don't use the MSBuild server process for init-build.proj. This appears to fix an issue where our SdkResolver is not loaded because there are still lingering dotnet processes when we try to binplace the resolver DLL. * Address code review feedback. * Update patch backport comments and remove obsolete patch (#14710) * [release/7.0.1xx] .NET Source-Build 7.0.100-rc.2 October 2022 Updates (#14728) * Update source-build to 7.0 RC2 * Also update tarball global.json * Don't touch root global.json in case it is managed by automation * Add back source-build patch that is still required in this branch Co-authored-by: Swapnali911 <Swapnali.Pawar1@ibm.com> Co-authored-by: Chris Rummel <crummel@microsoft.com> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Michael Simons <msimons@microsoft.com> Co-authored-by: Logan Bussell <loganbussell@microsoft.com> Co-authored-by: Jason Zhai <v-wuzhai@microsoft.com>
This appears to fix an issue where our SdkResolver is not loaded because there are still lingering dotnet processes when we try to binplace the resolver DLL.
Fixes dotnet/source-build#3021.