dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.05k stars 4.04k forks source link

VS doesn't support MSBuild <CopyLocalLockFileAssemblies> property #61940

Open Eli-Black-Work opened 2 years ago

Eli-Black-Work commented 2 years ago

Version Used: VS 17.3.0 Preview 1.1

Steps to Reproduce:

  1. Create a solution that has two projects: MyApp and MyGenerator.

  2. Make MyGenerator depend on a NuGet package, such as Microsoft.OpenApi.Readers, and set <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies> in the source generator's .csproj file.

    <Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <TargetFramework>netstandard2.0</TargetFramework>
        <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
    </PropertyGroup>
    
    <ItemGroup>
        ...
        <PackageReference Include="Microsoft.OpenApi.Readers" Version="1.2.3" />
    </ItemGroup>
    </Project>
  3. Make MyApp use the source generator.

    <Project Sdk="Microsoft.NET.Sdk">
    
    <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    </PropertyGroup>
    
    <ItemGroup>
        <ProjectReference Include="..\MyGenerator\MyGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" SetTargetFramework="TargetFramework=netstandard2.0" />
    </ItemGroup>
    </Project>
  4. Build MyApp from the command line. Everything builds fine.

  5. Open VS. The generator fails to run, with this error:

    Generator 'ControllerGenerator' failed to generate source.
    It will not contribute to the output and compilation errors may occur as a result.
    Exception was of type 'FileNotFoundException' with message 'Could not load file or assembly 'Microsoft.OpenApi.Readers, Version=1.2.3.0, Culture=neutral, PublicKeyToken=3f5743946376f042' or one of its dependencies. The system cannot find the file specified.'

Here's an example project that reproduces the issue: https://github.com/Bosch-Eli-Black/source-generator-example

Expected Behavior:

VS supports <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>

Actual Behavior:

VS appears to not yet support <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>

Misc:

It would be super awesome if VS support the CopyLocalLockFileAssemblies property, because then we wouldn't need to manually copy NuGet packages πŸ™‚

Today, we need to do this:

<!-- See https://github.com/dotnet/roslyn-sdk/blob/0313c80ed950ac4f4eef11bb2e1c6d1009b328c4/samples/CSharp/SourceGenerators/SourceGeneratorSamples/SourceGeneratorSamples.csproj#L13-L30
and https://github.com/dotnet/roslyn/discussions/47517#discussioncomment-64145 -->
<PropertyGroup>
    <GetTargetPathDependsOn>$(GetTargetPathDependsOn);GetDependencyTargetPaths</GetTargetPathDependsOn>
</PropertyGroup>

<Target Name="GetDependencyTargetPaths">
    <!-- Manually include the DLL of each NuGet package that this analyzer uses. -->
    <ItemGroup>
        <TargetPathWithTargetPlatformMoniker Include="$(PKGGraphQL)\lib\netstandard2.0\GraphQL.dll" IncludeRuntimeDependency="false" />
    </ItemGroup>
</Target>
dotnet-issue-labeler[bot] commented 2 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

rainersigwald commented 2 years ago

@jmarolf this is a Roslyn issue, but I learned offline that it's deliberate: analyzer dependencies don't follow the usual .NET assembly load rules, in an effort to improve incremental build behavior. Without knowing that, in dotnet/msbuild#7640 we suggested setting the property that would make the usual load behavior work.

dotnet-issue-labeler[bot] commented 2 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

jmarolf commented 2 years ago

thanks for the context @rainersigwald let me transfer this back

dotnet-issue-labeler[bot] commented 2 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

Eli-Black-Work commented 2 years ago

Thanks, @jmarolf and @rainersigwald πŸ™‚

The main issue is that if my analyzer/source-generator depends on a NuGet, I'd like for VS to automatically include the NuGet's .dll's, so that I don't need to manually include them πŸ™‚

Today, we have to do this:

<ItemGroup>
    <PackageReference Include="GraphQL" Version="4.7.1" GeneratePathProperty="true" PrivateAssets="all" />
</ItemGroup>

<!-- See https://github.com/dotnet/roslyn-sdk/blob/0313c80ed950ac4f4eef11bb2e1c6d1009b328c4/samples/CSharp/SourceGenerators/SourceGeneratorSamples/SourceGeneratorSamples.csproj#L13-L30
and https://github.com/dotnet/roslyn/discussions/47517#discussioncomment-64145 -->
<PropertyGroup>
    <GetTargetPathDependsOn>$(GetTargetPathDependsOn);GetDependencyTargetPaths</GetTargetPathDependsOn>
</PropertyGroup>

<Target Name="GetDependencyTargetPaths">
    <!-- Manually include the DLL of each NuGet package that this analyzer uses. -->
    <ItemGroup>
        <TargetPathWithTargetPlatformMoniker Include="$(PKGGraphQL)\lib\netstandard2.0\GraphQL.dll" IncludeRuntimeDependency="false" />
    </ItemGroup>
</Target>

Ideally, this would be automatic, so that when my analyzer references a NuGet, it's .dll's automatically get included, so I just do this:

<ItemGroup>
    <PackageReference Include="GraphQL" Version="4.7.1" PrivateAssets="all" />
</ItemGroup>

I think this would be of great value, as it seems to be one of the primary stumbling blocks that developers new to source generators seem to encounter πŸ™‚ (See https://github.com/dotnet/roslyn/discussions/47517, https://github.com/dotnet/roslyn/issues/60256#issuecomment-1131364519, https://github.com/dotnet/roslyn/issues/57997#issuecomment-1024601830, https://github.com/dotnet/roslyn/issues/52017#issuecomment-809113013, https://github.com/dotnet/roslyn/issues/50148, https://github.com/dotnet/roslyn/issues/49075, etc.)

jmarolf commented 2 years ago

yep, I need to write and SDK piece that does all the project and nuget reference resolution logic. It's not trivial but I have had some success in early experiments. Unfortunately I don't have an ETA

Eli-Black-Work commented 2 years ago

@jmarolf Oh, yay! πŸ™‚

pjt33 commented 2 years ago

Today, we have to do this:

Even doing that didn't work for me: it seems that in the processing of the csproj file the change to GetTargetPathDependsOn gets overwritten. I've resorted to making the source generator hook AppDomain.AssemblyResolve and load the assembly from ~/.nuget/packages.

A proper fix would definitely be appreciated.

Eli-Black-Work commented 2 years ago

@pjt33 Can you elaborate? That method has been working well for us.

pjt33 commented 2 years ago

I haven't saved the build log and I can't remember the details now, but I've reapplied the changes above to the csproj and generated a fresh build log. I can't see the overwriting that I previously mentioned, so in that sense I can't elaborate, but what I now see is that

This reference is not "CopyLocal" because at least one source item had "Private" set to "false" and no source items had "Private" set to "true".

even though the only reference has PrivateAssets="All".

csproj and (lightly anonymised) build.log at https://gist.github.com/pjt33/59a65ceccf221a7ae74be8b7fc628dc7

alienriver49 commented 1 year ago

I'm also having to copy all the dependencies of the NuGet package I'm referencing as a dependency, is that expected or am I missing something? I'm trying to create an incremental source generator for NJsonSchema to generate C# classes from JSON schema files specified under AdditionalFiles. Here's my csproj for the generator:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
    <LangVersion>10.0</LangVersion>
    <ImplicitUsings>enable</ImplicitUsings>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
    <IsRoslynComponent>true</IsRoslynComponent>
    <AnalyzerLanguage>cs</AnalyzerLanguage>
    <IsPackable>false</IsPackable>
    <GetTargetPathDependsOn>$(GetTargetPathDependsOn);GetDependencyTargetPaths</GetTargetPathDependsOn>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.3.0" PrivateAssets="all"/>
    <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" PrivateAssets="all"/>
    <PackageReference Include="NJsonSchema" Version="10.9.0" GeneratePathProperty="true" PrivateAssets="all" />
    <PackageReference Include="NJsonSchema.CodeGeneration" Version="10.9.0" GeneratePathProperty="true" PrivateAssets="all" />
    <PackageReference Include="NJsonSchema.CodeGeneration.CSharp" Version="10.9.0" GeneratePathProperty="true" PrivateAssets="all" />
  </ItemGroup>

  <ItemGroup>
    <AdditionalFiles Include="AnalyzerReleases.Shipped.md" />
    <AdditionalFiles Include="AnalyzerReleases.Unshipped.md" />
  </ItemGroup>

  <Target Name="GetDependencyTargetPaths">
    <ItemGroup>
      <TargetPathWithTargetPlatformMoniker Include="$(PKGNJsonSchema)\lib\netstandard2.0\NJsonSchema.dll" IncludeRuntimeDependency="false" />
      <TargetPathWithTargetPlatformMoniker Include="$(PKGNJsonSchema_CodeGeneration)\lib\netstandard2.0\NJsonSchema.CodeGeneration.dll" IncludeRuntimeDependency="false" />
      <TargetPathWithTargetPlatformMoniker Include="$(PKGNJsonSchema_CodeGeneration_CSharp)\lib\netstandard2.0\NJsonSchema.CodeGeneration.CSharp.dll" IncludeRuntimeDependency="false" />
    </ItemGroup>
  </Target>

</Project>

As soon as it goes to use NJsonSchema in the generator, it fails on finding Namotion.Reflection, a dependency of NJsonSchema.

Eli-Black-Work commented 1 year ago

@alienriver49 Yes, you currently need to include all dependencies, so in your case, you'll need to include Namotion.Reflection and any of its dependencies.