[Bug]: Properties being removed from project as part of solution build, leading to race conditions.

orehmane commented 1 year ago

Issue Description

Hello! We've been having intermittent failures when building one of our solutions. It includes a library being built at the top-level as well as because it is a dependency of another project in the solution. We are running into the file access issue described here, suggesting a race condition during the build.

I investigated the binlogs (would like to avoid posting them), and indeed there is a difference in Properties/Global for both of them. The build triggered because it's included as a dependency is missing TargetFramework. So that presumably is causing the project to be built twice.

My confusion is that the log includes a line that says

Removing Properties for project

and the project in question. It is removing one property: TargetFramework.

So, in summation, based on my research, the project is being built twice, leading to a race condition, because it is missing TargetFramework, and it is missing TargetFramework because something (MSBuild? dotnet?) is deciding to remove the property.

My question is, is my assessment correct? And if so, how do I get it to stop removing that property?

Apologies if this has been answered elsewhere or already filed, I couldn't find it.

Steps to Reproduce

The command I'm running is

dotnet build $SOLUTION_FILE --configuration Release --self-contained True --verbosity Normal /property:WarningLevel=0 --noLogo -p:$RUNTIME_PROP_NAME=win-x64 --framework net6.0

Expected Behavior

Build consistently succeeds.

Actual Behavior

Build intermittently fails with a file access error.

Analysis

No response

Versions & Configurations

Windows 11 dotnet 6.0.406 dotnet msbuild --version outputs

MSBuild version 17.3.2+561848881  for .NET
17.3.2.46303

rainersigwald commented 1 year ago

This is the expected behavior, though I'm having a hard time finding a canonical bug--https://github.com/dotnet/sdk/issues/26690 is too new. https://github.com/dotnet/sdk/issues/9585 is also related.

In short: don't specify a TargetFramework when building a solution, because it causes build race conditions exactly as you describe.

@dsplaisted do we have a better canonical bug for this problem? We should also consider doing TF negotiation from sln->individual projects again.

orehmane commented 1 year ago

Thanks for your help! So in the case of a solution that includes multi-targeted projects, it's not possible to build only for a specific framework without causing this race condition?

jrdodds commented 1 year ago

@orehmane Just to provide some background detail that may help:

A solution file (.sln) is not an MSBuild file. When a solution file is passed to MSBuild, MSBuild creates a 'meta project' in memory and runs it. The meta project builds each of the projects included in the solution.

The Removing Properties for project message indicates that one or more global properties are being 'undefined' for a project.

The --framework option to dotnet build manipulates the TargetFramework property. The documentation for the --framework option says that "The framework must be defined in the project file." so, in broad strokes, the --framework option performs an intersection (aka AND) operation against a project's existing TargetFramework property and updates the TargetFramework property value to the result. The solution meta project doesn't define a TargetFramework property for itself.

(Updated to strikeout and remove incorrect information.)

rainersigwald commented 1 year ago

in the case of a solution that includes multi-targeted projects, it's not possible to build only for a specific framework without causing this race condition?

Correct.

rainersigwald commented 1 year ago

in broad strokes, the --framework option performs an intersection (aka AND) operation against a project's existing TargetFramework property and updates the TargetFramework property value to the result.

@jrdodds this isn't quite right----framework {foo} is implemented by passing -p:TargetFramework={foo} along to MSBuild. It doesn't inspect the project file (or solution) that's being built first. The docs are indicating that it is confusing and almost certainly wrong to override what's in the project file--though it can in some cases work (for instance if you have a plain template 6.0 console app, you can build it as 8.0 with dotnet build --framework net8.0 and it'll almost certainly work, since there aren't any significant breaking changes to it in the net6->net8 update).

jrdodds commented 1 year ago

@rainersigwald Thanks for the correction.

rainersigwald commented 1 year ago

Details on the race:

Given an Overall.sln solution with two projects, a WebApp.csproj that has <TargetFrameworks>net6.0;net7.0</TargetFrameworks> and a Library.csproj with only <TargetFramework>net6.0</TargetFramework>, if you specify a TF as a global property for the solution, you get this:

---
title: Solution TargetFramework race
---
flowchart TD
    subgraph WebApp.csproj
        P_net6[WebApp<br/>TF=net6.0]
    end
    subgraph "Library.csproj"
        L_net6[Library<br/>global TF=net6.0]
        L[Library<br/>global TF=''<br/>TF=net6.0]
    end
    sln[Overall.sln] --> P_net6
    sln --> L_net6
    P_net6 <-->|get TFs| Library.csproj
    P_net6 --> L

The bottom two have all the same properties (TF=net6.0) but appear different to the MSBuild engine, because their global properties are different. So they run separately and in parallel, and can cause data races.