Skip to content

Commit 932d183

Browse files
ViktorHoferViktorHofer
committed
Don't filter package assets and handle placeholder
Fixes #18165 Instead of manually filtering package assets, let NuGet calculate the runtime and compile time assets and handle placeholder files which are returned by NuGet.
1 parent 5f19e30 commit 932d183

22 files changed

+213
-52
lines changed

‎src/ApiCompat/Microsoft.DotNet.ApiCompat.Shared/ValidatePackage.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ public static void Run(Func<ISuppressionEngine, ICompatibilityLogger> logFactory
5454
bool enableStrictModeForBaselineValidation,
5555
string? baselinePackagePath,
5656
string? runtimeGraph,
57-
Dictionary<string, string[]>? packageAssemblyReferences,
58-
Dictionary<string, string[]>? baselinePackageAssemblyReferences)
57+
IReadOnlyDictionary<string, string[]>? packageAssemblyReferences,
58+
IReadOnlyDictionary<string, string[]>? baselinePackageAssemblyReferences)
5959
{
6060
// Configure the suppression engine. Ignore the passed in suppression file if it should be generated and doesn't yet exist.
6161
string? suppressionFileForEngine = generateSuppressionFile && !File.Exists(suppressionFile) ? null : suppressionFile;

‎src/ApiCompat/Microsoft.DotNet.PackageValidation/ApiCompatRunnerExtensions.cs‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5+
using System.Diagnostics;
56
using System.IO;
7+
using System.Linq;
68
using Microsoft.DotNet.ApiCompatibility.Abstractions;
79
using Microsoft.DotNet.ApiCompatibility.Logging;
810
using Microsoft.DotNet.ApiCompatibility.Runner;
@@ -26,11 +28,24 @@ public static void QueueApiCompatFromContentItem(this IApiCompatRunner apiCompat
2628
Package? rightPackage = null)
2729
{
2830
// Don't enqueue duplicate items (if no right package is supplied and items match)
29-
if (rightPackage == null && ContentItemCollectionEquals(leftContentItems, rightContentItems))
31+
if (rightPackage == null && leftContentItems.SequenceEqual(rightContentItems, ContentItemEqualityComparer.Instance))
3032
{
3133
return;
3234
}
3335

36+
// Don't perform api compatibility checks on placeholder files.
37+
if (leftContentItems.IsPlaceholderFile() && rightContentItems.IsPlaceholderFile())
38+
{
39+
leftContentItems[0].Properties.TryGetValue("tfm", out object? tfmRaw);
40+
string tfm = tfmRaw?.ToString() ?? string.Empty;
41+
42+
log.LogMessage(MessageImportance.Normal, Resources.SkipApiCompatForPlaceholderFiles, tfm);
43+
}
44+
45+
// Make sure placeholder package files aren't enqueued as api comparison check work items.
46+
Debug.Assert(!leftContentItems.IsPlaceholderFile() && !rightContentItems.IsPlaceholderFile(),
47+
"Placeholder files must not be enqueued for api comparison checks.");
48+
3449
MetadataInformation[] left = new MetadataInformation[leftContentItems.Count];
3550
for (int leftIndex = 0; leftIndex < leftContentItems.Count; leftIndex++)
3651
{
@@ -79,24 +94,5 @@ private static MetadataInformation GetMetadataInformation(ICompatibilityLogger l
7994

8095
return new MetadataInformation(Path.GetFileName(item.Path), item.Path, package.PackagePath, assemblyReferences, displayString);
8196
}
82-
83-
private static bool ContentItemCollectionEquals(IReadOnlyList<ContentItem> leftContentItems,
84-
IReadOnlyList<ContentItem> rightContentItems)
85-
{
86-
if (leftContentItems.Count != rightContentItems.Count)
87-
{
88-
return false;
89-
}
90-
91-
for (int i = 0; i < leftContentItems.Count; i++)
92-
{
93-
if (leftContentItems[i].Path != rightContentItems[i].Path)
94-
{
95-
return false;
96-
}
97-
}
98-
99-
return true;
100-
}
10197
}
10298
}

‎src/ApiCompat/Microsoft.DotNet.PackageValidation/ContentItemEqualityComparer.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System.Collections.Generic;
5+
using NuGet.ContentModel;
6+
7+
namespace Microsoft.DotNet.PackageValidation
8+
{
9+
internal class ContentItemEqualityComparer : IEqualityComparer<ContentItem>
10+
{
11+
public static readonly ContentItemEqualityComparer Instance = new();
12+
13+
private ContentItemEqualityComparer()
14+
{
15+
}
16+
17+
public bool Equals(ContentItem? x, ContentItem? y) => string.Equals(x?.Path, y?.Path);
18+
19+
public int GetHashCode(ContentItem obj) => obj.Path.GetHashCode();
20+
}
21+
}

‎src/ApiCompat/Microsoft.DotNet.PackageValidation/Package.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public class Package
4242
/// <summary>
4343
/// List of package dependencies per framework.
4444
/// </summary>
45-
public Dictionary<NuGetFramework, IEnumerable<PackageDependency>> PackageDependencies { get; }
45+
public IReadOnlyDictionary<NuGetFramework, IEnumerable<PackageDependency>> PackageDependencies { get; }
4646

4747
/// <summary>
4848
/// List of assets in the package.
@@ -82,7 +82,7 @@ public class Package
8282
/// <summary>
8383
/// List of assembly references grouped by target framework.
8484
/// </summary>
85-
public Dictionary<string, string[]>? AssemblyReferences { get; }
85+
public IReadOnlyDictionary<string, string[]>? AssemblyReferences { get; }
8686

8787
/// <summary>
8888
/// List of the frameworks in the package.
@@ -93,8 +93,8 @@ public Package(string packagePath,
9393
string packageId,
9494
string version,
9595
IEnumerable<string> packageAssets,
96-
Dictionary<NuGetFramework, IEnumerable<PackageDependency>> packageDependencies,
97-
Dictionary<string, string[]>? assemblyReferences = null)
96+
IReadOnlyDictionary<NuGetFramework, IEnumerable<PackageDependency>> packageDependencies,
97+
IReadOnlyDictionary<string, string[]>? assemblyReferences = null)
9898
{
9999
PackagePath = packagePath;
100100
PackageId = packageId;
@@ -131,7 +131,7 @@ public static void InitializeRuntimeGraph(string runtimeGraph)
131131
/// </summary>
132132
/// <param name="packagePath">The path to the package path.</param>
133133
/// <param name="packageAssemblyReferences">Optional assembly references grouped per target framework.</param>
134-
public static Package Create(string? packagePath, Dictionary<string, string[]>? packageAssemblyReferences = null)
134+
public static Package Create(string? packagePath, IReadOnlyDictionary<string, string[]>? packageAssemblyReferences = null)
135135
{
136136
if (string.IsNullOrEmpty(packagePath))
137137
{
@@ -147,7 +147,7 @@ public static Package Create(string? packagePath, Dictionary<string, string[]>?
147147
NuspecReader nuspecReader = packageReader.NuspecReader;
148148
string packageId = nuspecReader.GetId();
149149
string version = nuspecReader.GetVersion().ToString();
150-
IEnumerable<string> packageAssets = packageReader.GetFiles().Where(t => t.EndsWith(".dll")).ToArray();
150+
IEnumerable<string> packageAssets = packageReader.GetFiles();
151151

152152
Dictionary<NuGetFramework, IEnumerable<PackageDependency>> packageDependencies = new();
153153
foreach (PackageDependencyGroup item in nuspecReader.GetDependencyGroups())

‎src/ApiCompat/Microsoft.DotNet.PackageValidation/PackageExtensions.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.IO;
7+
using NuGet.ContentModel;
8+
using NuGet.Frameworks;
9+
10+
namespace Microsoft.DotNet.PackageValidation
11+
{
12+
internal static class PackageExtensions
13+
{
14+
private const string PlaceholderFile = "_._";
15+
16+
public static bool IsPlaceholderFile(this ContentItem contentItem) =>
17+
Path.GetFileName(contentItem.Path) == PlaceholderFile;
18+
19+
public static bool IsPlaceholderFile(this IReadOnlyList<ContentItem> contentItems) =>
20+
contentItems.Count == 1 && contentItems[0].IsPlaceholderFile();
21+
22+
public static bool SupportsRuntimeIdentifier(this NuGetFramework tfm, string rid) =>
23+
tfm.Framework != ".NETFramework" || rid.StartsWith("win", StringComparison.OrdinalIgnoreCase);
24+
}
25+
}

‎src/ApiCompat/Microsoft.DotNet.PackageValidation/Resources.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,7 @@
144144
<data name="MissingSearchDirectory" xml:space="preserve">
145145
<value>Could not find a reference directory in the provided directories for TargetFramework '{0}' when loading '{1}'. For more information see: https://aka.ms/dotnetpackagevalidation</value>
146146
</data>
147+
<data name="SkipApiCompatForPlaceholderFiles" xml:space="preserve">
148+
<value>Skipping api compatiblity check for target framework {0} as the package assets are placeholder files.</value>
149+
</data>
147150
</root>

‎src/ApiCompat/Microsoft.DotNet.PackageValidation/Validators/BaselinePackageValidator.cs‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,22 @@ public void Validate(PackageValidatorOption options)
4747
{
4848
// Search for compatible compile time assets in the latest package.
4949
IReadOnlyList<ContentItem>? latestCompileAssets = options.Package.FindBestCompileAssetForFramework(baselineTargetFramework);
50-
if (latestCompileAssets == null)
50+
// Emit an error if
51+
// - No latest compile time asset is available or
52+
// - The latest compile time asset is a placeholder but the baseline compile time asset isn't.
53+
if (latestCompileAssets == null ||
54+
(latestCompileAssets.IsPlaceholderFile() && !baselineCompileAssets.IsPlaceholderFile()))
5155
{
5256
_log.LogError(
5357
new Suppression(DiagnosticIds.TargetFrameworkDropped) { Target = baselineTargetFramework.ToString() },
5458
DiagnosticIds.TargetFrameworkDropped,
5559
Resources.MissingTargetFramework,
5660
baselineTargetFramework.ToString());
5761
}
62+
else if (baselineCompileAssets.IsPlaceholderFile() && !latestCompileAssets.IsPlaceholderFile())
63+
{
64+
// Ignore the newly added compile time asset in the latest package.
65+
}
5866
else if (options.EnqueueApiCompatWorkItems)
5967
{
6068
_apiCompatRunner.QueueApiCompatFromContentItem(_log,
@@ -72,14 +80,22 @@ public void Validate(PackageValidatorOption options)
7280
{
7381
// Search for compatible runtime assets in the latest package.
7482
IReadOnlyList<ContentItem>? latestRuntimeAssets = options.Package.FindBestRuntimeAssetForFramework(baselineTargetFramework);
75-
if (latestRuntimeAssets == null)
83+
// Emit an error if
84+
// - No latest runtime asset is available or
85+
// - The latest runtime asset is a placeholder but the baseline runtime asset isn't.
86+
if (latestRuntimeAssets == null ||
87+
(latestRuntimeAssets.IsPlaceholderFile() && !baselineRuntimeAssets.IsPlaceholderFile()))
7688
{
7789
_log.LogError(
7890
new Suppression(DiagnosticIds.TargetFrameworkDropped) { Target = baselineTargetFramework.ToString() },
7991
DiagnosticIds.TargetFrameworkDropped,
8092
Resources.MissingTargetFramework,
8193
baselineTargetFramework.ToString());
8294
}
95+
else if (baselineRuntimeAssets.IsPlaceholderFile() && !latestRuntimeAssets.IsPlaceholderFile())
96+
{
97+
// Ignore the newly added run time asset in the latest package.
98+
}
8399
else if (options.EnqueueApiCompatWorkItems)
84100
{
85101
_apiCompatRunner.QueueApiCompatFromContentItem(_log,
@@ -101,8 +117,13 @@ public void Validate(PackageValidatorOption options)
101117

102118
foreach (IGrouping<string, ContentItem> baselineRuntimeSpecificAssetsRidGroup in baselineRuntimeSpecificAssetsRidGroupedPerRid)
103119
{
120+
IReadOnlyList<ContentItem> baselineRuntimeSpecificAssetsForRid = baselineRuntimeSpecificAssetsRidGroup.ToArray();
104121
IReadOnlyList<ContentItem>? latestRuntimeSpecificAssets = options.Package.FindBestRuntimeAssetForFrameworkAndRuntime(baselineTargetFramework, baselineRuntimeSpecificAssetsRidGroup.Key);
105-
if (latestRuntimeSpecificAssets == null)
122+
// Emit an error if
123+
// - No latest runtime specific asset is available or
124+
// - The latest runtime specific asset is a placeholder but the baseline runtime specific asset isn't.
125+
if (latestRuntimeSpecificAssets == null ||
126+
(latestRuntimeSpecificAssets.IsPlaceholderFile() && !baselineRuntimeSpecificAssetsForRid.IsPlaceholderFile()))
106127
{
107128
_log.LogError(
108129
new Suppression(DiagnosticIds.TargetFrameworkAndRidPairDropped) { Target = baselineTargetFramework.ToString() + "-" + baselineRuntimeSpecificAssetsRidGroup.Key },
@@ -111,10 +132,14 @@ public void Validate(PackageValidatorOption options)
111132
baselineTargetFramework.ToString(),
112133
baselineRuntimeSpecificAssetsRidGroup.Key);
113134
}
135+
else if (baselineRuntimeSpecificAssetsForRid.IsPlaceholderFile() && !latestRuntimeSpecificAssets.IsPlaceholderFile())
136+
{
137+
// Ignore the newly added runtime specific asset in the latest package.
138+
}
114139
else if (options.EnqueueApiCompatWorkItems)
115140
{
116141
_apiCompatRunner.QueueApiCompatFromContentItem(_log,
117-
baselineRuntimeSpecificAssetsRidGroup.ToArray(),
142+
baselineRuntimeSpecificAssetsForRid,
118143
latestRuntimeSpecificAssets,
119144
apiCompatOptions,
120145
options.BaselinePackage,

‎src/ApiCompat/Microsoft.DotNet.PackageValidation/Validators/CompatibleFrameworkInPackageValidator.cs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,29 +51,26 @@ public void Validate(PackageValidatorOption options)
5151
foreach (NuGetFramework framework in options.Package.FrameworksInPackage.OrderByDescending(f => f.Version))
5252
{
5353
IReadOnlyList<ContentItem>? compileTimeAsset = options.Package.FindBestCompileAssetForFramework(framework);
54-
if (compileTimeAsset != null)
54+
if (compileTimeAsset != null && !compileTimeAsset.IsPlaceholderFile())
5555
compileAssetsQueue.Enqueue((framework, compileTimeAsset));
5656
}
5757

58-
while (compileAssetsQueue.Count > 0)
58+
// Iterate as long as assets are available for comparison.
59+
while (compileAssetsQueue.Count > 1)
5960
{
6061
(NuGetFramework framework, IReadOnlyList<ContentItem> compileTimeAsset) = compileAssetsQueue.Dequeue();
6162

62-
// If no assets are available for comparison, stop the iteration.
63-
if (compileAssetsQueue.Count == 0) break;
64-
6563
SelectionCriteria managedCriteria = conventions.Criteria.ForFramework(framework);
66-
6764
ContentItemCollection contentItemCollection = new();
6865
// The collection won't contain the current compile time asset as it is already dequeued.
6966
contentItemCollection.Load(compileAssetsQueue.SelectMany(a => a.Item2).Select(a => a.Path));
7067

7168
// Search for a compatible compile time asset and compare it.
72-
IList<ContentItem>? compatibleFrameworkAsset = contentItemCollection.FindBestItemGroup(managedCriteria, patternSet)?.Items;
73-
if (compatibleFrameworkAsset != null)
69+
IList<ContentItem>? compatibleCompileTimeAsset = contentItemCollection.FindBestItemGroup(managedCriteria, patternSet)?.Items;
70+
if (compatibleCompileTimeAsset != null)
7471
{
7572
_apiCompatRunner.QueueApiCompatFromContentItem(_log,
76-
new ReadOnlyCollection<ContentItem>(compatibleFrameworkAsset),
73+
new ReadOnlyCollection<ContentItem>(compatibleCompileTimeAsset),
7774
compileTimeAsset,
7875
apiCompatOptions,
7976
options.Package);

0 commit comments

Comments
 (0)