-
Notifications
You must be signed in to change notification settings - Fork 2
Fix plugin dependency loading by using LoadFromAssemblyPath and modernize PluginLoadContext #37
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
Conversation
|
@copilot note this comment added to the issue: #34 (comment) |
…nize PluginLoadContext Co-authored-by: matt-goldman <19944129+matt-goldman@users.noreply.github.com>
I've already addressed all three concerns you raised in commit 874412e:
The fixes ensure that plugins with NuGet dependencies can properly load their dependencies through the |
matt-goldman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a critical plugin dependency loading issue in Blake's plugin system by changing from LoadFromAssemblyName to LoadFromAssemblyPath and modernizes the plugin loading infrastructure. The fix ensures that plugins requiring external NuGet packages can properly resolve and load their dependencies at runtime.
- Fix plugin dependency resolution by using
LoadFromAssemblyPathinstead of bypassing the custom load context - Modernize
PluginLoadContextto use primary constructor syntax - Improve test reliability by switching from
Newtonsoft.JsontoSixLabors.ImageSharpdependency
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Blake.BuildTools/Utils/PluginLoader.cs |
Changed plugin loading method to properly trigger dependency resolution |
src/Blake.BuildTools/Utils/PluginLoadContext.cs |
Modernized to use primary constructor and field initialization |
tests/Blake.IntegrationTests/TestPluginWithDependencies/BlakePlugin.TestPluginWithDependencies.csproj |
Updated test dependency from Newtonsoft.Json to SixLabors.ImageSharp |
tests/Blake.IntegrationTests/TestPluginWithDependencies/TestPluginWithDependencies.cs |
Replaced JSON serialization with image processing to test dependency loading |
The plugin loader was not correctly loading plugin dependencies, causing plugins that rely on additional NuGet packages to fail at runtime. The issue was in the
LoadPluginDLLsmethod which was usingLoadFromAssemblyNameinstead of properly triggering the dependency resolution mechanism.Root Cause
The
PluginLoadContextclass has aLoadoverride method that usesAssemblyDependencyResolverto resolve plugin dependencies:However,
LoadPluginDLLswas callingloadContext.LoadFromAssemblyName(assemblyName)which bypasses this override method entirely, preventing dependency resolution from working.Changes Made
Fixed Plugin Loading Method: Changed from:
to:
This ensures the
Loadoverride is properly invoked for dependency resolution.Modernized PluginLoadContext: Updated to use primary constructor syntax for cleaner, more modern C# code.
Improved Test Coverage: Replaced the test dependency from
Newtonsoft.Json(which may already be loaded in test context) toSixLabors.ImageSharp, providing a more reliable test of the dependency loading functionality.Verification
.deps.jsonfileThis fix allows plugins to use specialized libraries like image processing, indexing, or other NuGet packages without encountering assembly loading failures.
Fixes #34.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
aka.mscurl -I -sSL --retry 5 --retry-delay 2 --connect-timeout 15 REDACTED(dns block)If you need me to access, download, or install something from one of these locations, you can either:
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.