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

Visual Studio FastUpToDate assumes Up-to-Date wrongly #6713

Closed
felix-ri opened this issue Jul 28, 2021 · 14 comments
Closed

Visual Studio FastUpToDate assumes Up-to-Date wrongly #6713

felix-ri opened this issue Jul 28, 2021 · 14 comments
Assignees
Labels
Area: Visual Studio Issues related to interactions with Visual Studio or project systems. bug needs-attention Needs a member of the team to respond. needs-design Requires discussion with the dev team before attempting a fix. triaged
Milestone

Comments

@felix-ri
Copy link

felix-ri commented Jul 28, 2021

Issue Description

When incrementally building a project with a dependency to another project, content files of the latter won't update in the output directory of the former.

Steps to Reproduce

Sample

  • MSBuildBug.zip
  • run the sample, then change the content of the Content file

Manually

  • Create a dotnet 5.0 executable Project and a library Project
  • Make the executable Project reference the library Project
  • Add a testfile.txt to the library as Content, PreserveNewest
  • Build the executable
  • The testfile.txt gets copied into the output directory of the executable
  • Change the content of the testfile.txt
  • build the executable again

Expected Behavior

  • the output directory of the executable should contain the updated content file.

Actual Behavior

  • the output directory of the executable only contains the old one.

Analysis

  • it doesn't happen when building with dotnet build
  • Visual Studio's FastUpToDate doesn't consider the executable project out of date and doesn't call msbuild for it.
  • FastUpToDate doesn't know about the content files, but seems to rely on the CopyCompleted file of the library to be touched to determine the UpToDateness of dependent projects.
  • In the Microsoft.Common.CurrentVersion.targets only the Target _CopyFilesMarkedCopyLocal does touch the CopyCompleted file.
  • but for subsequent builds the _CopyFilesMarkedCopyLocal is skipped; _CopyOutOfDateSourceItemsToOutputDirectory does the copying then.

Versions & Configurations

  • OS: Win10
  • Msbuild 17.0.0-preview-21329-01+1b7661f36 with Visual Studio Professional 2022 Preview (64-bit) Version 17.0.0 Preview 2.1
  • Msbuild 16.10.2+857e5a733 with Visual Studio Professional 2019 Version 16.10.4
@felix-ri felix-ri added bug needs-triage Have yet to determine what bucket this goes in. labels Jul 28, 2021
@benvillalobos
Copy link
Member

This may be fixed as of #6622. What happens when you set MSBuildCopyContentTransitively to true in your project?

@felix-ri
Copy link
Author

I added <MSBuildCopyContentTransitively>true</MSBuildCopyContentTransitively> to the properties of both projects, but the behaviour is still the same.

@benvillalobos
Copy link
Member

Team Triage: This doesn't reproduce with dotnet msbuild or msbuild on the command line so it likely isn't MSBuild. Is this a project-system issue? cc @ocallesp

@drewnoakes
Copy link
Member

@benvillalobos I think @felix-ri's analysis is correct. The VS up-to-date check uses the copy marker to identify the up-to-date state of referenced projects. Can the CopyCompleted marker file be touched in _CopyOutOfDateSourceItemsToOutputDirectory rather than _CopyFilesMarkedCopyLocal?

@benvillalobos
Copy link
Member

Can the CopyCompleted marker file be touched in _CopyOutOfDateSourceItemsToOutputDirectory rather than _CopyFilesMarkedCopyLocal?

@rainersigwald Do you have an idea the impact of this change?

@felix-ri
Copy link
Author

felix-ri commented Aug 6, 2021

The problem also occurs with <CopyToOutputDirectory>Always</CopyToOutputDirectory>. The corresponding target _CopyOutOfDateSourceItemsToOutputDirectoryAlways doesn't touch the Marker File either.

@benvillalobos
Copy link
Member

Team Triage: What's happening is the exe project doesn't have testfile.txt as an input. As a result, FUTD thinks it's up to date WRT its inputs because it's not aware of that input.

