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

[Bug] System.Data warnings for .NET 8 WPF assemblies #9459

Closed
rohahn opened this issue Nov 23, 2023 · 11 comments · Fixed by #9460 or #10303
Closed

[Bug] System.Data warnings for .NET 8 WPF assemblies #9459

rohahn opened this issue Nov 23, 2023 · 11 comments · Fixed by #9460 or #10303
Labels
bug A bug to fix

Comments

@rohahn
Copy link

rohahn commented Nov 23, 2023

Describe the bug
Running docfx metadata on a .NET 8 WPF DLL generates a bunch of wrong warnings.
Since we use --warningsAsErrors, this is a blocker for us. :(

The transitive dependency DirectWriteForwarder.dll references assemblies from .NET 8 and .NET Framework. UniversalAssemblyResolver cannot handle this.

To Reproduce

  1. Create a simple WPF assembly with
<TargetFramework>net8.0-windows</TargetFramework>
<UseWPF>true</UseWPF>
  1. Configure docfx.json to process assemblies (the problem does not occur when processing project files)
{
  "metadata": [
    {
      "src": [
        {
          "files": ["artifacts/assemblies/*.dll"],
          "src": "./"
        }
      ],
      "dest": "api"
    }
  ]
}
  1. Execute dotnet docfx metadata .\docfx.json

This results in the following warnings:

dotnet docfx metadata .\docfx.json
                                                                           Using .NET Core SDK 8.0.100
Loading assembly C:/Users/hahn/Projects/DocFxTest/artifacts/assemblies/DocFxWarningTest.dll
warning: Unable to resolve assembly reference System.Data.SqlClient, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
warning: Unable to resolve assembly reference System.Data.Odbc, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.Data.OleDb, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.IO.Ports, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.Runtime.Serialization.Schema, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
warning: Unable to resolve assembly reference System.ServiceModel.Syndication, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.ServiceProcess.ServiceController, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
Processing DocFxWarningTest
Creating output...


Build succeeded with warning.

    7 warning(s)
    0 error(s)

Expected behavior
There should be no warnings.

Context (please complete the following information):

  • OS: Windows 11
  • Docfx version: 2.74.0+0a19218b660d592013a1d75479f013d69a6c5d71

Additional context
I ran docfx in the debugger and was able to see where the dependency is coming from:

PresentationCore -> DirectWriteForwarder -> System.Data -> System.Data.SqlClient

DirectWriteForwarder is referencing System.Data from .NET Framework (C:\Windows\Microsoft.NET\Framework64\v4.0.30319\System.Data.dll).

image

UniversalAssemblyResolver.FindAssemblyFile in CompilationHelper is resolving C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.0\System.Data.dll instead, which is a reference assembly that only contains forwarded types.

image

Since these references with 0.0.0.0 version cannot be resolved, the warnings are generated.

@rohahn rohahn added the bug A bug to fix label Nov 23, 2023
@yufeih
Copy link
Contributor

yufeih commented Nov 23, 2023

Same error from ILSpy:

image

@rohahn
Copy link
Author

rohahn commented Nov 23, 2023

I also checked ILSpy and it seems the problem is not new with .NET 8.

image

ILSpy is using the wrong path.

We were previously using .NET 6 and Docfx 2.59.4.0 without the warnings.
When upgrading to .NET 8, we also tried to upgrade to the latest Docfx.

So it seems recent changes in CompilationHelper must have led to the problem being uncovered.

@bitbonk
Copy link
Contributor

bitbonk commented Nov 23, 2023

We have the same problem since we upgraded docfx to .NET 8.

We generally want to build the docs warning-free to validate that we don't have any missing links or other errors in the docs, that's also why we turned on WarningsAsErrors.
Since in docfx, there doesn't seem to be a way to ignore warnings individually, we currently can only ignore all warnings which defeats the purpose of validation.

@rohahn
Copy link
Author

rohahn commented Nov 23, 2023

I also opened an issue for ILSpy.

@yufeih yufeih reopened this Nov 24, 2023
@Herrmel
Copy link

Herrmel commented Mar 12, 2024

I have the same problem with net6 assemblies but in my case I sometimes even get the following error that prevents my build:

error: (1,7): error CS0518: Predefined type 'System.Object' is not defined or imported
error: (3,29): error CS0518: Predefined type 'System.String' is not defined or imported
error: (3,19): error CS0518: Predefined type 'System.Void' is not defined or imported

Since ILSpy does not seem to do anything about it I tested some stuff and noticed ILSpy is able to resolve the previously unresolvable assembly after I opended it in ILSpy before I (re)load my assembly. Maybe it is possible to work around this issue in docfx aswell if we could add the framework-dlls via references in docfx.json and load them in the ILSpy-reflection beforehand? Does docfx currently use the references configuration in assembly reflection at all?

@siegfriedpammer
Copy link

ILSpy is using the wrong path.

No, it's not. The System.Data.dll located in the .NET 6.0 SDK directory has the correct version number (which is 4.0.0.0). Why should ILSpy fall back to a possibly outdated/incompatible version from classic .NET?

See also my comment in the ILSpy repository: icsharpcode/ILSpy#3128 (comment)

@siegfriedpammer
Copy link

siegfriedpammer commented Oct 19, 2024

UniversalAssemblyResolver.FindAssemblyFile in CompilationHelper is resolving C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.0\System.Data.dll instead, which is a reference assembly that only contains forwarded types.

It is NOT a reference assembly. It just exists for backwards compatibility (like mscorlib.dll and netstandard.dll). Reference assemblies are assemblies that contain all externally visible type definitions, but no IL (just a dummy throw null; statement in the body).

Since ILSpy does not seem to do anything about it

@Herrmel I tested this and this stuff works in ILSpy. Since we didn't even know that docfx is using our assembly resolver and we don't know what their expectations are it's hard for us to provide a solution. I would be glad to provide support, if the maintainers of this project got in touch with us and shared some more info.

One thing I noticed:

warning: Unable to resolve assembly reference System.Data.SqlClient, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
warning: Unable to resolve assembly reference System.Data.Odbc, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.Data.OleDb, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.IO.Ports, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.Runtime.Serialization.Schema, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
warning: Unable to resolve assembly reference System.ServiceModel.Syndication, Version=0.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
warning: Unable to resolve assembly reference System.ServiceProcess.ServiceController, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a

These assemblies are not part of the default .NET 8.0 Windows Desktop framework. If you want to use them, you need to add references to nuget packages. For ILSpy it is impossible to detect whether a reference originated from a nuget package, because there is just a normal reference in the binary. However, there should be a .deps.json file next to the DLL, which is understood by UniversalAssemblyResolver (or rather DotNetCorePathFinder) if it is configured correctly - then it should be able to find references from nuget packages.

I can help you debug this, if somebody provides setup and reproduction steps with docfx. Thank you very much!

@siegfriedpammer
Copy link

siegfriedpammer commented Oct 19, 2024

I had a quick look at the code in CompilationHelper and I noticed, that in CreateCompilationFromAssembly you create a CSharpCompilation without any syntax trees but just references and even the "main assembly" added as a reference. The problem here is that from my experience with Roslyn (of course, someone from the C# compiler/Roslyn team would have to confirm this), it doesn't go well, if you add not only the directly used references, but also transitive references to a compilation, especially with the MetadataImportOptions.All option. Last time I tried this at our company, I got lots of strange compiler errors for some projects.

Also, can someone explain what the DotnetApiCatalog.Compile implementation is used for? What results are expected from UniversalAssemblyResolver? Thanks!

Also keep in mind that ILSpy and the decompiler engine, for which UniversalAssemblyResolver was created, are designed to work even if some references cannot be resolved. It will add extra casts in the decompiled code and may not be able to detect all patterns, but still do its best. So, if you are using UniversalAssemblyResolver, you should not expect it to deliver flawless results in all cases. Roslyn on the other hand expects you to provide all necessary information and produces an error if you fail to do so. I believe that ILSpy is not exactly the right tool for this job, but instead the dotnet/docfx people should talk to the dotnet/runtime people and ask them, to provide an assembly resolver that "does what the runtime does".

@filzrev
Copy link
Contributor

filzrev commented Oct 19, 2024

I've confirmed reported issue can be reproduced with following steps.

  1. Install docfx as .NET Global Tools with following command

    dotnet tool install docfx -g

  2. Create work directory.
  3. Initialize docfx project with following command.

    docfx init --yes

  4. Edit docfx.json and replace metadata section with following content.
    "metadata": [
      {
        "src": [
          {
            "src": "./",
            "files": [
              "dlls/*.dll"
            ]
          }
        ],
        "dest": "api"
      }
    ],
    
  5. Copy WPF application dll to dlls directory.
  6. Run docfx metadata command

@yufeih
Copy link
Contributor

yufeih commented Oct 20, 2024

Despite disguising themselves as 4.0 version assemblies, these assemblies usually contain type forwarders and sometimes new types which are not available in older frameworks. The version number is actually wrong/misleading, but that's the mess we're living in for the sake of eternal backwards compatibility. The assembly resolving logic in ILSpy and the decompiler engine has a lot of special cases and hacks because of this mess.

True. This is the mess to live with and I'll exclude the warning from docfx side. Thank you @siegfriedpammer for looking into it.

@Herrmel
Copy link

Herrmel commented Oct 21, 2024

@Herrmel I tested this and this stuff works in ILSpy. Since we didn't even know that docfx is using our assembly resolver and we don't know what their expectations are it's hard for us to provide a solution. I would be glad to provide support, if the maintainers of this project got in touch with us and shared some more info.

I actually had a different Problem wich is now fixed in ILSpy. In my case for whatever reason .net left behind empty Version folders. IlSpy tried to use these older and empty version folders to find the referenced assemblies and its problematic to use a compilation without the core library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug to fix
Projects
None yet
6 participants