-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Plugin types not resolving #145
Comments
Update: I have been poking around the code in the master branch and have been doing some debugging locally. I found the cause of the issue with loading the Api library. I put the plugin in its own nuget package, which means its dependencies have to be resolved by nuget, which doesn't happen when dynamically loading the plugin so the Api library can't be found. The only way to make this work is to copy the Api library to the output directory when packing the plugin, which doesn't happen by default. To fix this, I made the following changes to the plugin project: <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<DevelopmentDependency>true</DevelopmentDependency>
<SuppressDependenciesWhenPacking>true</SuppressDependenciesWhenPacking>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="DefaultDocumentation.Api" Version="0.8.2" PrivateAssets="all" GeneratePathProperty="true" />
</ItemGroup>
<ItemGroup>
<None Include="DefaultDocumentation.PluginExample.targets" Pack="true" PackagePath="build\" />
<None Include="$(PkgDefaultDocumentation_Api)\lib\netstandard2.0\DefaultDocumentation.Api.dll" Pack="true" PackagePath="lib\netstandard2.0\" />
</ItemGroup>
</Project> I will add this to a PR later to help others who run into this issue when trying to adopt the PluginExample. Once I fixed the issue with the Api library, I ran right into the other issue where it can't find the FolderFileNameFactory. I haven't figured that out yet. It looks like issue #109 addresses something similar, but this is still happening to me. |
Update: I found a better solution that fixes both issues. The problem is that even if we solve the first issue by copying the DefaultDocumentation.Api assembly into the same directory, two assemblies will end up getting loaded, the Api nuget library referenced by the plugin and the Api project in the DefaultDocumentation solution. However, even though both assemblies have an interface called IFileNameFactory, it considers the IFileNameFactory in the Api project to be a different type than the one in the nuget library. Same with all the other types in DefaultDocumentation.Api. So, when it tries to locate all implementations of IFileNameFactory, it uses the IFileNameFactory in the Api project rather than the nuget library, so it doesn't find the plugin implementation. The best solution I could find is to assign our own method to the AppDomain.AssemblyResolve event. This event will get called when Assembly.LoadFrom fails to resolve an assembly or its dependencies, so we can try to locate the assembly manually instead of throwing an error. We can then search the assemblies that have already been loaded and return one that matches the full name, or return null if one has not been found. The idea is that the DefaultDocumentation.Api assembly and its dependencies have already been loaded by the DefaultDocumentation solution, and when they can't be found in the same directory as the plugin, instead use the ones that have already been loaded. That resolves both issues as it is now able to locate the assembly and it can find all the types because the DefaultDocumentation solution and the plugin are both referencing the same assembly. private Generator(Target loggerTarget, IRawSettings settings)
{
...
AppDomain.CurrentDomain.AssemblyResolve += OnResolve;
_context = new GeneralContext(
_configuration,
new[] { typeof(Markdown.Writers.MarkdownWriter).Assembly }
.Concat((GetSetting<string[]>(nameof(settings.Plugins)) ?? Enumerable.Empty<string>()).Select(Assembly.LoadFrom))
.SelectMany(a => a.GetTypes())
.ToArray(),
resolvedSettings,
DocItemReader.GetItems(resolvedSettings));
}
private Assembly OnResolve(Object sender, ResolveEventArgs e)
{
return Array.Find(AppDomain.CurrentDomain.GetAssemblies(), asm => asm.FullName == e.Name);
} The only problem with this solution is that it only works if the references on both sides, the DefaultDocumentation solution and the plugin, are the same. That means if the plugin has a package reference to DefaultDocumentation.Api version 0.8.2, then the DefaultDocumentation solution must also have a package reference to DefaultDocumentation.Api version 0.8.2. They both have to reference the nuget library with the same version. The only downside to this is that the Api version has to be manually updated if we change the Api package. I will add these fixes to a pull request later. Below is the documentation for the AssemblyResolve event for more information on this: https://learn.microsoft.com/en-us/dotnet/standard/assembly/resolve-loads |
Update: I was seeing if there was a less hacky solution than using a package reference for the Api library. After manually testing the packages locally, I found that the Api assembly being referenced could be different between the base project and the plugin as long as they have the same qualified name (the name, version, culture, and public key token all have to be the same). I thought the two assemblies had different public key tokens (the one in the base project was null and the one in the nuget library was not null) but that was not actually true. The public key token is generated because the Api library and the other DefaultDocumentation libraries are signed, something I did not know about until now. Turns out it was the one in the nuget library where the public key token was appearing as null in the debugger and the reason for this was my plugin library was unsigned and an unsigned assembly can't reference a signed one. I added the ProjectReference to the Api project back in as that was not the real reason for the assembly not being loaded. The solution, in addition to the code I already pushed in the PR, is to also sign the assembly for my plugin using my own snk file. Once it is signed, the correct public key token for the Api library will be exposed and it the plugin should work. I also feel like there is a gap in this project's documentation. I didn't know about signing NuGet libraries until I looked into this issue and it was not mentioned anywhere that the plugins had to be signed. I will update my PR with this info to the documentation and plugin example. |
Thank you @juliansangillo for studying this issue. We ran across this in our project and I would love for it to be fixed. The URL factory and file name factory don't work for our process. We want a folder structure with pretty filenames in a Azure DevOps wiki and reference things between multiple assemblies, which absolutely requires code changes or a plugin. But since plugins don't work because of this issue, we are stuck. I hope @Doraku will get to it eventually (Thanks for the great package!) |
Sorry for the long period of inactivity and thank you @juliansangillo for all this write up. I just started to get back into my projects and ran into the |
We ran into the same issue. Build works using MSBuild 17.10.4+10fbfbf2e from Visual Studio 2022 but does not work using dotnet build version 8.0.202. Even more, it does not work with MSBuild 17.10.4+10fbfbf2e from Rider 2024.1.4. I hope this will be fixed in the future. |
As a workaround, you can apply the fix @juliansangillo found in a build task that runs right before DefaultDocumentation. This way you don't have to fork DefaultDocumentation. <Target Name="FixDefaultDocumentationPluginLoading" BeforeTargets="DefaultDocumentation"
Condition="$(SkipDefaultDocumentationHack)!='true'">
<FixDefaultDocumentationPluginLoading/>
</Target>
<UsingTask TaskName="FixDefaultDocumentationPluginLoading" TaskFactory="RoslynCodeTaskFactory"
AssemblyFile="$(MSBuildToolsPath)\Microsoft.Build.Tasks.Core.dll">
<Task>
<Code Type="Fragment" Language="cs">
<![CDATA[
AppDomain.CurrentDomain.AssemblyResolve += (_, e) =>
Array.Find(AppDomain.CurrentDomain.GetAssemblies(), asm => asm.FullName == e.Name);
]]>
</Code>
</Task>
</UsingTask> |
So after a long hiatus I'm (really) back, Just did a new preview version after lot of cleanup before going back into this issue but I can't seem to be able to reproduce it anymore, documentation is generating fine, locally and on ci, even with an unsigned plugin so maybe it was an issue in donet 🤔 ? Sorry for asking after such a long time but do any of you still have the problem with the latest 0.9.0-beta01? |
Hi, I think my issue was only vaguely related. As I mentioned, I wanted to generate links into different assemblies and the plugin system was my attempt to do that. Since then, we actually stopped using DefaultDocumentation, I'm sorry to say. |
no worries, totally understandable 👍 |
I can still reproduce it with 0.9.0-beta02. It only happens when building with the "dotnet" toolchain but not when running msbuild (such as from within Visual Studio). I think this is because msbuild uses .NET Framework runtime and "dotnet" uses the new .NET, and there is a difference in how reflection assembly loading resolves dependencies or something like that. I have attached a simple repro you could take a look at. It has a simple plugin and a target project that should generate documentation and it has two batch files: "build-msbuild.bat" should work and "build-dotnet.bat" fails. |
Alright, I basically applied your fix here directly in the generator code, I also changed it so it's no longer version specific (so the versions of DefaultDocumentation and the plugins don't have to be exactly the same, I'm thinking of just producing a warning log instead 🤔 that way plugin creators won't have to always create a new release if I don't break any api). Thanks again for your help and sorry for the long delay! |
Hello. Thank you for this library. This is definitely the best library I found for generating markdown docs.
I am trying to create a plugin that can be installed from NuGet and the plugin will have my own implementation of a FolderFileNameFactory like the one in your plugin example. Unfortunately I am running into some errors when building the target project. First I tried with version 0.8.2 and received an error that DefaultDocumentation.Api could not be found even though it is referenced in the NuGet plugin.
Then I tried to downgrade to 0.8.1 and I ran into a different error that it couldn't find the FileNameFactory with name 'Folder' even though it does exist.
I get the same 'Folder' not found error in version 0.8.0 as well. I don't think I can downgrade further than that since plugins were only introduced in 0.8.0.
I looked a bit into the code myself and it seems reflection is used to get the available types in the Generator class and is past into GeneralContext and its parent, Context. It seems Context is what throws that error when it can't find the FileNameFactory, so it is likely that reflection isn't finding my FolderFileNameFactory and isn't adding it to the available types like it should be. Since it is an MSBuild Task I can't debug it so I can't test this though.
The text was updated successfully, but these errors were encountered: