-
Notifications
You must be signed in to change notification settings - Fork 2
Optimize PluginLoader to check DLLs before running dotnet restore #52
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,9 @@ | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| namespace Blake.BuildTools.Utils; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| internal record NuGetPluginInfo(string PackageName, string Version, string DllPath); | ||||||||||||||||||||||||||||
| internal record ProjectRefPluginInfo(string ProjectPath, string DllPath); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| internal static class PluginLoader | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| internal static List<PluginContext> LoadPlugins(string directory, string config, ILogger? logger) | ||||||||||||||||||||||||||||
|
|
@@ -21,7 +24,7 @@ internal static List<PluginContext> LoadPlugins(string directory, string config, | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (csprojFile == null) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger.LogWarning("No .csproj file found in the specified directory."); | ||||||||||||||||||||||||||||
| logger?.LogWarning("No .csproj file found in the specified directory."); | ||||||||||||||||||||||||||||
| return plugins; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
@@ -32,46 +35,62 @@ internal static List<PluginContext> LoadPlugins(string directory, string config, | |||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| catch (Exception ex) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger.LogError("Error loading .csproj file: {message}, {error}", ex.Message, ex); | ||||||||||||||||||||||||||||
| logger?.LogError("Error loading .csproj file: {message}, {error}", ex.Message, ex); | ||||||||||||||||||||||||||||
| return plugins; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| var fullCsprojPath = Path.GetFullPath(csprojFile); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // ensure dotnet restore has been run | ||||||||||||||||||||||||||||
| Process process = new() | ||||||||||||||||||||||||||||
| // Get plugin information first | ||||||||||||||||||||||||||||
| var nugetPlugins = GetNuGetPluginInfo(doc); | ||||||||||||||||||||||||||||
| var projectPlugins = GetProjectRefPluginInfo(doc, directory, config); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Check if all plugins are already valid (DLLs exist and have correct versions) | ||||||||||||||||||||||||||||
| bool needsRestore = !AreAllPluginsValid(nugetPlugins, projectPlugins, logger); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (needsRestore) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| StartInfo = new ProcessStartInfo | ||||||||||||||||||||||||||||
| logger?.LogDebug("Some plugin DLLs are missing or outdated, running dotnet restore..."); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // ensure dotnet restore has been run | ||||||||||||||||||||||||||||
| Process process = new() | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| FileName = "dotnet", | ||||||||||||||||||||||||||||
| Arguments = $"restore {fullCsprojPath}", | ||||||||||||||||||||||||||||
| WorkingDirectory = directory, | ||||||||||||||||||||||||||||
| RedirectStandardOutput = true, | ||||||||||||||||||||||||||||
| RedirectStandardError = true, | ||||||||||||||||||||||||||||
| UseShellExecute = false, | ||||||||||||||||||||||||||||
| CreateNoWindow = true | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
| StartInfo = new ProcessStartInfo | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| FileName = "dotnet", | ||||||||||||||||||||||||||||
| Arguments = $"restore {fullCsprojPath}", | ||||||||||||||||||||||||||||
| WorkingDirectory = directory, | ||||||||||||||||||||||||||||
| RedirectStandardOutput = true, | ||||||||||||||||||||||||||||
| RedirectStandardError = true, | ||||||||||||||||||||||||||||
| UseShellExecute = false, | ||||||||||||||||||||||||||||
| CreateNoWindow = true | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| process.Start(); | ||||||||||||||||||||||||||||
| process.Start(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| process.WaitForExit(); | ||||||||||||||||||||||||||||
| process.WaitForExit(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (process.ExitCode != 0) | ||||||||||||||||||||||||||||
| if (process.ExitCode != 0) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger?.LogError("dotnet restore failed with exit code {exitCode}.", process.ExitCode); | ||||||||||||||||||||||||||||
| logger?.LogDebug(process.StandardOutput.ReadToEnd()); | ||||||||||||||||||||||||||||
| logger?.LogError(process.StandardError.ReadToEnd()); | ||||||||||||||||||||||||||||
| return plugins; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger.LogError("dotnet restore failed with exit code {exitCode}.", process.ExitCode); | ||||||||||||||||||||||||||||
| logger.LogDebug(process.StandardOutput.ReadToEnd()); | ||||||||||||||||||||||||||||
| logger.LogError(process.StandardError.ReadToEnd()); | ||||||||||||||||||||||||||||
| return plugins; | ||||||||||||||||||||||||||||
| logger?.LogDebug("All plugin DLLs are present and valid, skipping dotnet restore."); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| LoadNuGetPlugins(doc, plugins, logger); | ||||||||||||||||||||||||||||
| LoadProjectRefPlugins(doc, directory, config, plugins, logger); | ||||||||||||||||||||||||||||
| LoadNuGetPlugins(nugetPlugins, plugins, logger); | ||||||||||||||||||||||||||||
| LoadProjectRefPlugins(projectPlugins, plugins, config, logger); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return plugins; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| private static void LoadNuGetPlugins(XDocument project, List<PluginContext> plugins, ILogger? logger) | ||||||||||||||||||||||||||||
| private static List<NuGetPluginInfo> GetNuGetPluginInfo(XDocument project) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| // Find the target framework dynamically from the .csproj file | ||||||||||||||||||||||||||||
| var targetFramework = project.Descendants("TargetFramework") | ||||||||||||||||||||||||||||
|
|
@@ -81,7 +100,7 @@ private static void LoadNuGetPlugins(XDocument project, List<PluginContext> plug | |||||||||||||||||||||||||||
| var userHomeDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); | ||||||||||||||||||||||||||||
| var globalPackagesFolder = Path.Combine(userHomeDirectory, ".nuget", "packages"); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| var pluginFiles = project.Descendants("PackageReference") | ||||||||||||||||||||||||||||
| return project.Descendants("PackageReference") | ||||||||||||||||||||||||||||
| .Where(p => p.Attribute("Include")?.Value.StartsWith("BlakePlugin.", StringComparison.OrdinalIgnoreCase) == true) | ||||||||||||||||||||||||||||
| .Where(p => !string.IsNullOrWhiteSpace(p.Attribute("Version")?.Value)) | ||||||||||||||||||||||||||||
| .Select(p => | ||||||||||||||||||||||||||||
|
|
@@ -90,52 +109,146 @@ private static void LoadNuGetPlugins(XDocument project, List<PluginContext> plug | |||||||||||||||||||||||||||
| var packageVersion = p.Attribute("Version")!.Value; | ||||||||||||||||||||||||||||
| var packageNameLower = packageName.ToLowerInvariant(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return Path.Combine( | ||||||||||||||||||||||||||||
| var dllPath = Path.Combine( | ||||||||||||||||||||||||||||
| globalPackagesFolder, | ||||||||||||||||||||||||||||
| packageNameLower, | ||||||||||||||||||||||||||||
| packageVersion, | ||||||||||||||||||||||||||||
| "lib", | ||||||||||||||||||||||||||||
| targetFramework, // e.g. "net9.0" | ||||||||||||||||||||||||||||
| $"{packageName}.dll" // case-sensitive match here | ||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return new NuGetPluginInfo(packageName, packageVersion, dllPath); | ||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||
| .Where(File.Exists) | ||||||||||||||||||||||||||||
| .ToList(); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (pluginFiles.Count > 0) | ||||||||||||||||||||||||||||
| private static List<ProjectRefPluginInfo> GetProjectRefPluginInfo(XDocument project, string projectDirectory, string configuration) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| var projectReferences = project.Descendants("ProjectReference") | ||||||||||||||||||||||||||||
| .Select(p => p.Attribute("Include")?.Value) | ||||||||||||||||||||||||||||
| .Where(path => path is not null && Path.GetFileName(path).StartsWith("BlakePlugin.")) | ||||||||||||||||||||||||||||
| .Select(path => Path.GetFullPath(Path.Combine(projectDirectory, path!))) | ||||||||||||||||||||||||||||
| .ToList(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (projectReferences.Count == 0) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger.LogInformation("Found {pluginCount} NuGet plugins in the .csproj file.", pluginFiles.Count); | ||||||||||||||||||||||||||||
| LoadPluginDLLs(pluginFiles, plugins, logger); | ||||||||||||||||||||||||||||
| return new List<ProjectRefPluginInfo>(); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Find the target framework dynamically from the .csproj file | ||||||||||||||||||||||||||||
| var targetFramework = project.Descendants("TargetFramework") | ||||||||||||||||||||||||||||
| .Select(tf => tf.Value) | ||||||||||||||||||||||||||||
| .FirstOrDefault() ?? "net9.0"; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return projectReferences.Select(pluginProject => | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger.LogInformation("No NuGet plugins found in the .csproj file."); | ||||||||||||||||||||||||||||
| var pluginName = Path.GetFileNameWithoutExtension(pluginProject); | ||||||||||||||||||||||||||||
| var outputPath = Path.Combine(Path.GetDirectoryName(pluginProject)!, "bin", configuration, targetFramework, $"{pluginName}.dll"); | ||||||||||||||||||||||||||||
| return new ProjectRefPluginInfo(pluginProject, outputPath); | ||||||||||||||||||||||||||||
| }).ToList(); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| private static bool IsNuGetPluginValid(NuGetPluginInfo plugin, ILogger? logger) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| if (!File.Exists(plugin.DllPath)) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger?.LogDebug("NuGet plugin DLL not found: {dllPath}", plugin.DllPath); | ||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| try | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| var fileVersionInfo = FileVersionInfo.GetVersionInfo(plugin.DllPath); | ||||||||||||||||||||||||||||
| var fileVersion = fileVersionInfo.FileVersion; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (string.IsNullOrEmpty(fileVersion)) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger?.LogDebug("No file version found for NuGet plugin: {dllPath}", plugin.DllPath); | ||||||||||||||||||||||||||||
| return true; // Assume valid if no version info | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // For NuGet packages, the file version should match the package version | ||||||||||||||||||||||||||||
| // Some packages may have different versioning schemes, so we'll be lenient | ||||||||||||||||||||||||||||
| if (fileVersion.StartsWith(plugin.Version)) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| logger?.LogDebug("Version mismatch for NuGet plugin {packageName}: expected {expectedVersion}, found {actualVersion}", | ||||||||||||||||||||||||||||
| plugin.PackageName, plugin.Version, fileVersion); | ||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| catch (Exception ex) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| logger?.LogDebug(ex, "Error checking version for NuGet plugin: {dllPath}", plugin.DllPath); | ||||||||||||||||||||||||||||
| return false; // Assume invalid if we can't check version | ||||||||||||||||||||||||||||
|
Comment on lines
+182
to
+185
|
||||||||||||||||||||||||||||
| catch (Exception ex) | |
| { | |
| logger?.LogDebug(ex, "Error checking version for NuGet plugin: {dllPath}", plugin.DllPath); | |
| return false; // Assume invalid if we can't check version | |
| catch (NotSupportedException ex) | |
| { | |
| logger?.LogWarning(ex, "File format not supported when checking version for NuGet plugin: {dllPath}. Assuming valid.", plugin.DllPath); | |
| return true; // Assume valid if file format is not supported | |
| } | |
| catch (Exception ex) | |
| { | |
| logger?.LogError(ex, "Unexpected error checking version for NuGet plugin: {dllPath}. Assuming invalid.", plugin.DllPath); | |
| return false; // Assume invalid for other exceptions |
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.
Using
StartsWithfor version comparison is unreliable and could lead to false positives. For example, version '1.0' would incorrectly match file version '1.0.1.2' or '1.00.0'. Consider usingSystem.Versionfor proper semantic version comparison or exact string matching.