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

IncrementalClean can delete a needed file from output #9709

Open
KirillOsenkov opened this issue Feb 3, 2024 · 3 comments
Open

IncrementalClean can delete a needed file from output #9709

KirillOsenkov opened this issue Feb 3, 2024 · 3 comments

Comments

@KirillOsenkov
Copy link
Member

KirillOsenkov commented Feb 3, 2024

If a project has its output directory outside the project directory, and it copies a dependent file to output for more than one reason (say, via None as well as via RAR), and you remove one reason, building the project deletes the file from output, even though the file still needs to be there. Building the project again brings the file back.

  1. unzip this into C:\temp\IncrementalClean or any other empty directory: IncrementalClean.zip
  2. msbuild /r
  3. ensure C:\temp\IncrementalClean\bin\B\A.dll exists (hereafter it's just called "The File")
  4. open C:\temp\IncrementalClean\B\B.csproj and comment out line 13 (to ensure the None item is not added), save file
  5. msbuild
  6. observe that The File was deleted by the IncrementalBuild target
  7. msbuild
  8. The File is back again

This is because @(ReferenceCopyLocalPaths) is not added to @(FileWrites), it is added to @(FileWritesShareable). If a .dll is copied to output because it was in @(ReferenceCopyLocalPaths), it is not added to B.csproj.FileListAbsolute.txt.

In our repro, The File was added to FileListAbsolute.txt during step 2, because the None items added it to @(FileWrites). In step 5, the file is still copied by _CopyFilesMarkedCopyLocal, but since it's no longer in @(FileWrites), but it is in @(_CleanPriorFileWrites), IncrementalClean deletes it. In step 8 The File is copied back by _CopyFilesMarkedCopyLocal, but since it's neither in current nor prior file writes, IncrementalClean doesn't delete it.

Basically I'd say it's an unfortunate confluence of the output directory being outside the project cone and FileWritesShareable not being added to FileWrites. I think this is the problematic logic that assumes the bin folder is inside the project directory:

<!--
Of shareable files, keep only those that are in the project's directory.
We never clean shareable files outside of the project directory because
the build may be to a common output directory and other projects may need
them.
Only subtract the outputs from ResolveAssemblyReferences target because that's the
only "Resolve" target that tries to resolve assemblies directly from the output
directory.
-->
<FindUnderPath Path="$(MSBuildProjectDirectory)" Files="@(FileWritesShareable)" UpdateToAbsolutePaths="true">
<Output TaskParameter="InPath" ItemName="FileWrites"/>
</FindUnderPath>

I doubt there's a safe fix that we can do to fix the bug but I'm filing it just the same.

@jzabroski
Copy link

jzabroski commented Nov 22, 2024

What is the recommended workaround? / @KirillOsenkov How do you workaround this bug? I think I am hitting this in an indirect way due to how Microsoft.Data.SqlClient.SNI imports native files in a net462 context. Technically, this is the guidance recommended by Microsoft. See https://learn.microsoft.com/en-us/nuget/create-packages/native-files-in-net-packages#projects-targeting-net-framework

I think the generalization is you don't need "If a project has its output directory outside the project directory" to reproduce this problem. Any clean checkout that transitively imports targets from a nuget package under buildTransitive\<any netfx tfm>\<PackageName>.targets will have this problem, I think.

I've tested:

  • indirect references where project A ships A.exe and contains project B reference shipping B.dll and a reference to Microsoft.Data.SqlClient.SNI
  • direct references where project A ships A.exe and contains direct reference to Microsoft.Data.SqlClient.SNI

@KirillOsenkov
Copy link
Member Author

I just add <Target Name="IncrementalClean" /> to my Directory.Build.targets to neuter the in the box one.

If I need to clean I just git clean -xdf

@KirillOsenkov
Copy link
Member Author

and yes, Microsoft.Data.SqlClient.SNI is a poorly authored NuGet package, might consider setting ExcludeAssets="all" PrivateAssets="all" GeneratePathProperty="true" and manually consuming the parts you need

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants