Skip to content

Commit

Permalink
Addressed feedback, added silent fail on TFM formatting errors
Browse files Browse the repository at this point in the history
  • Loading branch information
drewgillies committed Apr 13, 2021
1 parent e5aa6b1 commit 6ef7d6c
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/GalleryTools/Commands/BackfillCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
45 changes: 40 additions & 5 deletions src/GalleryTools/Commands/BackfillTfmMetadataCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -30,10 +31,34 @@ public static void Configure(CommandLineApplication config)
protected override List<string> ReadMetadata(IList<string> files, NuspecReader nuspecReader)
{
var supportedTFMs = new List<string>();
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<NuGetFramework>();
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;
Expand All @@ -48,9 +73,19 @@ protected override void ConfigureClassMap(PackageMetadataClassMap map)

protected override void UpdatePackage(Package package, List<string> metadata, EntitiesContext context)
{
var existingTFMs = package.SupportedFrameworks == null
? Enumerable.Empty<string>()
: 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<string>();
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<string>()
Expand Down

0 comments on commit 6ef7d6c

Please sign in to comment.