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

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

Closed
hotfix-houdini opened this issue May 4, 2023 · 12 comments · Fixed by #9454
Closed
Assignees
Labels
Feature-Build-Acceleration Build Acceleration can skip calls to MSBuild within Visual Studio Triage-Approved Reviewed and prioritized
Milestone

Comments

@hotfix-houdini
Copy link

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

  • we have ProjectA and ProjectA.Tests.
  • ProjectA.Tests references ProjectA.
  • both projects have an appsettings.json with different values.
  • both appsettings.json have Copy to Output Directory as Copy If Newer or Copy Always
  • run tests multiple times in ProjectA.Tests via fast build
  • Fast Build acceleration copies a file to the output directory each run - alternating between having appsettings.json from ProjectA (bad), and ProjectA.Tests (good).

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

Expected Behavior

  • Build Acceleration does not copy any files, as nothing has changed between test runs.
  • Unless reading a file on disk triggers the Fast Up To Date Check so build acceleration copies the file again to be safe, BUT Build Acceleration should not copy files from other referenced projects over; it should copy the correct file over.

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:

  • disable fast build :(
  • get hacky and compromise by adding some Fast Up To Date checks configuration which seems to force the building of the single project instead of the entire solution. Something like:
    <ItemGroup>
      <UpToDateCheckInput Include="appsettings.json" />
      <UpToDateCheckOutput Include="appsettings.json" />
    </ItemGroup>
  • live with the pain that every other fast build is going to give the wrong files in the output dir

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
Copy link
Contributor

@drewnoakes can you look into this?

@haileymck haileymck added the Triage-Approved Reviewed and prioritized label May 4, 2023
@drewnoakes
Copy link
Member

drewnoakes commented May 5, 2023

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;
Loading

...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 drewnoakes added Bug Feature-Build-Acceleration Build Acceleration can skip calls to MSBuild within Visual Studio labels May 5, 2023
@drewnoakes drewnoakes added this to the 17.7 milestone May 5, 2023
@drewnoakes drewnoakes changed the title Build Acceleration Copies Wrong Project's Output Files Inconsistent copying of duplicate output folder items under build acceleration May 5, 2023
@drewnoakes
Copy link
Member

A quick investigation using builds without acceleration:

  • In the case where App and Lib1 both produce the same file, App's file will always end up in the output directory, even if Lib1's is newer.
  • In the case where Lib1 and Lib2 both produce the same file, the newer of the two wins out.

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

@drewnoakes
Copy link
Member

@niemyjski
Copy link

This is still an issue, please fix.

@drewnoakes drewnoakes modified the milestones: 17.9, 17.10 Feb 14, 2024
@MattOG
Copy link

MattOG commented Feb 23, 2024

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
Copy link

simonfox commented Mar 6, 2024

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
Copy link

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

@DaveSenn
Copy link

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
Copy link
Member

Thanks everyone for your patience here. A fix is in review: #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
Copy link

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
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature-Build-Acceleration Build Acceleration can skip calls to MSBuild within Visual Studio Triage-Approved Reviewed and prioritized
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants