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

Building roslyn in the VMR fails non-deterministically #4390

Closed
ViktorHofer opened this issue May 8, 2024 · 10 comments · Fixed by dotnet/roslyn#73468
Closed

Building roslyn in the VMR fails non-deterministically #4390

ViktorHofer opened this issue May 8, 2024 · 10 comments · Fixed by dotnet/roslyn#73468

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented May 8, 2024

Sometimes roslyn fails to build in the VMR because of a race condition:

CSC : error CS2012: Cannot open 'D:\a_work\1\vmr\src\roslyn\artifacts\obj\SemanticSearch.ReferenceAssemblies\Release\net8.0\SemanticSearch.ReferenceAssemblies.dll' for writing -- 'The process cannot access the file 'D:\a_work\1\vmr\src\roslyn\artifacts\obj\SemanticSearch.ReferenceAssemblies\Release\net8.0\SemanticSearch.ReferenceAssemblies.dll' because it is being used by another process.' [D:\a_work\1\vmr\src\roslyn\src\Tools\SemanticSearch\ReferenceAssemblies\SemanticSearch.ReferenceAssemblies.csproj]

Example build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=669255&view=logs&jobId=9050e078-31bf-5111-d8ec-8b6fa95caf9c&j=9050e078-31bf-5111-d8ec-8b6fa95caf9c&t=523d8a07-ca4d-5c7a-367a-d86c5fc4038b

This is probably due to the project being built twice with the same outputs (an anti-pattern in msbuild). The binlog should tell. @jaredpar who would be a good person on the roslyn team to take a look?

Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@jaredpar
Copy link
Member

jaredpar commented May 9, 2024

This is probably due to the project being built twice with the same outputs (an anti-pattern in msbuild). The binlog should tell. @jaredpar who would be a good person on the roslyn team to take a look?

I only see one binlog attached to that pipeline and it's for the outer build. The inner builds only seem to have non-binary logs. Am I looking in the wrong place?

image

@mthalman
Copy link
Member

You can find the inner log here:

image

@ViktorHofer
Copy link
Member Author

Yes. It's the "source-inner-build.binlog".

@jaredpar
Copy link
Member

I've looked at the binlog and it does seem like there are two paths to building SemnaticSearch.ReferenceAsesmblies.csproj

image

image

At the same time though ... I don't see how that is a build authoring error on our part. There is nothing particularly special I can see here. The relationship between Roslyn.VisualStudio.Setup.csproj to SemanticSearch.ReferenceAssemblies.csproj is a simple ProjectReference.

It does seem odd that Microsoft.VisualStudio.LanguageServices.New.IntegrationTests.csproj has a .metaproj. That's beyond my understanding of msbuild.

@ViktorHofer
Copy link
Member Author

I see that the csc task is invoked twice non-incrementally on the non-multi-targeting project via this msbuild binlog viewer query:

$task csc under($project SemanticSearch.ReferenceAssemblies.csproj)

image

Here's the global property diff for those two project evaluations: https://www.diffchecker.com/YbB9Ksu0/
Because the global properties differ, the project builds twice.

One evaluation has an additional TargetFramework global property which is wrong as single targeting projects shouldn't have that. This is because of this code: https://github.com/dotnet/roslyn/blob/324e078a9b47b62cd6dec6c78c6308c4540aa1e5/src/VisualStudio/Setup/Roslyn.VisualStudio.Setup.csproj#L311-L312

Remove SetTargetFramework and the overbuilding and race condition will go away. FWIW the msbuild team is adding analyzers that will detect such cases.

@tmat
Copy link
Member

tmat commented May 14, 2024

FWIW the msbuild team is adding analyzers that will detect such cases.

We need to enable these analyzers for all our repos asap.
@rainersigwald

@rainersigwald
Copy link
Member

They don't exist yet, but @JanKrivanek will be happy to have eager customers when they do.

@jaredpar
Copy link
Member

In this case it looks like the problem is that SemanticSearch.Reference.Assemblies targets net8.0 while the VS project targets net472. Assume the SetTargetFramework element is trying to resolve that problem. What is the correct way to solve that?

@tmat

jaredpar pushed a commit to dotnet/roslyn that referenced this issue May 14, 2024
* Stop overbuilding SemanticSearch.ReferenceAssemblies

Fixes dotnet/source-build#4390

* Update Roslyn.VisualStudio.Setup.csproj

* Update Roslyn.VisualStudio.Setup.csproj
@ViktorHofer
Copy link
Member Author

A ProjectReference pointing to a single targeting project doesn't need and shouldn't have the SetTargetFramework metadata as there's only one TFM in the project. The important part is the SkipGetTargetFrameworkProperties metadata which silences NuGet's compatibility analysis. For multi-targeting projects, the usage of SetTargetFramework is valid if you want to choose the nearest compatible TFM yourself (instead of NuGet).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants