C# Change desktop dotnet assembly lookup to fall back to nuget reference assemblies #15600
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,7 +61,7 @@ public DependencyManager(string srcDir, IDependencyOptions options, ILogger logg | |
| try | ||
| { | ||
| this.dotnet = DotNet.Make(options, logger, tempWorkingDirectory); | ||
| runtimeLazy = new Lazy<Runtime>(() => new Runtime(dotnet, logger)); | ||
| } | ||
| catch | ||
| { | ||
|
|
@@ -111,7 +111,7 @@ public DependencyManager(string srcDir, IDependencyOptions options, ILogger logg | |
| logger.LogInfo($"Unresolved reference {r.Key} in project {r.Value}"); | ||
| } | ||
|
|
||
| var webViewExtractionOption = Environment.GetEnvironmentVariable(EnvironmentVariableNames.WebViewGeneration); | ||
| if (webViewExtractionOption == null || | ||
| bool.TryParse(webViewExtractionOption, out var shouldExtractWebViews) && | ||
| shouldExtractWebViews) | ||
|
|
@@ -158,6 +158,53 @@ private HashSet<string> AddFrameworkDlls(HashSet<string> dllPaths) | |
| { | ||
| var frameworkLocations = new HashSet<string>(); | ||
|
|
||
| var frameworkReferences = Environment.GetEnvironmentVariable(EnvironmentVariableNames.DotnetFrameworkReferences); | ||
| var frameworkReferencesUseSubfolders = Environment.GetEnvironmentVariable(EnvironmentVariableNames.DotnetFrameworkReferencesUseSubfolders); | ||
| _ = bool.TryParse(frameworkReferencesUseSubfolders, out var useSubfolders); | ||
| if (!string.IsNullOrWhiteSpace(frameworkReferences)) | ||
| { | ||
| RemoveFrameworkNugetPackages(dllPaths); | ||
| RemoveNugetPackageReference(FrameworkPackageNames.AspNetCoreFramework, dllPaths); | ||
| RemoveNugetPackageReference(FrameworkPackageNames.WindowsDesktopFramework, dllPaths); | ||
|
|
||
| var frameworkPaths = frameworkReferences.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries); | ||
|
|
||
| foreach (var path in frameworkPaths) | ||
| { | ||
| if (!Directory.Exists(path)) | ||
| { | ||
| logger.LogError($"Specified framework reference path '{path}' does not exist."); | ||
| continue; | ||
| } | ||
|
|
||
| if (useSubfolders) | ||
| { | ||
| dllPaths.Add(path); | ||
| frameworkLocations.Add(path); | ||
| continue; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| var dlls = Directory.GetFiles(path, "*.dll", new EnumerationOptions { RecurseSubdirectories = false, MatchCasing = MatchCasing.CaseInsensitive }); | ||
| if (dlls.Length == 0) | ||
| { | ||
| logger.LogError($"No DLLs found in specified framework reference path '{path}'."); | ||
| continue; | ||
| } | ||
|
|
||
| dllPaths.UnionWith(dlls); | ||
| frameworkLocations.UnionWith(dlls); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| logger.LogError($"Error while searching for DLLs in '{path}': {e.Message}"); | ||
| } | ||
|
Comment on lines
+199
to
+202
Check noticeCode scanning / CodeQL Generic catch clause |
||
| } | ||
|
|
||
| return frameworkLocations; | ||
| } | ||
|
|
||
| AddNetFrameworkDlls(dllPaths, frameworkLocations); | ||
| AddAspNetCoreFrameworkDlls(dllPaths, frameworkLocations); | ||
| AddMicrosoftWindowsDesktopDlls(dllPaths, frameworkLocations); | ||
|
|
@@ -203,9 +250,9 @@ private void RestoreNugetPackages(List<FileInfo> allNonBinaryFiles, IEnumerable< | |
| nugetPackageDllPaths.ExceptWith(excludedPaths); | ||
| dllPaths.UnionWith(nugetPackageDllPaths); | ||
| } | ||
| catch (Exception exc) | ||
| { | ||
| logger.LogError($"Failed to restore Nuget packages with nuget.exe: {exc.Message}"); | ||
| } | ||
|
|
||
| var restoredProjects = RestoreSolutions(allSolutions, out var assets1); | ||
|
|
@@ -296,14 +343,23 @@ private static DirectoryInfo[] GetPackageVersionSubDirectories(string packagePat | |
| .ToArray(); | ||
| } | ||
|
|
||
| private void RemoveFrameworkNugetPackages(ISet<string> dllPaths, int fromIndex = 0) | ||
| { | ||
| var packagesInPrioOrder = FrameworkPackageNames.NetFrameworks; | ||
| for (var i = fromIndex; i < packagesInPrioOrder.Length; i++) | ||
| { | ||
| RemoveNugetPackageReference(packagesInPrioOrder[i], dllPaths); | ||
| } | ||
| } | ||
|
|
||
| private void AddNetFrameworkDlls(ISet<string> dllPaths, ISet<string> frameworkLocations) | ||
| { | ||
| // Multiple dotnet framework packages could be present. | ||
| // The order of the packages is important, we're adding the first one that is present in the nuget cache. | ||
| var packagesInPrioOrder = FrameworkPackageNames.NetFrameworks; | ||
|
|
||
| var frameworkPaths = packagesInPrioOrder | ||
| .Select((s, index) => (Index: index, Path: GetPackageDirectory(s, packageDirectory))) | ||
| .Where(pair => pair.Path is not null) | ||
| .ToArray(); | ||
|
|
||
|
|
@@ -317,12 +373,7 @@ private void AddNetFrameworkDlls(ISet<string> dllPaths, ISet<string> frameworkLo | |
| } | ||
|
|
||
| SelectNewestFrameworkPath(frameworkPath.Path, ".NET Framework", dllPaths, frameworkLocations); | ||
| RemoveFrameworkNugetPackages(dllPaths, frameworkPath.Index + 1); | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -335,6 +386,16 @@ private void AddNetFrameworkDlls(ISet<string> dllPaths, ISet<string> frameworkLo | |
| else if (fileContent.IsLegacyProjectStructureUsed) | ||
| { | ||
| runtimeLocation = Runtime.DesktopRuntime; | ||
|
|
||
| if (runtimeLocation is null) | ||
| { | ||
| logger.LogInfo("No .NET Desktop Runtime location found. Attempting to restore the .NET Framework reference assemblies manually."); | ||
|
|
||
| if (TryRestorePackageManually(FrameworkPackageNames.LatestNetFrameworkReferenceAssemblies, null)) | ||
| { | ||
| runtimeLocation = GetPackageDirectory(FrameworkPackageNames.LatestNetFrameworkReferenceAssemblies, missingPackageDirectory); | ||
| } | ||
|
Comment on lines
+394
to
+397
There was a problem hiding this comment. Early commits in this PR tested this behaviour, but that's not the case any longer. Is there an easy way of covering this with an integration test? There was a problem hiding this comment. Is the problem that this is an extremely unhappy path that is hard to replicate (as there none of the runners that satisfy the criteria for missing paths)? There was a problem hiding this comment. Yes, that's correct. We'd get here if there was no mono installation on linux/macos runners, and no .net install on windows. (I've faced the original issue on a mac with no mono.) There was a problem hiding this comment. Hmm, yes, then we have reached the limit for what is integration testable unless we introduce some way of adding extra test configuration (we probably shouldn't do this). There was a problem hiding this comment. When #15650 and this PR are merged, I'll set up some manual testing to see if the extractor is really working without mono. |
||
| } | ||
| } | ||
|
|
||
| runtimeLocation ??= Runtime.ExecutingRuntime; | ||
|
|
@@ -374,7 +435,7 @@ private void AddAspNetCoreFrameworkDlls(ISet<string> dllPaths, ISet<string> fram | |
| } | ||
|
|
||
| // First try to find ASP.NET Core assemblies in the NuGet packages | ||
| if (GetPackageDirectory(FrameworkPackageNames.AspNetCoreFramework, packageDirectory) is string aspNetCorePackage) | ||
| { | ||
| SelectNewestFrameworkPath(aspNetCorePackage, "ASP.NET Core", dllPaths, frameworkLocations); | ||
| return; | ||
|
|
@@ -390,15 +451,15 @@ private void AddAspNetCoreFrameworkDlls(ISet<string> dllPaths, ISet<string> fram | |
|
|
||
| private void AddMicrosoftWindowsDesktopDlls(ISet<string> dllPaths, ISet<string> frameworkLocations) | ||
| { | ||
| if (GetPackageDirectory(FrameworkPackageNames.WindowsDesktopFramework, packageDirectory) is string windowsDesktopApp) | ||
| { | ||
| SelectNewestFrameworkPath(windowsDesktopApp, "Windows Desktop App", dllPaths, frameworkLocations); | ||
| } | ||
| } | ||
|
|
||
| private string? GetPackageDirectory(string packagePrefix, TemporaryDirectory root) | ||
| { | ||
| return new DirectoryInfo(root.DirInfo.FullName) | ||
| .EnumerateDirectories(packagePrefix + "*", new EnumerationOptions { MatchCasing = MatchCasing.CaseInsensitive, RecurseSubdirectories = false }) | ||
| .FirstOrDefault()? | ||
| .FullName; | ||
|
|
@@ -434,19 +495,19 @@ private void GenerateSourceFileFromImplicitUsings() | |
| } | ||
|
|
||
| // Hardcoded values from https://learn.microsoft.com/en-us/dotnet/core/project-sdk/overview#implicit-using-directives | ||
| usings.UnionWith([ "System", "System.Collections.Generic", "System.IO", "System.Linq", "System.Net.Http", "System.Threading", | ||
tamasvajk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "System.Threading.Tasks" ]); | ||
|
|
||
| if (fileContent.UseAspNetCoreDlls) | ||
| { | ||
| usings.UnionWith([ "System.Net.Http.Json", "Microsoft.AspNetCore.Builder", "Microsoft.AspNetCore.Hosting", | ||
| "Microsoft.AspNetCore.Http", "Microsoft.AspNetCore.Routing", "Microsoft.Extensions.Configuration", | ||
| "Microsoft.Extensions.DependencyInjection", "Microsoft.Extensions.Hosting", "Microsoft.Extensions.Logging" ]); | ||
| } | ||
|
|
||
| if (fileContent.UseWindowsForms) | ||
| { | ||
| usings.UnionWith(["System.Drawing", "System.Windows.Forms"]); | ||
| } | ||
|
|
||
| usings.UnionWith(fileContent.CustomImplicitUsings); | ||
|
|
@@ -828,47 +889,58 @@ private void DownloadMissingPackages(List<FileInfo> allFiles, ISet<string> dllPa | |
|
|
||
| Parallel.ForEach(notYetDownloadedPackages, new ParallelOptions { MaxDegreeOfParallelism = options.Threads }, package => | ||
| { | ||
| var success = TryRestorePackageManually(package, nugetConfig); | ||
| if (!success) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| lock (sync) | ||
| { | ||
| successCount++; | ||
| } | ||
| }); | ||
|
|
||
| CompilationInfos.Add(("Successfully ran fallback nuget restore", successCount.ToString())); | ||
|
|
||
| dllPaths.Add(missingPackageDirectory.DirInfo.FullName); | ||
| } | ||
|
|
||
| private bool TryRestorePackageManually(string package, string? nugetConfig) | ||
| { | ||
| logger.LogInfo($"Restoring package {package}..."); | ||
| using var tempDir = new TemporaryDirectory(ComputeTempDirectory(package, "missingpackages_workingdir")); | ||
| var success = dotnet.New(tempDir.DirInfo.FullName); | ||
| if (!success) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| success = dotnet.AddPackage(tempDir.DirInfo.FullName, package); | ||
| if (!success) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| var res = dotnet.Restore(new(tempDir.DirInfo.FullName, missingPackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: false, PathToNugetConfig: nugetConfig)); | ||
| if (!res.Success) | ||
| { | ||
| if (res.HasNugetPackageSourceError && nugetConfig is not null) | ||
| { | ||
| // Restore could not be completed because the listed source is unavailable. Try without the nuget.config: | ||
| res = dotnet.Restore(new(tempDir.DirInfo.FullName, missingPackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: false, PathToNugetConfig: null, ForceReevaluation: true)); | ||
| } | ||
|
|
||
| // TODO: the restore might fail, we could retry with a prerelease (*-* instead of *) version of the package. | ||
|
|
||
| if (!res.Success) | ||
| { | ||
| logger.LogInfo($"Failed to restore nuget package {package}"); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| public void Dispose(TemporaryDirectory? dir, string name) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| namespace Semmle.Extraction.CSharp.DependencyFetching | ||
| { | ||
| internal class EnvironmentVariableNames | ||
tamasvajk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| /// <summary> | ||
| /// Controls whether to generate source files from Asp.Net Core views (`.cshtml`, `.razor`). | ||
| /// </summary> | ||
| public const string WebViewGeneration = "CODEQL_EXTRACTOR_CSHARP_BUILDLESS_EXTRACT_WEB_VIEWS"; | ||
|
|
||
| /// <summary> | ||
| /// Specifies the location of .Net framework references added to the compilation. | ||
| /// </summary> | ||
| public const string DotnetFrameworkReferences = "CODEQL_EXTRACTOR_CSHARP_BUILDLESS_DOTNET_FRAMEWORK_REFERENCES"; | ||
|
|
||
| /// <summary> | ||
| /// Controls whether to use framework dependencies from subfolders. | ||
| /// </summary> | ||
| public const string DotnetFrameworkReferencesUseSubfolders = "CODEQL_EXTRACTOR_CSHARP_BUILDLESS_DOTNET_FRAMEWORK_REFERENCES_USE_SUBFOLDERS"; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,25 @@ | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace Semmle.Extraction.CSharp.DependencyFetching | ||
| { | ||
| internal static class FrameworkPackageNames | ||
| { | ||
| public static string LatestNetFrameworkReferenceAssemblies { get; } = "microsoft.netframework.referenceassemblies.net481"; | ||
|
|
||
| public static string AspNetCoreFramework { get; } = "microsoft.aspnetcore.app.ref"; | ||
|
|
||
| public static string WindowsDesktopFramework { get; } = "microsoft.windowsdesktop.app.ref"; | ||
|
|
||
| // The order of the packages is important. | ||
| public static string[] NetFrameworks { get; } = | ||
| [ | ||
| "microsoft.netcore.app.ref", // net7.0, ... net5.0, netcoreapp3.1, netcoreapp3.0 | ||
| "microsoft.netframework.referenceassemblies.", // net48, ..., net20 | ||
| "netstandard.library.ref", // netstandard2.1 | ||
| "netstandard.library" // netstandard2.0 | ||
| ]; | ||
|
|
||
| public static IEnumerable<string> AllFrameworks { get; } = | ||
| [.. NetFrameworks, AspNetCoreFramework, WindowsDesktopFramework]; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
I think this doesn't work on some early .net core variants, where the framework was split up into many nuget packages, and therefore they wouldn't be identified as framework references but as standard nuget packages. But for the moment, I think it's good enough.