If we were to change where the CopyComplete marker executed, it wouldn't have an impact because it only cares about implementation assemblies. @drewnoakes, are you suggesting changing the copycomplete marker to mean "did literally any copy actually happen?" If so, it may impact downstream referencing projects, but we don't think it's guaranteed to do so. FUTD would fail or return false more often. We'd have to decide whether that's an acceptable tradeoff.

@rainersigwald rainersigwald added Area: Visual Studio Issues related to interactions with Visual Studio or project systems. needs-design Requires discussion with the dev team before attempting a fix. and removed needs-triage Have yet to determine what bucket this goes in. labels Aug 27, 2021
@rainersigwald
Copy link
Member

Another possibility here could be to skip FUTD on any project where a referenced project built (at the project-system layer). That wouldn't be ideal (especially for cross-project-system things like referencing a csproj.dll project) but could avoid a bunch of these "we don't know the full input closure because it's a build-time-discovered thing" problems.

@drewnoakes
Copy link
Member

@drewnoakes, are you suggesting changing the copycomplete marker to mean "did literally any copy actually happen?" If so, it may impact downstream referencing projects, but we don't think it's guaranteed to do so. FUTD would fail or return false more often. We'd have to decide whether that's an acceptable tradeoff.

I'm not aware of other consumers of that file, but if a project changed in such a way that a consuming project must be rebuilt, then that needs to be visible to the VS FUTD check. The CopyComplete marker is the current mechanism for this. Considering only Visual Studio's consumption of that file, expanding the occasions in which it's touched will trigger more builds in VS. But the problem here is that we're not building when we should, and so that sounds like the fix we want.

My understanding is that a more optimal solution would be to have all depending projects model the CopyToOutputDirectory items of those projects they depend upon, taking into account any PrivateAssets logic. Absent that, simply touching the marker seems the correct behaviour to me here.

Another possibility here could be to skip FUTD on any project where a referenced project built (at the project-system layer).

This would require deep changes to VS, as VS builds project-by-project and very little information flows between builds today, other than simple status flags.

@jcouv
Copy link
Member

jcouv commented Sep 24, 2021

From chat with @rainersigwald we first need to sort out ownership (is this an msbuild or a project system bug?). He'll follow up with @drewnoakes .
But we agree it's definitely important to fix in 17.0 timeframe.
Thanks!

@jcouv
Copy link
Member

jcouv commented Oct 1, 2021

@rainersigwald @drewnoakes I didn't see any update on this. Did you get a chance to discuss and confirm the root cause?

@benvillalobos benvillalobos added the needs-attention Needs a member of the team to respond. label Oct 4, 2021
@Valdiralita
Copy link

This issues impacts our solution of 110 projects.
As a workaround we use <DisableFastUpToDateCheck>true</DisableFastUpToDateCheck> in the projectwide Directory.Build.props which helps that the build is correct but it takes longer to partially build the solution.

@marcpopMSFT marcpopMSFT modified the milestones: VS 17.1, VS 17.2 Jan 7, 2022
@drewnoakes
Copy link
Member

This is a duplicate of dotnet/project-system#4665. @PathogenDavid has kindly documented some workarounds.

We are working on a fix for it at the moment. dotnet/project-system#7932 does fix this, but introduces some performance regressions that I'm trying to work around right now. It is expensive to have each project recurse through its references to gather items (which that PR does). That PR also won't keep the list of items up to date over time as referenced projects change, as we don't trigger design-time builds when referenced projects change.

@marcpopMSFT
Copy link
Member

Per Drew's comment, closing out this issue and will track it on the project system side going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Visual Studio Issues related to interactions with Visual Studio or project systems. bug needs-attention Needs a member of the team to respond. needs-design Requires discussion with the dev team before attempting a fix. triaged
Projects
None yet
Development

No branches or pull requests

8 participants