Skip to content

Commit 874b4b6

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 0d7a9fe commit 874b4b6

20 files changed

+193
-50
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.DotNet.ApiCompatibility;
66
using Microsoft.DotNet.ApiCompatibility.Logging;
77
using Microsoft.DotNet.ApiCompatibility.Runner;
8+
using Microsoft.DotNet.ApiSymbolExtensions.Logging;
89
using NuGet.ContentModel;
910
using NuGet.Frameworks;
1011

@@ -31,6 +32,19 @@ public static void QueueApiCompatFromContentItem(this IApiCompatRunner apiCompat
3132
return;
3233
}
3334

35+
// Don't perform api compatibility checks on placeholder files.
36+
if (leftContentItems.IsPlaceholderFile() && rightContentItems.IsPlaceholderFile())
37+
{
38+
leftContentItems[0].Properties.TryGetValue("tfm", out object? tfmRaw);
39+
string tfm = tfmRaw?.ToString() ?? string.Empty;
40+
41+
log.LogMessage(MessageImportance.Normal, string.Format(Resources.SkipApiCompatForPlaceholderFiles, tfm));
42+
}
43+
44+
// Make sure placeholder package files aren't enqueued as api comparison check work items.
45+
Debug.Assert(!leftContentItems.IsPlaceholderFile() && !rightContentItems.IsPlaceholderFile(),
46+
"Placeholder files must not be enqueued for api comparison checks.");
47+
3448
// If a right hand side package is provided in addition to the left side, package validation runs in baseline comparison mode.
3549
// The assumption stands that the right hand side is "current" and has more information than the left, i.e. assembly references.
3650
// If the left package doesn't provide assembly references, fallback to the references from the right side if the TFMs are compatible.

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using NuGet.ContentModel;
77
using NuGet.Frameworks;
88
using NuGet.Packaging;
9-
using NuGet.Packaging.Core;
109
using NuGet.RuntimeModel;
1110

1211
namespace Microsoft.DotNet.PackageValidation
@@ -136,7 +135,7 @@ public static Package Create(string? packagePath, IReadOnlyDictionary<NuGetFrame
136135
NuspecReader nuspecReader = packageReader.NuspecReader;
137136
string packageId = nuspecReader.GetId();
138137
string version = nuspecReader.GetVersion().ToString();
139-
IEnumerable<string> packageAssets = packageReader.GetFiles().Where(t => t.EndsWith(".dll")).ToArray();
138+
IEnumerable<string> packageAssets = packageReader.GetFiles().ToArray();
140139

141140
return new Package(packagePath!, packageId, version, packageAssets, packageAssemblyReferences);
142141
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using NuGet.ContentModel;
45
using NuGet.Frameworks;
56

67
namespace Microsoft.DotNet.PackageValidation
78
{
89
internal static class PackageExtensions
910
{
11+
private const string PlaceholderFile = "_._";
12+
13+
public static bool IsPlaceholderFile(this ContentItem contentItem) =>
14+
Path.GetFileName(contentItem.Path) == PlaceholderFile;
15+
16+
public static bool IsPlaceholderFile(this IReadOnlyList<ContentItem> contentItems) =>
17+
contentItems.Count == 1 && contentItems[0].IsPlaceholderFile();
18+
1019
public static bool SupportsRuntimeIdentifier(this NuGetFramework tfm, string rid) =>
1120
tfm.Framework != ".NETFramework" || rid.StartsWith("win", StringComparison.OrdinalIgnoreCase);
1221
}

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

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<root>
3-
<!--
4-
Microsoft ResX Schema
5-
3+
<!--
4+
Microsoft ResX Schema
5+
66
Version 2.0
7-
8-
The primary goals of this format is to allow a simple XML format
9-
that is mostly human readable. The generation and parsing of the
10-
various data types are done through the TypeConverter classes
7+
8+
The primary goals of this format is to allow a simple XML format
9+
that is mostly human readable. The generation and parsing of the
10+
various data types are done through the TypeConverter classes
1111
associated with the data types.
12-
12+
1313
Example:
14-
14+
1515
... ado.net/XML headers & schema ...
1616
<resheader name="resmimetype">text/microsoft-resx</resheader>
1717
<resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
2626
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
2727
<comment>This is a comment</comment>
2828
</data>
29-
30-
There are any number of "resheader" rows that contain simple
29+
30+
There are any number of "resheader" rows that contain simple
3131
name/value pairs.
32-
33-
Each data row contains a name, and value. The row also contains a
34-
type or mimetype. Type corresponds to a .NET class that support
35-
text/value conversion through the TypeConverter architecture.
36-
Classes that don't support this are serialized and stored with the
32+
33+
Each data row contains a name, and value. The row also contains a
34+
type or mimetype. Type corresponds to a .NET class that support
35+
text/value conversion through the TypeConverter architecture.
36+
Classes that don't support this are serialized and stored with the
3737
mimetype set.
38-
39-
The mimetype is used for serialized objects, and tells the
40-
ResXResourceReader how to depersist the object. This is currently not
38+
39+
The mimetype is used for serialized objects, and tells the
40+
ResXResourceReader how to depersist the object. This is currently not
4141
extensible. For a given mimetype the value must be set accordingly:
42-
43-
Note - application/x-microsoft.net.object.binary.base64 is the format
44-
that the ResXResourceWriter will generate, however the reader can
42+
43+
Note - application/x-microsoft.net.object.binary.base64 is the format
44+
that the ResXResourceWriter will generate, however the reader can
4545
read any of the formats listed below.
46-
46+
4747
mimetype: application/x-microsoft.net.object.binary.base64
48-
value : The object must be serialized with
48+
value : The object must be serialized with
4949
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
5050
: and then encoded with base64 encoding.
51-
51+
5252
mimetype: application/x-microsoft.net.object.soap.base64
53-
value : The object must be serialized with
53+
value : The object must be serialized with
5454
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
5555
: and then encoded with base64 encoding.
5656
5757
mimetype: application/x-microsoft.net.object.bytearray.base64
58-
value : The object must be serialized into a byte array
58+
value : The object must be serialized into a byte array
5959
: using a System.ComponentModel.TypeConverter
6060
: and then encoded with base64 encoding.
6161
-->
@@ -150,4 +150,7 @@
150150
<data name="BaselineTargetFrameworkIgnoredButPresentInCurrentPackage" xml:space="preserve">
151151
<value>Target framework '{0}' in the baseline package is ignored but exists in the current package.</value>
152152
</data>
153-
</root>
153+
<data name="SkipApiCompatForPlaceholderFiles" xml:space="preserve">
154+
<value>Skipping api compatiblity check for target framework {0} as the package assets are placeholder files.</value>
155+
</data>
156+
</root>

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

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,21 @@ public void Validate(PackageValidatorOption options)
4343
{
4444
// Search for compatible compile time assets in the latest package.
4545
IReadOnlyList<ContentItem>? latestCompileAssets = options.Package.FindBestCompileAssetForFramework(baselineTargetFramework);
46-
if (latestCompileAssets == null)
46+
// Emit an error if
47+
// - No latest compile time asset is available or
48+
// - The latest compile time asset is a placeholder but the baseline compile time asset isn't.
49+
if (latestCompileAssets == null ||
50+
(latestCompileAssets.IsPlaceholderFile() && !baselineCompileAssets.IsPlaceholderFile()))
4751
{
4852
log.LogError(new Suppression(DiagnosticIds.TargetFrameworkDropped) { Target = baselineTargetFramework.ToString() },
4953
DiagnosticIds.TargetFrameworkDropped,
5054
string.Format(Resources.MissingTargetFramework,
5155
baselineTargetFramework));
5256
}
57+
else if (baselineCompileAssets.IsPlaceholderFile() && !latestCompileAssets.IsPlaceholderFile())
58+
{
59+
// Ignore the newly added compile time asset in the latest package.
60+
}
5361
else if (options.EnqueueApiCompatWorkItems)
5462
{
5563
apiCompatRunner.QueueApiCompatFromContentItem(log,
@@ -67,13 +75,21 @@ public void Validate(PackageValidatorOption options)
6775
{
6876
// Search for compatible runtime assets in the latest package.
6977
IReadOnlyList<ContentItem>? latestRuntimeAssets = options.Package.FindBestRuntimeAssetForFramework(baselineTargetFramework);
70-
if (latestRuntimeAssets == null)
78+
// Emit an error if
79+
// - No latest runtime asset is available or
80+
// - The latest runtime asset is a placeholder but the baseline runtime asset isn't.
81+
if (latestRuntimeAssets == null ||
82+
(latestRuntimeAssets.IsPlaceholderFile() && !baselineRuntimeAssets.IsPlaceholderFile()))
7183
{
7284
log.LogError(new Suppression(DiagnosticIds.TargetFrameworkDropped) { Target = baselineTargetFramework.ToString() },
7385
DiagnosticIds.TargetFrameworkDropped,
7486
string.Format(Resources.MissingTargetFramework,
7587
baselineTargetFramework));
7688
}
89+
else if (baselineRuntimeAssets.IsPlaceholderFile() && !latestRuntimeAssets.IsPlaceholderFile())
90+
{
91+
// Ignore the newly added run time asset in the latest package.
92+
}
7793
else if (options.EnqueueApiCompatWorkItems)
7894
{
7995
apiCompatRunner.QueueApiCompatFromContentItem(log,
@@ -95,19 +111,28 @@ public void Validate(PackageValidatorOption options)
95111

96112
foreach (IGrouping<string, ContentItem> baselineRuntimeSpecificAssetsRidGroup in baselineRuntimeSpecificAssetsRidGroupedPerRid)
97113
{
114+
IReadOnlyList<ContentItem> baselineRuntimeSpecificAssetsForRid = baselineRuntimeSpecificAssetsRidGroup.ToArray();
98115
IReadOnlyList<ContentItem>? latestRuntimeSpecificAssets = options.Package.FindBestRuntimeAssetForFrameworkAndRuntime(baselineTargetFramework, baselineRuntimeSpecificAssetsRidGroup.Key);
99-
if (latestRuntimeSpecificAssets == null)
116+
// Emit an error if
117+
// - No latest runtime specific asset is available or
118+
// - The latest runtime specific asset is a placeholder but the baseline runtime specific asset isn't.
119+
if (latestRuntimeSpecificAssets == null ||
120+
(latestRuntimeSpecificAssets.IsPlaceholderFile() && !baselineRuntimeSpecificAssetsForRid.IsPlaceholderFile()))
100121
{
101122
log.LogError(new Suppression(DiagnosticIds.TargetFrameworkAndRidPairDropped) { Target = baselineTargetFramework.ToString() + "-" + baselineRuntimeSpecificAssetsRidGroup.Key },
102123
DiagnosticIds.TargetFrameworkAndRidPairDropped,
103124
string.Format(Resources.MissingTargetFrameworkAndRid,
104125
baselineTargetFramework,
105126
baselineRuntimeSpecificAssetsRidGroup.Key));
106127
}
128+
else if (baselineRuntimeSpecificAssetsForRid.IsPlaceholderFile() && !latestRuntimeSpecificAssets.IsPlaceholderFile())
129+
{
130+
// Ignore the newly added runtime specific asset in the latest package.
131+
}
107132
else if (options.EnqueueApiCompatWorkItems)
108133
{
109134
apiCompatRunner.QueueApiCompatFromContentItem(log,
110-
baselineRuntimeSpecificAssetsRidGroup.ToArray(),
135+
baselineRuntimeSpecificAssetsForRid,
111136
latestRuntimeSpecificAssets,
112137
apiCompatOptions,
113138
options.BaselinePackage,

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

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,29 +40,28 @@ public void Validate(PackageValidatorOption options)
4040
foreach (NuGetFramework framework in options.Package.FrameworksInPackage.OrderByDescending(f => f.Version))
4141
{
4242
IReadOnlyList<ContentItem>? compileTimeAsset = options.Package.FindBestCompileAssetForFramework(framework);
43-
if (compileTimeAsset != null)
43+
if (compileTimeAsset != null && !compileTimeAsset.IsPlaceholderFile())
44+
{
4445
compileAssetsQueue.Enqueue((framework, compileTimeAsset));
46+
}
4547
}
4648

47-
while (compileAssetsQueue.Count > 0)
49+
// Iterate as long as assets are available for comparison.
50+
while (compileAssetsQueue.Count > 1)
4851
{
4952
(NuGetFramework framework, IReadOnlyList<ContentItem> compileTimeAsset) = compileAssetsQueue.Dequeue();
5053

51-
// If no assets are available for comparison, stop the iteration.
52-
if (compileAssetsQueue.Count == 0) break;
53-
5454
SelectionCriteria managedCriteria = conventions.Criteria.ForFramework(framework);
55-
5655
ContentItemCollection contentItemCollection = new();
5756
// The collection won't contain the current compile time asset as it is already dequeued.
5857
contentItemCollection.Load(compileAssetsQueue.SelectMany(a => a.Item2).Select(a => a.Path));
5958

6059
// Search for a compatible compile time asset and compare it.
61-
IList<ContentItem>? compatibleFrameworkAsset = contentItemCollection.FindBestItemGroup(managedCriteria, patternSet)?.Items;
62-
if (compatibleFrameworkAsset != null)
60+
IList<ContentItem>? compatibleCompileTimeAsset = contentItemCollection.FindBestItemGroup(managedCriteria, patternSet)?.Items;
61+
if (compatibleCompileTimeAsset != null)
6362
{
6463
apiCompatRunner.QueueApiCompatFromContentItem(log,
65-
new ReadOnlyCollection<ContentItem>(compatibleFrameworkAsset),
64+
new ReadOnlyCollection<ContentItem>(compatibleCompileTimeAsset),
6665
compileTimeAsset,
6766
apiCompatOptions,
6867
options.Package);

0 commit comments

Comments
 (0)