From c80faea95ec961426f3ac3938d465e17fe418fe6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 8 Sep 2024 02:40:17 +0000 Subject: [PATCH] [release/9.0.1xx] ResolvePackageReferences: PackageDependenciesDesignTime inherits transitive package DiagnosticLevel (#43262) Co-authored-by: Andy Zivkovic --- ...WantToGetDependenciesViaDesignTimeBuild.cs | 52 ++++++++- .../ResolvePackageAssets.cs | 104 +++++++++++++++--- 2 files changed, 135 insertions(+), 21 deletions(-) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenThatWeWantToGetDependenciesViaDesignTimeBuild.cs b/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenThatWeWantToGetDependenciesViaDesignTimeBuild.cs index bcc395e48d64..5a97538139b3 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenThatWeWantToGetDependenciesViaDesignTimeBuild.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenThatWeWantToGetDependenciesViaDesignTimeBuild.cs @@ -37,10 +37,10 @@ public void ItShouldIgnoreAllDependenciesWithTypeNotEqualToPackageOrUnresolved() Assert.Equal(2, task.PackageDependenciesDesignTime.Count()); - // Verify only + // Verify only // top.package1 is type 'package' // top.package2 is type 'unresolved' - // + // // top.package3 is type 'unknown'. Should not appear in the list var item1 = task.PackageDependenciesDesignTime[0]; Assert.Equal("top.package1/1.0.0", item1.ItemSpec); @@ -372,14 +372,38 @@ public void ItShouldOnlyReturnTopLevelPackages() var item1 = task.PackageDependenciesDesignTime[0]; Assert.Equal("top.package1/1.0.0", item1.ItemSpec); + Assert.Equal("Error", item1.GetMetadata("DiagnosticLevel")); var item2 = task.PackageDependenciesDesignTime[1]; Assert.Equal("top.package2/1.0.0", item2.ItemSpec); + Assert.Equal(string.Empty, item2.GetMetadata("DiagnosticLevel")); var item3 = task.PackageDependenciesDesignTime[2]; Assert.Equal("top.package3/1.0.0", item3.ItemSpec); + Assert.Equal("Warning", item3.GetMetadata("DiagnosticLevel")); } + /// + /// Create a NuGet assets file for a project with 3 direct references, and 3 transitive references. + /// + /// Path to the NuGet global packages folder that the assets file will refer to. + /// Whether "top.package2" is a "package" (default) or "project" reference. + /// Whether "top.package2" is a "package" (default) or "project" reference. + /// The JSON string representing the project. + /// + /// The reference structure is as follows:
+ /// proj
+ /// + top.package1
+ /// | + dependent.package1
+ /// | | + dependent.package3
+ /// | + dependent.package2
+ /// + top.package2
+ /// + top.package2
+ /// - + dependent.package1
+ /// - - + dependent.package3
+ /// + /// dependent.package2 has an error message, and dependent.package3 has a warning. + /// private string CreateBasicProjectAssetsFile(string testRoot, string package2Type = "package", string package3Type = "package") { var json = @@ -587,7 +611,29 @@ private string CreateBasicProjectAssetsFile(string testRoot, string package2Type } } } - } + }, + "logs": [ + { + "code": "NU1001", + "level": "Error", + "warningLevel": 2, + "message": "some warning message", + "libraryId": "dependent.package2", + "targetGraphs": [ + "net6.0" + ] + }, + { + "code": "NU1002", + "level": "Warning", + "warningLevel": 1, + "message": "some warning message", + "libraryId": "dependent.package3", + "targetGraphs": [ + "net6.0" + ] + } + ] } """; return json.Replace("PACKAGE2_TYPE", package2Type) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs b/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs index 364d9b04a7f5..e1e9fcf1b39d 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs @@ -1438,6 +1438,7 @@ private void WritePackageDependenciesDesignTime() // Scan PackageDependencies to build the set of packages in our target. var allowItemSpecs = GetPackageDependencies(); + var diagnosticLevels = GetPackageDiagnosticLevels(); foreach (var package in _lockFile.Libraries) { @@ -1477,8 +1478,11 @@ private void WritePackageDependenciesDesignTime() : itemPath) ?? string.Empty; WriteMetadata(MetadataKeys.Path, path); - string itemDiagnosticLevel = GetPackageDiagnosticLevel(package); - var diagnosticLevel = itemDiagnosticLevel ?? string.Empty; + string diagnosticLevel = string.Empty; + if (diagnosticLevels?.TryGetValue(package.Name, out LogLevel level) ?? false) + { + diagnosticLevel = level.ToString(); + } WriteMetadata(MetadataKeys.DiagnosticLevel, diagnosticLevel); } } @@ -1506,28 +1510,92 @@ HashSet GetPackageDependencies() static string GetPackageId(LockFileTargetLibrary package) => $"{package.Name}/{package.Version.ToNormalizedString()}"; - string GetPackageDiagnosticLevel(LockFileLibrary package) + Dictionary GetPackageDiagnosticLevels() { - string target = _task.TargetFramework ?? ""; + if (_lockFile.LogMessages.Count == 0) + { + return null; + } + + var packageReverseDependencies = GetReverseDependencies(); + var result = new Dictionary(); + for (int i = 0; i < _lockFile.LogMessages.Count; i++) + { + var message = _lockFile.LogMessages[i]; + if (string.IsNullOrEmpty(message.LibraryId)) continue; - var messages = _lockFile.LogMessages.Where(log => - log.LibraryId == package.Name && - log.TargetGraphs.Any(tg => + if (message.TargetGraphs is null || message.TargetGraphs.Count == 0 || message.TargetGraphs.Any(ForCurrentTargetFramework)) { - var parts = tg.Split(LockFile.DirectorySeparatorChar); - var parsedTargetGraph = NuGetFramework.Parse(parts[0]); - var alias = _lockFile.PackageSpec.TargetFrameworks - .FirstOrDefault(tf => tf.FrameworkName == parsedTargetGraph) - ?.TargetAlias ?? tg; - return alias == target; - })); - - if (!messages.Any()) + ApplyDiagnosticLevel(message.LibraryId, message.Level, result, packageReverseDependencies); + } + } + + return result; + + Dictionary> GetReverseDependencies() { - return string.Empty; + var packageReverseDependencies = new Dictionary>(_compileTimeTarget.Libraries.Count, StringComparer.OrdinalIgnoreCase); + for (int i = 0; i < _compileTimeTarget.Libraries.Count; i++) + { + var parentPackage = _compileTimeTarget.Libraries[i]; + if (string.IsNullOrEmpty(parentPackage.Name)) continue; + + if (!packageReverseDependencies.ContainsKey(parentPackage.Name)) + { + packageReverseDependencies[parentPackage.Name] = new HashSet(StringComparer.OrdinalIgnoreCase); + } + + for (int j = 0; j < parentPackage.Dependencies.Count; j++) + { + var dependency = parentPackage.Dependencies[j].Id; + + if (!packageReverseDependencies.TryGetValue(dependency, out HashSet parentPackages)) + { + parentPackages = new HashSet(StringComparer.OrdinalIgnoreCase); + packageReverseDependencies[dependency] = parentPackages; + } + + parentPackages.Add(parentPackage.Name); + } + } + return packageReverseDependencies; + } + + bool ForCurrentTargetFramework(string targetFramework) + { + var parts = targetFramework.Split(LockFile.DirectorySeparatorChar); + var parsedTargetGraph = NuGetFramework.Parse(parts[0]); + var alias = _lockFile.PackageSpec.TargetFrameworks + .FirstOrDefault(tf => tf.FrameworkName == parsedTargetGraph) + ?.TargetAlias ?? targetFramework; + return alias == _task.TargetFramework; } - return messages.Max(log => log.Level).ToString(); + void ApplyDiagnosticLevel(string package, LogLevel messageLevel, Dictionary diagnosticLevels, Dictionary> reverseDependencies) + { + if (!reverseDependencies.TryGetValue(package, out HashSet parentPackages)) + { + // The package is not used in the current TargetFramework + return; + } + + if (diagnosticLevels.TryGetValue(package, out LogLevel cachedLevel)) + { + // Only continue if we need to increase the level + if (cachedLevel >= messageLevel) + { + return; + } + } + + diagnosticLevels[package] = messageLevel; + + // Flow changes upwards, towards the direct PackageReference + foreach (var parentPackage in parentPackages) + { + ApplyDiagnosticLevel(parentPackage, messageLevel, diagnosticLevels, reverseDependencies); + } + } } static DependencyType GetDependencyType(string dependencyTypeString)