dotnet / project-system

The .NET Project System for Visual Studio
MIT License
968 stars 387 forks source link

Inconsistent copying of duplicate output folder items under build acceleration #9001

Closed hotfix-houdini closed 5 months ago

hotfix-houdini commented 1 year ago

Visual Studio Version

17.5.5

Summary

With Build Acceleration enabled, Visual Studio is copying "Copy to output directory" same-named files from other projects, into the project that's running via fast build. It will alternate between copying the incorrect file (from another project) to the output directory, and the correct file (from the project that's running).

Steps to Reproduce

See the vs-futdc-bug-demo repo to pull and immediately observe the issue, or for in-depth reproduction steps.

Expected Behavior

Actual Behavior

Build Acceleration copies the wrong file(s) over - alternating between the wrong and correct file every other Fast Build.

User Impact

In our case, erroneous test fails every other time as they depend on an appsettings.json file that's different than the project under test. We have to run our tests twice (once with bad config and they fail immediately, and a second time with the good config and an actual test run).

For work arounds we can either:

It seems like there could be broader impact though, if any same-name file from any referenced project could be copied over to the output bin. Maybe only if they trigger a fast-up-to-date-check like if the file has been read.

haileymck commented 1 year ago

@drewnoakes can you look into this?

drewnoakes commented 1 year ago

This is an interesting one, thanks for the report.

So, this happens when multiple projects that contribute a file with the same name to the output directory.

We need to see what MSBuild does here and replicate that.

Given...

graph LR;
  App --> Lib1;
  App --> Lib2;

...there are two scenarios of interest.

  1. Both Lib1 and App produce the same file. From the above, it sounds like App should win, which seems reasonable.
  2. Both Lib1 and Lib2 produce the same file. It's less clear who should win here. It might be based on the order of the ProjectReference elements in the project file.

Both of these should be investigated.

drewnoakes commented 1 year ago

A quick investigation using builds without acceleration:

This will need some further investigation before we can apply a fix.

drewnoakes commented 11 months ago

Also reported in https://developercommunity.visualstudio.com/t/AccelerateBuildsInVisualStudio-is-causin/10450799

niemyjski commented 8 months ago

This is still an issue, please fix.

MattOG commented 7 months ago

Just spent two days trying to figure out why my appsettings.json wasn't copying over on a new machine when it works fine on an older one.

I wasn't seeing the same as OP though. In my instance doing "Build/Rebuild" always used the correct file (App from @drewnoakes reply above) however debugging always used the incorrect one (Lib1), unless I deleted the bin/Debug folder. In that instance the very first time debugging would use the App appsettings.json, and every subsequent one would use Lib1's appsettings.json.

So, that's a long winded way of saying, it's happening to me too, and please fix it.

simonfox commented 7 months ago

We have just started to experience this issue after VS2022 17.9 upgrade.

We're on .net8.0 and disabling accelerated builds works as a workaround

<PropertyGroup>
    <AccelerateBuildsInVisualStudio>false</AccelerateBuildsInVisualStudio>
</PropertyGroup>
jchesshirAspire commented 7 months ago

I have to wonder why this was not considered a blocking issue for #9106.

DaveSenn commented 6 months ago

Just had the pleasure of debugging some unit tests for hours to find this as the cause of my failed tests. Please fix it.

drewnoakes commented 5 months ago

Thanks everyone for your patience here. A fix is in review: https://github.com/dotnet/project-system/pull/9454

Teaching Build Acceleration to understand the exact ordering the MSBuild would use is complex and error prone. Instead, I've opted to have BA disable itself when it detects that multiple copy items want to write to the same path in the project's output. That way, in the rare case that this situation exists, we will fall back to calling MSBuild.

With this, the FUTDC log will show a message like:

Build acceleration is not available for this project because it copies duplicate files to the output directory: 'appsettings.json'

simonfox commented 5 months ago

Reposting from developer community issue as this is probably a better place for the discussion...

In my case I have projects A, B and C - all have an appsettings.json C has a project reference to A and to B. I would expect that the output folder for C contains the appsettings.json of C itself - in my case I don’t think there are any rules in play, as the file that wins seems to be nondeterministic. Wouldn’t taking the local version be the logical solution? Is this really a rare case?

drewnoakes commented 5 months ago

@simonfox I agree and did consider that. My concern is for cases like UseCommonOutputDirectory. Perhaps we'll try it in future, and handle that case. There are trade-offs to be made when trying to match the behaviour of a different software system (e.g. MSBuild) without actually asking that system, especially when that system is infinitely customisable and Turing complete.