Skip to content
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

[Bug]: Msbuild crashes when using cache extension with proxy targets pointing to original targets #9117

Open
jacdavis opened this issue Aug 9, 2023 · 3 comments · Fixed by #9130
Assignees

Comments

@jacdavis
Copy link

jacdavis commented Aug 9, 2023

Issue Description

While implementing build acceleration, I ran into an issue where msbuild is crashing. Csproj passes "Build" and 10 other targets related to scraping the outputgroup data ofr the DTE. These are the additional targets:
"BuiltProjectOutputGroup"
"BuiltProjectOutputGroupDependencies"
"DebugSymbolsProjectOutputGroup"
"DebugSymbolsProjectOutputGroupDependencies"
"DocumentationProjectOutputGroup"
"DocumentationProjectOutputGroupDependencies"
"SatelliteDllsProjectOutputGroup"
"SatelliteDllsProjectOutputGroupDependencies"
"SGenFilesOutputGroup"
"SGenFilesOutputGroupDependencies"

In speaking to David Federman, the design is I should be able to return these as proxies to themselves so msbuild will just execute them. However, when I do that, msbuild crashes because they are already added to the build result collection. Callstack is here:
Severity Code Description Project File Line Suppression State
Error This is an unhandled exception in MSBuild -- PLEASE UPVOTE AN EXISTING ISSUE OR FILE A NEW ONE AT
https://aka.ms/msbuild/unhandled

Microsoft.Build.Framework.InternalErrorException: MSB0001: Internal MSBuild Error: Items already exist for target BuiltProjectOutputGroup.

at Microsoft.Build.Shared.ErrorUtilities.ThrowInternalError(String message, Object[] args)
at Microsoft.Build.Shared.ErrorUtilities.VerifyThrow(Boolean condition, String unformattedMessage, Object arg0)
at Microsoft.Build.Execution.BuildResult.AddResultsForTarget(String target, TargetResult result)
at Microsoft.Build.BackEnd.RequestBuilder.g__CopyTargetResultsFromProxyTargetsToRealTargets|68_0(BuildResult resultFromTargetBuilder)
at Microsoft.Build.BackEnd.RequestBuilder.d__68.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.Build.BackEnd.RequestBuilder.d__59.MoveNext() ClassLibrary1

Steps to Reproduce

  1. Create an msbuild cache extension and invoke that extension from csproj
  2. Create a new build result where the proxy targets match the existing targets.

Expected Behavior

This works

Actual Behavior

msbuild crashes

Analysis

See above

Versions & Configurations

No response

@jacdavis jacdavis added bug needs-triage Have yet to determine what bucket this goes in. labels Aug 9, 2023
@dfederm
Copy link
Contributor

dfederm commented Aug 9, 2023

Note to MSBuild team: This is something I should look at

@JanKrivanek JanKrivanek removed the needs-triage Have yet to determine what bucket this goes in. label Aug 10, 2023
@JanKrivanek
Copy link
Member

@dfederm - assigning to you. Please let us know if you'd want any assitance or to hand off the investigation

AR-May pushed a commit that referenced this issue Oct 17, 2023
Fixes #9117

For project cache plugins to only partially handle a build request, it makes sense for it to only proxy some targets and not others. For example, in VS the build request has:

"Build"
"BuiltProjectOutputGroup"
"BuiltProjectOutputGroupDependencies"
"DebugSymbolsProjectOutputGroup"
"DebugSymbolsProjectOutputGroupDependencies"
"DocumentationProjectOutputGroup"
"DocumentationProjectOutputGroupDependencies"
"SatelliteDllsProjectOutputGroup"
"SatelliteDllsProjectOutputGroupDependencies"
"SGenFilesOutputGroup"
"SGenFilesOutputGroupDependencies"
"Build" is the only relevant one that a plugin would want to handle, and would likely redirect to "GetTargetPath", while the rest are "information gathering" targets which should just be "passed through" and allowed to execute as-is.

Today if the plugin does this, the targets it does not proxy are dropped entire, which would make the caller confused about not getting results for a target they specifically requested. This change instead just fills in the remaining targets which were not proxied.

Example:
Original request: Build, BuiltProjectOutputGroup, BuiltProjectOutputGroupDependencies, ...
Cache plugin returns proxy targets Build -> GetTargetPath, and nothing more.
New request: GetTargetPath, BuiltProjectOutputGroup, BuiltProjectOutputGroupDependencies, ...

This fixes #9117 indirectly by not requiring a plugin to handle targets it wants to pass through at all, allowing it to just ignore them.
@JanKrivanek
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants