From 6ef7d6c40f140b205e97a5627e110a56ec64f882 Mon Sep 17 00:00:00 2001 From: Drew Gillies Date: Mon, 12 Apr 2021 13:43:53 +1000 Subject: [PATCH] Addressed feedback, added silent fail on TFM formatting errors --- src/GalleryTools/Commands/BackfillCommand.cs | 2 +- .../Commands/BackfillTfmMetadataCommand.cs | 45 ++++++++++++++++--- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/GalleryTools/Commands/BackfillCommand.cs b/src/GalleryTools/Commands/BackfillCommand.cs index e92028c2b8..f051d43321 100644 --- a/src/GalleryTools/Commands/BackfillCommand.cs +++ b/src/GalleryTools/Commands/BackfillCommand.cs @@ -125,7 +125,7 @@ public async Task Collect(SqlConnection connection, Uri serviceDiscoveryUri, Dat packages = packages .Where(p => p.Created < lastCreateTime && p.Created > startTime) - .Where(p => p.PackageStatusKey == PackageStatus.Available || p.PackageStatusKey == PackageStatus.Validating) + .Where(p => p.PackageStatusKey == PackageStatus.Available) .OrderBy(p => p.Created); if (LimitTo > 0) { diff --git a/src/GalleryTools/Commands/BackfillTfmMetadataCommand.cs b/src/GalleryTools/Commands/BackfillTfmMetadataCommand.cs index d4f0720120..e8a1e55564 100644 --- a/src/GalleryTools/Commands/BackfillTfmMetadataCommand.cs +++ b/src/GalleryTools/Commands/BackfillTfmMetadataCommand.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Linq.Expressions; using Microsoft.Extensions.CommandLineUtils; +using NuGet.Frameworks; using NuGet.Packaging; using NuGet.Services.Entities; using NuGetGallery; @@ -30,10 +31,34 @@ public static void Configure(CommandLineApplication config) protected override List ReadMetadata(IList files, NuspecReader nuspecReader) { var supportedTFMs = new List(); - var supportedFrameworks = _packageService.GetSupportedFrameworks(nuspecReader, files); + + // We wrap this int a try catch because fetching supported frameworks on existing packages can sometimes throw due to format errors + // (usually portable TFM formatting). In this case we'll clear the TFMs. + var supportedFrameworks = Enumerable.Empty(); + try + { + supportedFrameworks = _packageService.GetSupportedFrameworks(nuspecReader, files); + } + catch (Exception e) + { + // do nothing--this is a known scenario and we'll skip this package quietly, which will give us a more useful error log file + } + foreach (var tfm in supportedFrameworks) { - supportedTFMs.Add(tfm.GetShortFolderName()); + // We wrap this in a try-catch because some poorly-crafted portable TFMs will make it through GetSupportedFrameworks and fail here, e.g. for a + // non-existent profile name like "Profile1", which will cause GetShortFolderName to throw. We want to fail silently for these as this is a known + // scenario (more useful error log) and not failing will allow us to still capture all of the valid TFMs. + // See https://github.com/NuGet/NuGet.Client/blob/ba008e14611f4fa518c2d02ed78dfe5969e4a003/src/NuGet.Core/NuGet.Frameworks/NuGetFramework.cs#L297 + // See https://github.com/NuGet/NuGet.Client/blob/ba008e14611f4fa518c2d02ed78dfe5969e4a003/src/NuGet.Core/NuGet.Frameworks/FrameworkNameProvider.cs#L487 } + try + { + supportedTFMs.Add(tfm.ToShortNameOrNull()); + } + catch + { + // skip this TFM and only collect well-formatted ones + } } return supportedTFMs; @@ -48,9 +73,19 @@ protected override void ConfigureClassMap(PackageMetadataClassMap map) protected override void UpdatePackage(Package package, List metadata, EntitiesContext context) { - var existingTFMs = package.SupportedFrameworks == null - ? Enumerable.Empty() - : package.SupportedFrameworks.Select(f => f.FrameworkName.GetShortFolderName()).OrderBy(f => f); + // Note that extracting old TFMs may throw for formatting reasons. In this case we'll force a full replacement by leaving the collection empty. + var existingTFMs = Enumerable.Empty(); + if (package.SupportedFrameworks != null) + { + try + { + existingTFMs = package.SupportedFrameworks.Select(f => f.FrameworkName.GetShortFolderName()).OrderBy(f => f); + } + catch + { + // do nothing and replace in full + } + } var newTFMs = metadata == null || metadata.Count == 0 ? Enumerable.Empty()