diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/BinLogProcessor.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/BinLogProcessor.cs index d4134147b..5a1fc23f9 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/BinLogProcessor.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/BinLogProcessor.cs @@ -221,30 +221,10 @@ private void AddOrMergeProjectInfo( existing.InnerBuilds.Add(projectInfo); } } - else if (existing.IsOuterBuild && projectInfo.IsOuterBuild) - { - // Both are outer builds (e.g. build + publish of a multi-targeted project) - // Merge inner builds: for matching TFMs, merge; for new TFMs, add - foreach (var newInner in projectInfo.InnerBuilds) - { - var matchingInner = existing.InnerBuilds.FirstOrDefault( - ib => string.Equals(ib.TargetFramework, newInner.TargetFramework, StringComparison.OrdinalIgnoreCase)); - if (matchingInner != null) - { - matchingInner.MergeWith(newInner); - } - else - { - existing.InnerBuilds.Add(newInner); - } - } - - // Merge the outer build properties/items too - existing.MergeWith(projectInfo); - } else { - // Fallback: merge properties/items as superset + // Both are outer builds (e.g. build + publish of a multi-targeted project), or fallback: + // merge properties/items and inner builds (by TFM) as superset existing.MergeWith(projectInfo); } } diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs index f155b0bbe..253180bd1 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs @@ -149,7 +149,7 @@ internal MSBuildBinaryLogComponentDetector( public override IEnumerable SupportedComponentTypes { get; } = [ComponentType.NuGet, ComponentType.DotNet]; /// - public override int Version { get; } = 1; + public override int Version { get; } = 2; /// public override Task ExecuteDetectorAsync(ScanRequest request, CancellationToken cancellationToken = default) @@ -246,6 +246,26 @@ private static MSBuildProjectInfo GetInnerBuildOrDefault(MSBuildProjectInfo proj return projectInfo; } + /// + /// Gets the assets files a project info should be indexed by. + /// + /// + /// The outer build of a multi-targeted project never has ProjectAssetsFile, only the inner builds do. + /// + private static IEnumerable GetProjectAssetsFiles(MSBuildProjectInfo projectInfo) + { + if (!string.IsNullOrEmpty(projectInfo.ProjectAssetsFile)) + { + return [projectInfo.ProjectAssetsFile]; + } + + return projectInfo.InnerBuilds + .Select(ib => ib.ProjectAssetsFile) + .Where(f => !string.IsNullOrEmpty(f)) + .Select(f => f!) + .Distinct(StringComparer.OrdinalIgnoreCase); + } + /// /// Maps the MSBuild OutputType property to "application" or "library". /// Returns null for empty/unknown values rather than guessing. @@ -336,30 +356,38 @@ private void IndexProjectInfo(MSBuildProjectInfo projectInfo, List asset // Use AddOrUpdate+MergeWith so that multiple binlogs for the same project // (e.g., build and publish passes) form a superset rather than keeping only the first. // Normalize to forward slashes so lookup from OS-native ComponentStream.Location matches. - if (!string.IsNullOrEmpty(projectInfo.ProjectAssetsFile)) + foreach (var assetsFile in GetProjectAssetsFiles(projectInfo)) { - var normalizedAssetsPath = PathRebasingUtility.NormalizePath(projectInfo.ProjectAssetsFile); + var normalizedAssetsPath = PathRebasingUtility.NormalizePath(assetsFile); this.projectInfoByAssetsFile.AddOrUpdate( normalizedAssetsPath, _ => projectInfo, (_, existing) => existing.MergeWith(projectInfo)); - assetsFilesFound.Add(projectInfo.ProjectAssetsFile); + assetsFilesFound.Add(assetsFile); } } private void LogMissingAssetsWarnings(MSBuildProjectInfo projectInfo) { - if (string.IsNullOrEmpty(projectInfo.ProjectAssetsFile)) + var hasAssetFile = false; + + foreach (var assetsFile in GetProjectAssetsFiles(projectInfo)) { - this.Logger.LogWarning( - "No ProjectAssetsFile property found in binlog for project: {ProjectPath}. NuGet dependencies may not be detected.", - projectInfo.ProjectPath); + hasAssetFile = true; + + if (!this.fileUtilityService.Exists(assetsFile)) + { + this.Logger.LogWarning( + "Project.assets.json referenced in binlog does not exist: {AssetsFile} (from project {ProjectPath})", + assetsFile, + projectInfo.ProjectPath); + } } - else if (!this.fileUtilityService.Exists(projectInfo.ProjectAssetsFile)) + + if (!hasAssetFile) { this.Logger.LogWarning( - "Project.assets.json referenced in binlog does not exist: {AssetsFile} (from project {ProjectPath})", - projectInfo.ProjectAssetsFile, + "No ProjectAssetsFile property found in binlog for project: {ProjectPath}. NuGet dependencies may not be detected.", projectInfo.ProjectPath); } } diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildProjectInfo.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildProjectInfo.cs index 34be3bc78..af8f0e10f 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildProjectInfo.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildProjectInfo.cs @@ -2,6 +2,7 @@ namespace Microsoft.ComponentDetection.Detectors.NuGet; using System; using System.Collections.Generic; +using System.Linq; using Microsoft.Build.Framework; /// @@ -238,11 +239,17 @@ public bool TryRemoveItem(string itemType, string itemSpec) /// and target framework — the merge fills in any values that were not set rather than overriding. /// Boolean properties use logical OR (any true value is sufficient to classify the project). /// Items from are added if not already present. + /// Inner builds are merged by target framework: matching ones are merged recursively, new ones are added. /// /// The other project info to merge from. /// This instance for fluent chaining. public MSBuildProjectInfo MergeWith(MSBuildProjectInfo other) { + if (ReferenceEquals(this, other)) + { + return this; + } + // Merge boolean properties: true wins. For all classification booleans (IsTestProject, // IsDevelopment, IsShipping, etc.), if any pass reports true it is sufficient to classify // the project accordingly. These values are not expected to differ across passes, if they @@ -266,6 +273,22 @@ public MSBuildProjectInfo MergeWith(MSBuildProjectInfo other) MergeItems(this.PackageReference, other.PackageReference); MergeItems(this.PackageDownload, other.PackageDownload); + // Merge inner builds by target framework, so that per-TFM properties and items + // from later passes (e.g. publish) of a multi-targeted project are not lost. + foreach (var otherInner in other.InnerBuilds) + { + var matchingInner = this.InnerBuilds.FirstOrDefault( + ib => string.Equals(ib.TargetFramework, otherInner.TargetFramework, StringComparison.OrdinalIgnoreCase)); + if (matchingInner != null) + { + matchingInner.MergeWith(otherInner); + } + else + { + this.InnerBuilds.Add(otherInner); + } + } + return this; } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs index fbf82a9c1..5bd580ca4 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs @@ -502,6 +502,51 @@ public async Task WithBinlog_MultiTarget_PerTfmDevDependency() recorder.GetEffectiveDevDependencyValue(component.Component.Id).Should().BeFalse(); } + [TestMethod] + public async Task WithBinlog_MultiTarget_OuterBuildWithoutAssetsFile_IsShippingFalse_AllDependenciesMarkedAsDev() + { + var outer = CreateProjectInfo(targetFramework: null); + outer.ProjectAssetsFile = null; + outer.IsShipping = false; + outer.TrySetProperty("TargetFrameworks", "net8.0;net6.0"); + + var innerNet8 = new MSBuildProjectInfo { ProjectPath = ProjectPath, ProjectAssetsFile = AssetsFilePath, TargetFramework = "net8.0", IsShipping = false }; + var innerNet6 = new MSBuildProjectInfo { ProjectPath = ProjectPath, ProjectAssetsFile = AssetsFilePath, TargetFramework = "net6.0", IsShipping = false }; + + outer.InnerBuilds.Add(innerNet8); + outer.InnerBuilds.Add(innerNet6); + + var (result, recorder) = await ExecuteWithBinlogAsync([outer], MultiTargetAssetsJson()); + + result.ResultCode.Should().Be(ProcessingResultCode.Success); + var component = recorder.GetDetectedComponents() + .Single(c => c.Component is NuGetComponent { Name: "Newtonsoft.Json" }); + recorder.GetEffectiveDevDependencyValue(component.Component.Id).Should().BeTrue(); + } + + [TestMethod] + public async Task WithBinlog_MultiTarget_OuterBuildWithoutAssetsFile_UsesInnerBuildPerTfm() + { + var outer = CreateProjectInfo(targetFramework: null); + outer.ProjectAssetsFile = null; + outer.TrySetProperty("TargetFrameworks", "net8.0;net6.0"); + + var innerNet8 = new MSBuildProjectInfo { ProjectPath = ProjectPath, ProjectAssetsFile = AssetsFilePath, TargetFramework = "net8.0" }; + AddPackageReference(innerNet8, "Newtonsoft.Json", isDevelopmentDependency: true); + var innerNet6 = new MSBuildProjectInfo { ProjectPath = ProjectPath, ProjectAssetsFile = AssetsFilePath, TargetFramework = "net6.0" }; + AddPackageReference(innerNet6, "Newtonsoft.Json", isDevelopmentDependency: true); + + outer.InnerBuilds.Add(innerNet8); + outer.InnerBuilds.Add(innerNet6); + + var (result, recorder) = await ExecuteWithBinlogAsync([outer], MultiTargetAssetsJson()); + + result.ResultCode.Should().Be(ProcessingResultCode.Success); + var component = recorder.GetDetectedComponents() + .Single(c => c.Component is NuGetComponent { Name: "Newtonsoft.Json" }); + recorder.GetEffectiveDevDependencyValue(component.Component.Id).Should().BeTrue(); + } + [TestMethod] public async Task WithBinlog_NoBinlogMatch_FallsBackToStandardProcessing() { @@ -654,6 +699,65 @@ public async Task MultipleBinlogs_MergesTestProjectFlag_AllDepsAreDev() recorder.GetEffectiveDevDependencyValue(component.Component.Id).Should().BeTrue(); } + [TestMethod] + public async Task MultipleBinlogs_MultiTarget_MergesInnerBuildProperties_AllDepsAreDev() + { + // Outer builds of a multi-targeted project don't have ProjectAssetsFile: + // properties set only on the inner builds of the second binlog must still be merged. + static MSBuildProjectInfo CreateMultiTargetProjectInfo(bool? isTestProject) + { + var outer = CreateProjectInfo(targetFramework: null); + outer.ProjectAssetsFile = null; + outer.TrySetProperty("TargetFrameworks", "net8.0;net6.0"); + outer.InnerBuilds.Add(new MSBuildProjectInfo { ProjectPath = ProjectPath, ProjectAssetsFile = AssetsFilePath, TargetFramework = "net8.0", IsTestProject = isTestProject }); + outer.InnerBuilds.Add(new MSBuildProjectInfo { ProjectPath = ProjectPath, ProjectAssetsFile = AssetsFilePath, TargetFramework = "net6.0", IsTestProject = isTestProject }); + return outer; + } + + var binlogs = new Dictionary> + { + [BinlogFilePath] = [CreateMultiTargetProjectInfo(isTestProject: null)], + [TestBinlogFilePath] = [CreateMultiTargetProjectInfo(isTestProject: true)], + }; + + var (result, recorder) = await ExecuteWithMultipleBinlogsAsync(binlogs, MultiTargetAssetsJson()); + + result.ResultCode.Should().Be(ProcessingResultCode.Success); + var component = recorder.GetDetectedComponents() + .Single(c => c.Component is NuGetComponent { Name: "Newtonsoft.Json" }); + recorder.GetEffectiveDevDependencyValue(component.Component.Id).Should().BeTrue(); + } + + [TestMethod] + public async Task MultipleBinlogs_MultiTarget_AddsInnerBuildsFromLaterBinlog() + { + // First binlog only built net8.0, second only built net6.0: both inner builds + // (and their package metadata) must be available when processing the shared assets file. + static MSBuildProjectInfo CreateMultiTargetProjectInfo(string targetFramework) + { + var outer = CreateProjectInfo(targetFramework: null); + outer.ProjectAssetsFile = null; + outer.TrySetProperty("TargetFrameworks", "net8.0;net6.0"); + var inner = new MSBuildProjectInfo { ProjectPath = ProjectPath, ProjectAssetsFile = AssetsFilePath, TargetFramework = targetFramework }; + AddPackageReference(inner, "Newtonsoft.Json", isDevelopmentDependency: true); + outer.InnerBuilds.Add(inner); + return outer; + } + + var binlogs = new Dictionary> + { + [BinlogFilePath] = [CreateMultiTargetProjectInfo("net8.0")], + [PublishBinlogFilePath] = [CreateMultiTargetProjectInfo("net6.0")], + }; + + var (result, recorder) = await ExecuteWithMultipleBinlogsAsync(binlogs, MultiTargetAssetsJson()); + + result.ResultCode.Should().Be(ProcessingResultCode.Success); + var component = recorder.GetDetectedComponents() + .Single(c => c.Component is NuGetComponent { Name: "Newtonsoft.Json" }); + recorder.GetEffectiveDevDependencyValue(component.Component.Id).Should().BeTrue(); + } + // ================================================================ // Cancellation // ================================================================