Skip to content

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Jan 13, 2023

This is an attempt to fix a race condition in our build, where Mono.Cecil was being built twice into the same output path. See dotnet/installer#15228 (comment) for context.

Before this change, I saw this line twice in the output:

Mono.Cecil -> C:\Users\svbomer\src\linker\artifacts\bin\Mono.Cecil\Debug\netstandard2.0\Mono.Cecil.dll

After this change, I only see it once. I don't understand why SetTargetFramework was needed in the first place in #1891 (comment), but it no longer seems necessary. Probably the recent changes to the cecil build (arcade onboarding) caused this to become a problem.

I have been able to build locally >100 times in a row with this change, without hitting the race, so I think this fixes it. Previously I was seeing it fail about one in ten builds locally.

@marek-safar
Copy link
Contributor

I don't think this change is fixing Mono.Cecil repeated builds, but it does not seem to hurt either.

@marek-safar marek-safar enabled auto-merge (squash) January 14, 2023 08:50
@marek-safar marek-safar merged commit 4452038 into dotnet:main Jan 14, 2023
Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. SetTargetFramework shouldn't be set when the referenced project doesn't multi-target as otherwise you could run into double evaluation and double builds. Great find!

tlakollo pushed a commit to tlakollo/runtime that referenced this pull request Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants