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

Up-to-date check awareness of reference assemblies #2254

Closed
rainersigwald opened this issue May 22, 2017 · 21 comments · Fixed by #2414
Closed

Up-to-date check awareness of reference assemblies #2254

rainersigwald opened this issue May 22, 2017 · 21 comments · Fixed by #2414
Milestone

Comments

@rainersigwald
Copy link
Member

The C# and VB compilers introduced “reference assemblies”, a new compiler output that contains only the public interface of an assembly, and that does not change when private implementation details are changed: dotnet/roslyn#2184. This allows projects to avoid compile time when the output of recompilation would be identical, because no local source files have changed and the interface of referenced assemblies is identical.

This presents new scenarios for the fast up-to-date check, because

  • The set of inputs to projects that use reference assemblies has expanded.
    • The new inputs are the implementation assemblies of references. The reference assemblies are passed to the compiler and thus discovered by the existing design-time build.
  • Some assumptions of the FUTD check no longer hold.
    • Specifically, if a transitive dependency has its implementation updated, that no longer ensures that the direct dependency referencing it is updated.

Scenarios

Several new scenarios arise in a solution that uses reference assemblies.

For illustrative purposes, consider a solution consisting of Client.csproj that depends on Lib1.csproj that depends on Lib2.csproj. In this way Client is "downstream" of Lib1 and both Lib1 and Client are downstream of Lib2.

  • Change the interface of Lib2, debug Client.
    • Expected behavior: The compiler is run for Lib1 and Lib2; an updated copy of Lib2.dll is copied to the output folder of Client and used at debug time.
  • Change the implementation but not the interface of Lib2, debug Client.
    • Expected behavior: an updated copy of Lib2.dll is copied to the output folder of Client and used at debug time. The compiler is not run for Lib1 or Client.
  • Change nothing, debug Client.
    • Expected behavior: MSBuild is not invoked, the extant copy of Client is used at debug time.

Proposed design

To account for the transitive-dependency-copying behavior, I propose extending the up-to-date check with an additional set of inputs and outputs.

The MSBuild item that represents the output of a project is annotated with metadata for ReferenceAssembly and a new concept, the CopyUpToDateMarker. This is done in dotnet/msbuild#2039.

The CopyUpToDateMarker for a project is touched when references are copied to its output folder. This can then indicate that downstream projects need to build in order to copy the references along.

The timestamp of references’ CopyUpToDateMarkers must be compared only to the current project's CopyUpToDateMarker, because copying references will not update the primary output of the project.

Let Inputs = sources + references.select( impl and ref assemblies )
Let CopyMarkerInputs = references.select( copy markers )
Let Outputs = dll + pdb + xml
if oldest(Outputs) older than any Inputs:
    build(reason: compiler output change expected)
else if CurrentCopyMarker older than any CopyMarkerInputs:
    build(reason: copied reference update expected)
else
    skip

image

Alternatives considered

  • Use the UpToDateCheckInput item to avoid having detailed knowledge of reference assemblies in the project system.
    • The value of the item is used after the project is evaluated, but no targets are run, so it's impossible to dynamically discover the outputs of a ProjectReference to populate the item in time.
  • Discover the complete closure of assemblies that will be copied to the project's output, for example by passing FindDependencies=true to ResolveAssemblyReference within a design-time build.
    • This is currently explicitly disabled for performance reasons—doing this discovery as RAR does requires recursively loading and analyzing every assembly referenced by each reference, and doing a fair amount of path probing to find the location of each.
    • Without more sophisticated input:output correlation, this is susceptible to the subsequent-builds-not-up-to-date problem described next.
  • Add the copy marker file of referenced projects to the standard set of inputs and do not update the FUTD algorithm.
    • This works for the first build after a dependency's reference-only update. But because the primary output of the project wouldn't change (its reference inputs haven't changed), all subsequent checks would continue to require building. That would invalidate the fast up-to-date-check in many scenarios.
  • Updating the timestamp of the primary output assembly when any references are copied to its output folder.
    • Would result in copy cascades of identical-but-newer files in subsequent builds without the FUTD check (command line builds).

This is the result of extensive offline discussions with @jcouv, @tmeschter, @panopticoncentral, @davkean, and others.

@srivatsn srivatsn added the Bug label May 26, 2017
@srivatsn srivatsn added this to the 15.6 milestone May 26, 2017
@srivatsn srivatsn modified the milestones: 15.3, 15.6 Jun 2, 2017
@panopticoncentral
Copy link
Contributor

@rainersigwald Basic question: how/where does MSBuild do up-to-date checking on ref assemblies? It looks like when I build a project that outputs a ref assembly and have only changed the implementation, csc will still output a new ref assembly. So the "last write" on the ref assembly will be updated and would normally trigger an "up to date" check failure. But MSBuild correctly does not rebuild a dependent project, so it must look at more than just the timestamp. Can you point me to the logic it uses?

@jcouv
Copy link
Member

jcouv commented Jun 3, 2017

@panopticoncentral Yes, csc.exe will always write its outputs: obj/a.dll and obj/ref/a.dll.
There is another task (CopyRefAssembly, provided by the same assembly that hosts the Csc and Vbc tasks) which is used to copy obj/ref/a.dll to bin/ref/a.dll, except if the contents are determined to match (same MVID). In that case, bin/ref/a.dll keeps its old timestamp.

@jcouv
Copy link
Member

jcouv commented Jun 6, 2017

Relates to #62

rainersigwald pushed a commit to dotnet/msbuild that referenced this issue Jun 9, 2017
Required for dotnet/project-system#2254.

Given:

ClassLibrary1 -> ClassLibrary2 -> ConsoleApp1

Where ClassLibrary2 references ClassLibrary1 and ConsoleApp1 references only ClassLibrary2. And both ClassLibrary1 and ClassLibrary2 output ref assemblies.

So we do a full build, touch the implementation of ClassLibrary1, and then build.

- ClassLibrary1 is out of date, so it builds ClassLibrary1.dll. However, since the interface didn't change, ref\ClassLibrary1.dll is not touched.
- ClassLibrary2 is out of date because input ClassLibrary1.dll is now newer than output ClassLibrary2.dll. So it builds. However, since all inputs of CoreCompile are up to date (since ref\ClassLibrary1.dll isn't newer than ClassLibrary2.dll), we don't rebuild ClassLibrary2.dll or ref\ClassLibrary2.dll. We just copy the updated implementation of ClassLibrary1, and touch ClassLibrary2.marker.
- ConsoleApp1 is out of date because input ClassLibrary2.marker is now newer than output ConsoleApp1.dll. So it builds. However, since all inputs of CoreCompile are up to date (since ref\ClassLibrary2.dll isn't newer than ConsoleApp1.dll), we don't rebuild ConsoleApp1.dll. We just copy the updated implementation of ClassLibrary1. There's no marker for ConsoleApp1.

The problem in this situation is that now if you do another build, ConsoleApp1 will fail the up to date check and copy ref\ClassLibrary2.dll again, because ClassLibrary2.marker will always be newer than ConsoleApp1.dll. If ConsoleApp1 had a marker, though, then we would just compare the markers and know it was up to date.
@panopticoncentral
Copy link
Contributor

@jcouv What do you need from me to test this? I've done testing on my own, but a doublecheck would be good.

@jcouv
Copy link
Member

jcouv commented Jun 16, 2017

I'm enlisting to project-systems and I will try to build and set it up locally. I'll let you know how it goes. Thanks

@jcouv
Copy link
Member

jcouv commented Jun 16, 2017

Running into some problems with build.cmd when buildinig VSIX. Any tips?

  Building NuGet packages [Debug]
  Microsoft.VisualStudio.Editors\Microsoft.VisualStudio.Editors.nuspec -> D:\repos\project-system\bin\Debug\NuGetPackages\Microsoft.VisualStudio.Editors.1.0.0-beta2.nupkg
  Microsoft.VisualStudio.AppDesigner\Microsoft.VisualStudio.AppDesigner.nuspec -> D:\repos\project-system\bin\Debug\NuGetPackages\Microsoft.VisualStudio.AppDesigner.1.0.0-beta2.nupkg
  Microsoft.VisualStudio.ProjectSystem.Managed\Microsoft.VisualStudio.ProjectSystem.Managed.nuspec -> D:\repos\project-system\bin\Debug\NuGetPackages\Microsoft.VisualStudio.ProjectSystem.Managed.1.0.0-beta2.nupkg
  Microsoft.VisualStudio.ProjectSystem.Managed.VS\Microsoft.VisualStudio.ProjectSystem.Managed.VS.nuspec -> D:\repos\project-system\bin\Debug\NuGetPackages\Microsoft.VisualStudio.ProjectSystem.Managed.VS.1.0.0-beta2.nupkg

  Rebuilding Modern VSIX Packages [Debug]
C:\Users\jcouv\.nuget\packages\microbuild.plugins.swixbuild\1.0.24\build\Microsoft.VisualStudio.Setup.Tools.targets(84,3): error MSB4011: "C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\bin\Microsoft.Comm
on.targets" cannot be imported again. It was already imported at "C:\Users\jcouv\.nuget\packages\microbuild.plugins.swixbuild\1.0.101\build\Microsoft.VisualStudio.Setup.Tools.targets (106,3)". This is most likely a build authoring error
. This subsequent import will be ignored. [D:\repos\project-system\src\VsixV3\EditorsPackage\Microsoft.VisualStudio.Editors.vsmanproj]
C:\Users\jcouv\.nuget\packages\microbuild.plugins.swixbuild\1.0.71\build\Microsoft.VisualStudio.Setup.Tools.targets(105,3): error MSB4011: "C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\bin\Microsoft.Com
mon.targets" cannot be imported again. It was already imported at "C:\Users\jcouv\.nuget\packages\microbuild.plugins.swixbuild\1.0.101\build\Microsoft.VisualStudio.Setup.Tools.targets (106,3)". This is most likely a build authoring erro
r. This subsequent import will be ignored. [D:\repos\project-system\src\VsixV3\EditorsPackage\Microsoft.VisualStudio.Editors.vsmanproj]
C:\Users\jcouv\.nuget\packages\microbuild.plugins.swixbuild\1.1.0-g0701ee829f\build\Microsoft.VisualStudio.Setup.Tools.targets(106,3): error MSB4011: "C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\bin\Mi
crosoft.Common.targets" cannot be imported again. It was already imported at "C:\Users\jcouv\.nuget\packages\microbuild.plugins.swixbuild\1.0.101\build\Microsoft.VisualStudio.Setup.Tools.targets (106,3)". This is most likely a build aut
horing error. This subsequent import will be ignored. [D:\repos\project-system\src\VsixV3\EditorsPackage\Microsoft.VisualStudio.Editors.vsmanproj]
C:\Users\jcouv\.nuget\packages\microbuild.plugins.swixbuild\1.1.0-g0701ee829f\build\Microsoft.VisualStudio.Setup.Tools.targets(419,7): error MSB4064: The "ProductPatchVersion" parameter is not supported by the "FinalizeManifest" task. V
erify the parameter exists on the task, and it is a settable public instance property. [D:\repos\project-system\src\VsixV3\EditorsPackage\Microsoft.VisualStudio.Editors.vsmanproj]
C:\Users\jcouv\.nuget\packages\microbuild.plugins.swixbuild\1.1.0-g0701ee829f\build\Microsoft.VisualStudio.Setup.Tools.targets(404,5): error MSB4063: The "FinalizeManifest" task could not be initialized with its input parameters.  [D:\r
epos\project-system\src\VsixV3\EditorsPackage\Microsoft.VisualStudio.Editors.vsmanproj]
[...]

@davkean
Copy link
Member

davkean commented Jun 16, 2017

Delete the MicroBuild.* packages from your NuGet directory. They aren't able to run side-by-side. They are aware and have zero plans to fix this. :(

@jcouv
Copy link
Member

jcouv commented Jun 16, 2017

Thanks @davkean

When I do that, most VSIXs now seem to build, but I still get a failure on FSharp. I'll try to proceed, hoping that the bits I need have been deployed to RoslynDev.

  Microsoft.NetStandard.FSharp.ProjectTemplates.Vsix -> D:\repos\project-system\bin\Debug\Microsoft.NetStandard.FSharp.ProjectTemplates.vsix
C:\Users\jcouv\.nuget\packages\microsoft.vssdk.buildtools\15.0.26124-rc3\tools\VSSDK\Microsoft.VsSDK.targets(649,5): error MSB4018: The "FindInstalledExtension" task failed unexpectedly.\r [D:\repos\project-system\src\Templates\Microsof
t.NetStandard.FSharp.ProjectTemplates.Vsix\Microsoft.NetStandard.FSharp.ProjectTemplates.Vsix.csproj]
C:\Users\jcouv\.nuget\packages\microsoft.vssdk.buildtools\15.0.26124-rc3\tools\VSSDK\Microsoft.VsSDK.targets(649,5): error MSB4018: System.AggregateException: One or more errors occurred. ---> System.NullReferenceException: Object refer
ence not set to an instance of an object.\r [D:\repos\project-system\src\Templates\Microsoft.NetStandard.FSharp.ProjectTemplates.Vsix\Microsoft.NetStandard.FSharp.ProjectTemplates.Vsix.csproj]
C:\Users\jcouv\.nuget\packages\microsoft.vssdk.buildtools\15.0.26124-rc3\tools\VSSDK\Microsoft.VsSDK.targets(649,5): error MSB4018:    at Microsoft.VisualStudio.ExtensionManager.ExtensionEngineImpl.AddScannedFile(CachedInstalledExtensio
nList cachedExtensionsList, ExtensionLocations location, Boolean perMachine, String fullPath, DateTime lastWriteTime, IEnumerable`1 pkgdefs)\r [D:\repos\project-system\src\Templates\Microsoft.NetStandard.FSharp.ProjectTemplates.Vsix\Mic
rosoft.NetStandard.FSharp.ProjectTemplates.Vsix.csproj]
C:\Users\jcouv\.nuget\packages\microsoft.vssdk.buildtools\15.0.26124-rc3\tools\VSSDK\Microsoft.VsSDK.targets(649,5): error MSB4018:    at Microsoft.VisualStudio.ExtensionManager.ExtensionEngineImpl.ScanInstalledExtensions(CachedInstalle
dExtensionList cachedExtensionsList, ExtensionLocations location, IEnumerable`1 enabledExtensions, Boolean safeMode)\r [D:\repos\project-system\src\Templates\Microsoft.NetStandard.FSharp.ProjectTemplates.Vsix\Microsoft.NetStandard.FShar
p.ProjectTemplates.Vsix.csproj]

@davkean
Copy link
Member

davkean commented Jun 16, 2017

Never seen that before. Do you have the RoslynDev hive open in VS right now?

@jcouv
Copy link
Member

jcouv commented Jun 16, 2017

No, I have default hive open.

@jcouv
Copy link
Member

jcouv commented Jun 16, 2017

It worked after re-running. Not sure what happened.

@davkean
Copy link
Member

davkean commented Jun 16, 2017

The VSIX has lots of these races - they are looking into them.

@jcouv
Copy link
Member

jcouv commented Jun 16, 2017

@panopticoncentral I'm this PR is introducing some regressions, even with ref assemblies turned off.
Here's the scenario: I open the Compilers.sln from Roslyn, I make a comment change in WellKnownMembers.cs (part of CodeAnalysis project), then I build the CSharpCompilerEmitTest project.

The test project depends on CSharpCodeAnalysis, which depends on CodeAnalysis, so I expect CSharpCodeAnalysis to be re-compiled.
I confirmed that is the case on my vanilla install of VS (dev15.6 preview 2).
But on RoslynDev hive (with this PR deployed), it is not getting re-compiled, which seems wrong.

In the attached logs, the compiler (searching for "/out:") is invoked 9 times with regular VS bits, but only 5 times with RoslynDev hive. The compilations for Microsoft.CodeAnalysis.CSharp.dll and Microsoft.CodeAnalysis.VisualBasic.dll seem to be missing.

msbuild-log-without-refout-preview.txt
msbuild-log-without-refout-roslyndev.txt

@panopticoncentral
Copy link
Contributor

I will repro in the morning, but could you do Tools | Options | Projects and Solutions | .NET Core and change your logging level to Verbose? Then try building again and in the output window it will give you a rundown of what the up to date checker is thinking. If you could include that, it'd be great.

@panopticoncentral
Copy link
Contributor

I pushed a fix that was causing a problem with the up to date check, and I can't reproduce your problem, can you retry with the logging on?

@jcouv
Copy link
Member

jcouv commented Jun 16, 2017

So this morning I couldn't repro the problem again (even without the last iteration of your PR).
For context, what I was doing last night was turning ProduceReferenceAssembly on and off (without closing VS) to compare the build steps.

@jcouv
Copy link
Member

jcouv commented Jun 16, 2017

Ok, I repro'ed after all and I have diagnostic logs.
The pattern I followed was: (1) switch ProduceReferenceAssembly (on/off), (2) build the test project (it should re-build everything), (3) add comment in CodeAnalysis file, (4) build test project, then start over from (1) toggling the setting.

But this is prior to your latest fix. I will update my local setup and try again.

@panopticoncentral
Copy link
Contributor

I have to run out for a little while (last day of school for the kids), but something strange is going on--your log doesn't show any references at all coming up in the up to date check. I'm going to need to dig further this afternoon to figure out what's going on.

@panopticoncentral
Copy link
Contributor

@jcouv We figured out on Friday that this was an issue with how you were running the project system (you have to run from IDE). Do you have an update on how this works for you?

@jcouv
Copy link
Member

jcouv commented Jun 19, 2017

@panopticoncentral Thanks for the ping. I lost track.
Now I remember: I need to deploy the project system from Visual Studio and not simply running build.cmd. I'll resume testing this afternoon.

@drewnoakes
Copy link
Member

A work item tracking adding this for CSPROJ is available at:

@drewnoakes drewnoakes removed the Bug label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants