From 2922659420c074a06253c188e8e8284fa194aa96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Mon, 12 Oct 2020 14:29:33 -0700 Subject: [PATCH 01/13] error when missing tpv during restore --- .../RestoreCommand/RestoreCommand.cs | 28 +++++++- .../NuGet.Commands/Strings.Designer.cs | 9 +++ src/NuGet.Core/NuGet.Commands/Strings.resx | 3 + .../NuGet.Common/Errors/NuGetLogCode.cs | 17 +++-- .../PublicAPI/net45/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net472/PublicAPI.Unshipped.txt | 1 + .../netstandard2.0/PublicAPI.Unshipped.txt | 1 + .../RestoreCommandTests.cs | 66 +++++++++++++++++++ 8 files changed, 117 insertions(+), 9 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index c0b5b8bfe9b..571733f3229 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -181,6 +181,8 @@ public async Task ExecuteAsync(CancellationToken token) _success = false; } + _success &= HasValidPlatformVersions(); + // evaluate packages.lock.json file var packagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); var isLockFileValid = false; @@ -404,6 +406,26 @@ public async Task ExecuteAsync(CancellationToken token) } } + private bool HasValidPlatformVersions() + { + IEnumerable badPlatforms = _request.Project.TargetFrameworks + .Select(frameworkInfo => frameworkInfo.FrameworkName) + .Where(framework => !string.IsNullOrEmpty(framework.Platform) && (framework.PlatformVersion == FrameworkConstants.EmptyVersion)); + + if (badPlatforms.Any()) + { + _logger.Log(RestoreLogMessage.CreateError( + NuGetLogCode.NU1012, + string.Format(CultureInfo.CurrentCulture, Strings.Error_PlatformVersionNotPresent, string.Join(", ", badPlatforms)) + )); + return false; + } + else + { + return true; + } + } + private async Task AreCentralVersionRequirementsSatisfiedAsync() { // The dependencies should not have versions explicitelly defined if cpvm is enabled. @@ -627,7 +649,7 @@ private bool ValidatePackagesSha512(PackagesLockFile lockFile, LockFile assetsFi { if (!noOp) { - // Clean up to preserve the pre no-op behavior. This should not be used, but we want to be cautious. + // Clean up to preserve the pre no-op behavior. This should not be used, but we want to be cautious. _request.LockFilePath = null; _request.Project.RestoreMetadata.CacheFilePath = null; } @@ -727,7 +749,7 @@ private LockFile BuildAssetsFile( /// /// Check if the given graphs are valid and log errors/warnings. /// If fatal errors are encountered the rest of the errors/warnings - /// are not logged. This is to avoid flooding the log with long + /// are not logged. This is to avoid flooding the log with long /// dependency chains for every package. /// private async Task ValidateRestoreGraphsAsync(IEnumerable graphs, ILogger logger) @@ -1108,7 +1130,7 @@ private List GetProjectReferences(RemoteWalkContext co else { // External references were passed, but the top level project wasn't found. - // This is always due to an internal issue and typically caused by errors + // This is always due to an internal issue and typically caused by errors // building the project closure. Debug.Fail("RestoreRequest.ExternalProjects contains references, but does not contain the top level references. Add the project we are restoring for."); throw new InvalidOperationException($"Missing external reference metadata for {_request.Project.Name}"); diff --git a/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs b/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs index ef8dac6a044..32beb2309a8 100644 --- a/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs @@ -672,6 +672,15 @@ internal static string Error_PackFailed { } } + /// + /// Looks up a localized string similar to Platform is not present for one or more target framework(s): {0}. + /// + internal static string Error_PlatformVersionNotPresent { + get { + return ResourceManager.GetString("Error_PlatformVersionNotPresent", resourceCulture); + } + } + /// /// Looks up a localized string similar to Error occurred when processing file '{0}': {1}. /// diff --git a/src/NuGet.Core/NuGet.Commands/Strings.resx b/src/NuGet.Core/NuGet.Commands/Strings.resx index 6fe849ff6aa..066c639a374 100644 --- a/src/NuGet.Core/NuGet.Commands/Strings.resx +++ b/src/NuGet.Core/NuGet.Commands/Strings.resx @@ -1114,4 +1114,7 @@ For more information, visit https://docs.nuget.org/docs/reference/command-line-r {0} contains more than one path + + Platform is not present for one or more target framework(s): {0} + \ No newline at end of file diff --git a/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs b/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs index bb8068e5d7b..ea0c78d6c10 100644 --- a/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs +++ b/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs @@ -4,18 +4,18 @@ namespace NuGet.Common { /// - /// This enum is used to quantify NuGet error and warning codes. + /// This enum is used to quantify NuGet error and warning codes. /// Format - NUxyzw where NU is the prefix indicating NuGet and xyzw is a 4 digit code /// /// Numbers - xyzw /// x - 'x' is the largest digit and should be used to quantify a set of errors. /// For example 1yzw are set of restore related errors and no other code path should use the range 1000 to 1999 for errors or warnings. - /// + /// /// y - 'y' is the second largest digit and should be used for sub sections withing a broad category. - /// + /// /// For example 12zw cvould be http related errors. /// Further 'y' = 0-4 should be used for errors and 'y' = 5-9 should be warnings. - /// + /// /// zw - 'zw' are the least two digit. /// These could be used for different errors or warnings within the broad categories set by digits 'xy'. /// @@ -31,9 +31,9 @@ namespace NuGet.Common /// 1200/1700 - Compat /// 1300/1800 - Feed /// 1400/1900 - Package - /// + /// /// All new codes need a corresponding MarkDown file under https://github.com/NuGet/docs.microsoft.com-nuget/tree/master/docs/reference/errors-and-warnings. - /// + /// /// public enum NuGetLogCode { @@ -102,6 +102,11 @@ public enum NuGetLogCode /// NU1011 = 1011, + /// + /// Platform version not found. + /// + NU1012 = 1012, + /// /// Unable to resolve package, generic message for unknown type constraints. /// diff --git a/src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt index ea407d9c998..27e57301d51 100644 --- a/src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt @@ -1,3 +1,4 @@ NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode +NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode diff --git a/src/NuGet.Core/NuGet.Common/PublicAPI/net472/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Common/PublicAPI/net472/PublicAPI.Unshipped.txt index ea407d9c998..27e57301d51 100644 --- a/src/NuGet.Core/NuGet.Common/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Common/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,3 +1,4 @@ NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode +NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode diff --git a/src/NuGet.Core/NuGet.Common/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/NuGet.Core/NuGet.Common/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index ea407d9c998..27e57301d51 100644 --- a/src/NuGet.Core/NuGet.Common/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Common/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,3 +1,4 @@ NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU1011 = 1011 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU5501 = 5501 -> NuGet.Common.NuGetLogCode +NuGet.Common.NuGetLogCode.NU1012 = 1012 -> NuGet.Common.NuGetLogCode diff --git a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs index 9b1e57c70ae..15ec779a484 100644 --- a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs +++ b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs @@ -10,6 +10,7 @@ using FluentAssertions; using Newtonsoft.Json.Linq; using NuGet.Commands.Test; +using NuGet.Common; using NuGet.Configuration; using NuGet.Frameworks; using NuGet.LibraryModel; @@ -2989,6 +2990,71 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } } + [Fact] + public async Task RestoreCommand_WhenPlatformVersionIsEmpty_ThrowsError() + { + using (var pathContext = new SimpleTestPathContext()) + using (var context = new SourceCacheContext()) + { + var configJson = JObject.Parse(@" + { + ""frameworks"": { + ""net5.0-windows"": { + ""dependencies"": { + ""A"": { + ""version"" : ""1.0.0"", + } + } + } + } + }"); + + // Arrange + var packageA = new SimpleTestPackageContext("a", "1.0.0"); + packageA.Files.Clear(); + packageA.AddFile("lib/net5.0-windows/a.dll"); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + packageA); + + var sources = new List + { + new PackageSource(pathContext.PackageSource) + }; + var logger = new TestLogger(); + + var projectDirectory = Path.Combine(pathContext.SolutionRoot, "TestProject"); + var cachingSourceProvider = new CachingSourceProvider(new PackageSourceProvider(NullSettings.Instance)); + + var spec = JsonPackageSpecReader.GetPackageSpec(configJson.ToString(), "TestProject", Path.Combine(projectDirectory, "project.csproj")).WithTestRestoreMetadata(); + var dgSpec = new DependencyGraphSpec(); + dgSpec.AddProject(spec); + dgSpec.AddRestore(spec.Name); + + var request = new TestRestoreRequest(spec, sources, pathContext.UserPackagesFolder, logger) + { + ProjectStyle = ProjectStyle.PackageReference, + DependencyGraphSpec = dgSpec, + AllowNoOp = true, + }; + var command = new RestoreCommand(request); + + // Act + var result = await command.ExecuteAsync(); + await result.CommitAsync(logger, CancellationToken.None); + + // Assert + Assert.False(result.Success); + Assert.Equal(1, logger.ErrorMessages.Count); + logger.ErrorMessages.TryDequeue(out var errorMessage); + Assert.True(errorMessage.Contains("platform version")); + var messagesForNU1012 = result.LockFile.LogMessages.Where(m => m.Code == NuGetLogCode.NU1012); + Assert.Equal(1, messagesForNU1012.Count()); + } + } + private static byte[] GetTestUtilityResource(string name) { return ResourceTestUtility.GetResourceBytes( From dbd5e0ffef63fa89a76e31e77d9e01b3c5abd7c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Fri, 16 Oct 2020 10:12:00 -0700 Subject: [PATCH 02/13] error when missing tpv during pack --- .../NuGet.Packaging/Rules/RuleSet.cs | 1 + ...rgetFrameworkMissingPlatformVersionRule.cs | 172 ++++++++++++++++++ .../NuGet.Packaging/Strings.Designer.cs | 45 +++++ src/NuGet.Core/NuGet.Packaging/Strings.resx | 17 +- 4 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 src/NuGet.Core/NuGet.Packaging/Rules/TargetFrameworkMissingPlatformVersionRule.cs diff --git a/src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs b/src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs index ddd07f1caa2..7bdc3c29ca5 100644 --- a/src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs +++ b/src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs @@ -30,6 +30,7 @@ public static class RuleSet new UpholdBuildConventionRule(), new ReferencesInNuspecMatchRefAssetsRule(), new InvalidUndottedFrameworkRule(), + new TargetFrameworkMissingPlatformVersionRule(), new IconUrlDeprecationWarning(AnalysisResources.IconUrlDeprecationWarning), } ); diff --git a/src/NuGet.Core/NuGet.Packaging/Rules/TargetFrameworkMissingPlatformVersionRule.cs b/src/NuGet.Core/NuGet.Packaging/Rules/TargetFrameworkMissingPlatformVersionRule.cs new file mode 100644 index 00000000000..c56834d63ee --- /dev/null +++ b/src/NuGet.Core/NuGet.Packaging/Rules/TargetFrameworkMissingPlatformVersionRule.cs @@ -0,0 +1,172 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Xml; +using System.Xml.Linq; +using NuGet.Client; +using NuGet.Common; +using NuGet.ContentModel; +using NuGet.Frameworks; +using NuGet.Packaging.Core; +using NuGet.RuntimeModel; + +namespace NuGet.Packaging.Rules +{ + internal class TargetFrameworkMissingPlatformVersionRule : IPackageRule + { + // NOTE: We never actually generate messages, only error on failure. + public string MessageFormat => ""; + + public TargetFrameworkMissingPlatformVersionRule() + { + } + + public IEnumerable Validate(PackageArchiveReader builder) + { + return Validate(new NuspecReader(builder.GetNuspec()), builder.GetFiles()); + } + + internal static IEnumerable Validate(NuspecReader reader, IEnumerable files) + { + var bads = (new string[] + { + ValidateDependencyGroups(reader), + ValidateReferenceGroups(reader), + ValidateFrameworkAssemblies(reader), + ValidateFiles(files) + }).Where(err => err != null); + + if (bads.Any()) + { + string items = string.Empty; + foreach (string badMessage in bads) + { + items += "\n- " + badMessage; + } + // PackagingLogMessage technically has an Error level, but + // that doesn't look like it fails pack, which is what we need + // to do in this case. + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersions, items)); + } + else + { + return new List(); + } + } + + internal static string ValidateDependencyGroups(NuspecReader reader) + { + var bads = new HashSet(reader.GetDependencyGroups() + .Select(group => group.TargetFramework) + .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) + .Select(framework => framework.GetShortFolderName())); + if (bads.Any()) + { + return string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromDependencyGroups, string.Join(", ", bads)); + } + else + { + return null; + } + } + + internal static string ValidateReferenceGroups(NuspecReader reader) + { + var bads = new HashSet(reader.GetReferenceGroups() + .Select(group => group.TargetFramework) + .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) + .Select(framework => framework.GetShortFolderName())); + if (bads.Any()) + { + return string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromReferenceGroups, string.Join(", ", bads)); + } + else + { + return null; + } + } + + internal static string ValidateFrameworkAssemblies(NuspecReader reader) + { + var bads = new HashSet(reader.GetFrameworkAssemblyGroups() + .Select(group => group.TargetFramework) + .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) + .Select(framework => framework.GetShortFolderName())); + if (bads.Any()) + { + return string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyGroups, string.Join(", ", bads)); + } + else + { + return null; + } + } + + internal static string ValidateFiles(IEnumerable files) + { + var set = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var file in files.Select(t => PathUtility.GetPathWithDirectorySeparator(t))) + { + set.Add(file); + } + + var managedCodeConventions = new ManagedCodeConventions(new RuntimeGraph()); + var collection = new ContentItemCollection(); + collection.Load(set.Select(path => path.Replace('\\', '/')).ToArray()); + + var patterns = managedCodeConventions.Patterns; + + var frameworkPatterns = new List() + { + patterns.RuntimeAssemblies, + patterns.CompileRefAssemblies, + patterns.CompileLibAssemblies, + patterns.NativeLibraries, + patterns.ResourceAssemblies, + patterns.MSBuildFiles, + patterns.ContentFiles, + patterns.ToolsAssemblies, + patterns.EmbedAssemblies, + patterns.MSBuildTransitiveFiles + }; + var warnPaths = new HashSet(); + + var bads = new HashSet(); + foreach (var pattern in frameworkPatterns) + { + IEnumerable targetedItemGroups = ContentExtractor.GetContentForPattern(collection, pattern); + foreach (ContentItemGroup group in targetedItemGroups) + { + foreach (ContentItem item in group.Items) + { + var framework = (NuGetFramework)item.Properties["tfm"]; + if (framework == null) + { + continue; + } + + if (framework.HasPlatform && framework.PlatformVersion == FrameworkConstants.EmptyVersion) + { + bads.Add(framework.GetShortFolderName()); + } + } + } + } + + if (bads.Any()) + { + return string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromIncludedFiles, string.Join(", ", bads)); + } + else + { + return null; + } + } + } +} + diff --git a/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs b/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs index 8225a8209b6..b109c151b73 100644 --- a/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs @@ -789,6 +789,51 @@ internal static string MissingPackageTypeName { } } + /// + /// Looks up a localized string similar to Some package components lack platform versions in their TFMs: {0}. + /// + internal static string MissingTargetPlatformVersions { + get { + return ResourceManager.GetString("MissingTargetPlatformVersions", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Some dependency group TFMs are missing a platform version: {0}. + /// + internal static string MissingTargetPlatformVersionsFromDependencyGroups { + get { + return ResourceManager.GetString("MissingTargetPlatformVersionsFromDependencyGroups", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Some reference assembly group TFMs are missing a platform version: {0}. + /// + internal static string MissingTargetPlatformVersionsFromFrameworkAssemblyGroups { + get { + return ResourceManager.GetString("MissingTargetPlatformVersionsFromFrameworkAssemblyGroups", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Some included files are included under TFMs which are missing a platform version: {0}. + /// + internal static string MissingTargetPlatformVersionsFromIncludedFiles { + get { + return ResourceManager.GetString("MissingTargetPlatformVersionsFromIncludedFiles", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Some reference group TFMs are missing a platform version: {0}. + /// + internal static string MissingTargetPlatformVersionsFromReferenceGroups { + get { + return ResourceManager.GetString("MissingTargetPlatformVersionsFromReferenceGroups", resourceCulture); + } + } + /// /// Looks up a localized string similar to Multiple {0} attributes are not allowed.. /// diff --git a/src/NuGet.Core/NuGet.Packaging/Strings.resx b/src/NuGet.Core/NuGet.Packaging/Strings.resx index 45c41490349..a0118f18916 100644 --- a/src/NuGet.Core/NuGet.Packaging/Strings.resx +++ b/src/NuGet.Core/NuGet.Packaging/Strings.resx @@ -839,4 +839,19 @@ Valid from: The timestamp service responded with HTTP status code '{0}' ('{1}'). {0} is the httpResponse.StatusCode, {1} is the httpResponse.ReasonPhrase. - + + Some package components lack platform versions in their TFMs: {0} + + + Some dependency group TFMs are missing a platform version: {0} + + + Some reference assembly group TFMs are missing a platform version: {0} + + + Some included files are included under TFMs which are missing a platform version: {0} + + + Some reference group TFMs are missing a platform version: {0} + + \ No newline at end of file From f3832fc77ac7ad3f888bbecab400d6e9f6eb9ea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Mon, 19 Oct 2020 16:13:20 -0700 Subject: [PATCH 03/13] do the validation in PackageBuilder instead --- .../Authoring/PackageBuilder.cs | 94 +++++++++- .../NuGet.Packaging/Rules/RuleSet.cs | 1 - ...rgetFrameworkMissingPlatformVersionRule.cs | 172 ------------------ 3 files changed, 93 insertions(+), 174 deletions(-) delete mode 100644 src/NuGet.Core/NuGet.Packaging/Rules/TargetFrameworkMissingPlatformVersionRule.cs diff --git a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs index 37d151cb480..493da2eef75 100644 --- a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs +++ b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs @@ -12,11 +12,14 @@ using System.Reflection; using System.Reflection.Emit; using System.Xml.Linq; +using NuGet.Client; using NuGet.Common; +using NuGet.ContentModel; using NuGet.Frameworks; using NuGet.Packaging.Core; using NuGet.Packaging.PackageCreation.Resources; using NuGet.Packaging.Rules; +using NuGet.RuntimeModel; using NuGet.Versioning; namespace NuGet.Packaging @@ -226,7 +229,7 @@ public ISet Tags } /// - /// Exposes the additional properties extracted by the metadata + /// Exposes the additional properties extracted by the metadata /// extractor or received from the command line. /// public Dictionary Properties @@ -377,8 +380,10 @@ public void Save(Stream stream) ValidateDependencies(Version, DependencyGroups); ValidateReferenceAssemblies(Files, PackageAssemblyReferences); + ValidateFrameworkAssemblies(FrameworkReferenceGroups); ValidateLicenseFile(Files, LicenseMetadata); ValidateIconFile(Files, Icon); + ValidateFileFrameworks(Files); using (var package = new ZipArchive(stream, ZipArchiveMode.Create, leaveOpen: true)) { @@ -543,6 +548,15 @@ private static bool HasXdtTransformFile(ICollection contentFiles) private static void ValidateDependencies(SemanticVersion version, IEnumerable dependencies) { + var bads = new HashSet(dependencies + .Select(group => group.TargetFramework) + .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) + .Select(framework => framework.GetShortFolderName())); + if (bads.Any()) + { + throw new PackagingException(NuGetLogCode.NU1012, String.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromDependencyGroups, string.Join(", ", bads))); + } + if (version == null) { // We have independent validation for null-versions. @@ -557,6 +571,15 @@ private static void ValidateDependencies(SemanticVersion version, public static void ValidateReferenceAssemblies(IEnumerable files, IEnumerable packageAssemblyReferences) { + var bads = new HashSet(packageAssemblyReferences + .Select(group => group.TargetFramework) + .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) + .Select(framework => framework.GetShortFolderName())); + if (bads.Any()) + { + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromReferenceGroups, string.Join(", ", bads))); + } + var libFiles = new HashSet(from file in files where !String.IsNullOrEmpty(file.Path) && file.Path.StartsWith("lib", StringComparison.OrdinalIgnoreCase) select Path.GetFileName(file.Path), StringComparer.OrdinalIgnoreCase); @@ -573,6 +596,18 @@ public static void ValidateReferenceAssemblies(IEnumerable files, } } + private static void ValidateFrameworkAssemblies(IEnumerable packageFrameworkAssemblies) + { + var bads = new HashSet(packageFrameworkAssemblies + .Select(group => group.TargetFramework) + .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) + .Select(framework => framework.GetShortFolderName())); + if (bads.Any()) + { + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyGroups, string.Join(", ", bads))); + } + } + private void ValidateLicenseFile(IEnumerable files, LicenseMetadata licenseMetadata) { if (!PackageTypes.Contains(PackageType.SymbolsPackage) && licenseMetadata?.Type == LicenseType.File) @@ -649,6 +684,63 @@ private void ValidateIconFile(IEnumerable files, string iconPath) } } + private static void ValidateFileFrameworks(IEnumerable files) + { + var set = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var file in files.Select(t => PathUtility.GetPathWithDirectorySeparator(t.Path))) + { + set.Add(file); + } + + var managedCodeConventions = new ManagedCodeConventions(new RuntimeGraph()); + var collection = new ContentItemCollection(); + collection.Load(set.Select(path => path.Replace('\\', '/')).ToArray()); + + var patterns = managedCodeConventions.Patterns; + + var frameworkPatterns = new List() + { + patterns.RuntimeAssemblies, + patterns.CompileRefAssemblies, + patterns.CompileLibAssemblies, + patterns.NativeLibraries, + patterns.ResourceAssemblies, + patterns.MSBuildFiles, + patterns.ContentFiles, + patterns.ToolsAssemblies, + patterns.EmbedAssemblies, + patterns.MSBuildTransitiveFiles + }; + var warnPaths = new HashSet(); + + var bads = new HashSet(); + foreach (var pattern in frameworkPatterns) + { + IEnumerable targetedItemGroups = ContentExtractor.GetContentForPattern(collection, pattern); + foreach (ContentItemGroup group in targetedItemGroups) + { + foreach (ContentItem item in group.Items) + { + var framework = (NuGetFramework)item.Properties["tfm"]; + if (framework == null) + { + continue; + } + + if (framework.HasPlatform && framework.PlatformVersion == FrameworkConstants.EmptyVersion) + { + bads.Add(framework.GetShortFolderName()); + } + } + } + } + + if (bads.Any()) + { + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromIncludedFiles, string.Join(", ", bads))); + } + } + private void ReadManifest(Stream stream, string basePath, Func propertyProvider) { // Deserialize the document and extract the metadata diff --git a/src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs b/src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs index 7bdc3c29ca5..ddd07f1caa2 100644 --- a/src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs +++ b/src/NuGet.Core/NuGet.Packaging/Rules/RuleSet.cs @@ -30,7 +30,6 @@ public static class RuleSet new UpholdBuildConventionRule(), new ReferencesInNuspecMatchRefAssetsRule(), new InvalidUndottedFrameworkRule(), - new TargetFrameworkMissingPlatformVersionRule(), new IconUrlDeprecationWarning(AnalysisResources.IconUrlDeprecationWarning), } ); diff --git a/src/NuGet.Core/NuGet.Packaging/Rules/TargetFrameworkMissingPlatformVersionRule.cs b/src/NuGet.Core/NuGet.Packaging/Rules/TargetFrameworkMissingPlatformVersionRule.cs deleted file mode 100644 index c56834d63ee..00000000000 --- a/src/NuGet.Core/NuGet.Packaging/Rules/TargetFrameworkMissingPlatformVersionRule.cs +++ /dev/null @@ -1,172 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Globalization; -using System.IO; -using System.Linq; -using System.Xml; -using System.Xml.Linq; -using NuGet.Client; -using NuGet.Common; -using NuGet.ContentModel; -using NuGet.Frameworks; -using NuGet.Packaging.Core; -using NuGet.RuntimeModel; - -namespace NuGet.Packaging.Rules -{ - internal class TargetFrameworkMissingPlatformVersionRule : IPackageRule - { - // NOTE: We never actually generate messages, only error on failure. - public string MessageFormat => ""; - - public TargetFrameworkMissingPlatformVersionRule() - { - } - - public IEnumerable Validate(PackageArchiveReader builder) - { - return Validate(new NuspecReader(builder.GetNuspec()), builder.GetFiles()); - } - - internal static IEnumerable Validate(NuspecReader reader, IEnumerable files) - { - var bads = (new string[] - { - ValidateDependencyGroups(reader), - ValidateReferenceGroups(reader), - ValidateFrameworkAssemblies(reader), - ValidateFiles(files) - }).Where(err => err != null); - - if (bads.Any()) - { - string items = string.Empty; - foreach (string badMessage in bads) - { - items += "\n- " + badMessage; - } - // PackagingLogMessage technically has an Error level, but - // that doesn't look like it fails pack, which is what we need - // to do in this case. - throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersions, items)); - } - else - { - return new List(); - } - } - - internal static string ValidateDependencyGroups(NuspecReader reader) - { - var bads = new HashSet(reader.GetDependencyGroups() - .Select(group => group.TargetFramework) - .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) - .Select(framework => framework.GetShortFolderName())); - if (bads.Any()) - { - return string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromDependencyGroups, string.Join(", ", bads)); - } - else - { - return null; - } - } - - internal static string ValidateReferenceGroups(NuspecReader reader) - { - var bads = new HashSet(reader.GetReferenceGroups() - .Select(group => group.TargetFramework) - .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) - .Select(framework => framework.GetShortFolderName())); - if (bads.Any()) - { - return string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromReferenceGroups, string.Join(", ", bads)); - } - else - { - return null; - } - } - - internal static string ValidateFrameworkAssemblies(NuspecReader reader) - { - var bads = new HashSet(reader.GetFrameworkAssemblyGroups() - .Select(group => group.TargetFramework) - .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) - .Select(framework => framework.GetShortFolderName())); - if (bads.Any()) - { - return string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyGroups, string.Join(", ", bads)); - } - else - { - return null; - } - } - - internal static string ValidateFiles(IEnumerable files) - { - var set = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var file in files.Select(t => PathUtility.GetPathWithDirectorySeparator(t))) - { - set.Add(file); - } - - var managedCodeConventions = new ManagedCodeConventions(new RuntimeGraph()); - var collection = new ContentItemCollection(); - collection.Load(set.Select(path => path.Replace('\\', '/')).ToArray()); - - var patterns = managedCodeConventions.Patterns; - - var frameworkPatterns = new List() - { - patterns.RuntimeAssemblies, - patterns.CompileRefAssemblies, - patterns.CompileLibAssemblies, - patterns.NativeLibraries, - patterns.ResourceAssemblies, - patterns.MSBuildFiles, - patterns.ContentFiles, - patterns.ToolsAssemblies, - patterns.EmbedAssemblies, - patterns.MSBuildTransitiveFiles - }; - var warnPaths = new HashSet(); - - var bads = new HashSet(); - foreach (var pattern in frameworkPatterns) - { - IEnumerable targetedItemGroups = ContentExtractor.GetContentForPattern(collection, pattern); - foreach (ContentItemGroup group in targetedItemGroups) - { - foreach (ContentItem item in group.Items) - { - var framework = (NuGetFramework)item.Properties["tfm"]; - if (framework == null) - { - continue; - } - - if (framework.HasPlatform && framework.PlatformVersion == FrameworkConstants.EmptyVersion) - { - bads.Add(framework.GetShortFolderName()); - } - } - } - } - - if (bads.Any()) - { - return string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromIncludedFiles, string.Join(", ", bads)); - } - else - { - return null; - } - } - } -} - From 0a83abdcb73b4612b331de6a711581aad1073495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Mon, 19 Oct 2020 16:37:21 -0700 Subject: [PATCH 04/13] review --- src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs | 9 --------- src/NuGet.Core/NuGet.Packaging/Strings.resx | 7 ++++--- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs b/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs index b109c151b73..e0dcfa15546 100644 --- a/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs @@ -789,15 +789,6 @@ internal static string MissingPackageTypeName { } } - /// - /// Looks up a localized string similar to Some package components lack platform versions in their TFMs: {0}. - /// - internal static string MissingTargetPlatformVersions { - get { - return ResourceManager.GetString("MissingTargetPlatformVersions", resourceCulture); - } - } - /// /// Looks up a localized string similar to Some dependency group TFMs are missing a platform version: {0}. /// diff --git a/src/NuGet.Core/NuGet.Packaging/Strings.resx b/src/NuGet.Core/NuGet.Packaging/Strings.resx index a0118f18916..5fe89dbc96e 100644 --- a/src/NuGet.Core/NuGet.Packaging/Strings.resx +++ b/src/NuGet.Core/NuGet.Packaging/Strings.resx @@ -839,19 +839,20 @@ Valid from: The timestamp service responded with HTTP status code '{0}' ('{1}'). {0} is the httpResponse.StatusCode, {1} is the httpResponse.ReasonPhrase. - - Some package components lack platform versions in their TFMs: {0} - Some dependency group TFMs are missing a platform version: {0} + 0 - comma-separated list of dependency group TFMs Some reference assembly group TFMs are missing a platform version: {0} + 0 - comma-separated list of reference assembly group TFMs Some included files are included under TFMs which are missing a platform version: {0} + 0 - comma-separated list of files with bad TFMs Some reference group TFMs are missing a platform version: {0} + 0 - comma-separated list of reference group TFMs \ No newline at end of file From 1b9d2548c406041d980788a45047d15a60108069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Mon, 19 Oct 2020 16:55:45 -0700 Subject: [PATCH 05/13] fix tests --- src/NuGet.Core/NuGet.Commands/Strings.Designer.cs | 2 +- src/NuGet.Core/NuGet.Commands/Strings.resx | 3 ++- .../NuGet.Commands.FuncTest/RestoreCommandTests.cs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs b/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs index 32beb2309a8..90571007e14 100644 --- a/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs @@ -673,7 +673,7 @@ internal static string Error_PackFailed { } /// - /// Looks up a localized string similar to Platform is not present for one or more target framework(s): {0}. + /// Looks up a localized string similar to Platform version is not present for one or more target frameworks, even though they have specified a platform: {0}. /// internal static string Error_PlatformVersionNotPresent { get { diff --git a/src/NuGet.Core/NuGet.Commands/Strings.resx b/src/NuGet.Core/NuGet.Commands/Strings.resx index 066c639a374..f4ed86956b5 100644 --- a/src/NuGet.Core/NuGet.Commands/Strings.resx +++ b/src/NuGet.Core/NuGet.Commands/Strings.resx @@ -1115,6 +1115,7 @@ For more information, visit https://docs.nuget.org/docs/reference/command-line-r {0} contains more than one path - Platform is not present for one or more target framework(s): {0} + Platform version is not present for one or more target frameworks, even though they have specified a platform: {0} + 0 - comma-separated list of TFMs missing a platform version \ No newline at end of file diff --git a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs index 15ec779a484..a0496becf86 100644 --- a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs +++ b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs @@ -3049,7 +3049,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( Assert.False(result.Success); Assert.Equal(1, logger.ErrorMessages.Count); logger.ErrorMessages.TryDequeue(out var errorMessage); - Assert.True(errorMessage.Contains("platform version")); + Assert.True(errorMessage.Contains("Platform version")); var messagesForNU1012 = result.LockFile.LogMessages.Where(m => m.Code == NuGetLogCode.NU1012); Assert.Equal(1, messagesForNU1012.Count()); } From a52deaeb868dbb6150a31b4221de7d2879daa45c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Mon, 19 Oct 2020 16:58:19 -0700 Subject: [PATCH 06/13] review --- .../Authoring/PackageBuilder.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs index 493da2eef75..88fe1c16db8 100644 --- a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs +++ b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs @@ -548,13 +548,13 @@ private static bool HasXdtTransformFile(ICollection contentFiles) private static void ValidateDependencies(SemanticVersion version, IEnumerable dependencies) { - var bads = new HashSet(dependencies + var frameworksMissingPlatformVersion = new HashSet(dependencies .Select(group => group.TargetFramework) .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) .Select(framework => framework.GetShortFolderName())); - if (bads.Any()) + if (frameworksMissingPlatformVersion.Any()) { - throw new PackagingException(NuGetLogCode.NU1012, String.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromDependencyGroups, string.Join(", ", bads))); + throw new PackagingException(NuGetLogCode.NU1012, String.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromDependencyGroups, string.Join(", ", frameworksMissingPlatformVersion))); } if (version == null) @@ -571,13 +571,13 @@ private static void ValidateDependencies(SemanticVersion version, public static void ValidateReferenceAssemblies(IEnumerable files, IEnumerable packageAssemblyReferences) { - var bads = new HashSet(packageAssemblyReferences + var frameworksMissingPlatformVersion = new HashSet(packageAssemblyReferences .Select(group => group.TargetFramework) .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) .Select(framework => framework.GetShortFolderName())); - if (bads.Any()) + if (frameworksMissingPlatformVersion.Any()) { - throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromReferenceGroups, string.Join(", ", bads))); + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromReferenceGroups, string.Join(", ", frameworksMissingPlatformVersion))); } var libFiles = new HashSet(from file in files @@ -598,13 +598,13 @@ public static void ValidateReferenceAssemblies(IEnumerable files, private static void ValidateFrameworkAssemblies(IEnumerable packageFrameworkAssemblies) { - var bads = new HashSet(packageFrameworkAssemblies + var frameworksMissingPlatformVersion = new HashSet(packageFrameworkAssemblies .Select(group => group.TargetFramework) .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) .Select(framework => framework.GetShortFolderName())); - if (bads.Any()) + if (frameworksMissingPlatformVersion.Any()) { - throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyGroups, string.Join(", ", bads))); + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyGroups, string.Join(", ", frameworksMissingPlatformVersion))); } } @@ -713,7 +713,7 @@ private static void ValidateFileFrameworks(IEnumerable files) }; var warnPaths = new HashSet(); - var bads = new HashSet(); + var frameworksMissingPlatformVersion = new HashSet(); foreach (var pattern in frameworkPatterns) { IEnumerable targetedItemGroups = ContentExtractor.GetContentForPattern(collection, pattern); @@ -729,15 +729,15 @@ private static void ValidateFileFrameworks(IEnumerable files) if (framework.HasPlatform && framework.PlatformVersion == FrameworkConstants.EmptyVersion) { - bads.Add(framework.GetShortFolderName()); + frameworksMissingPlatformVersion.Add(framework.GetShortFolderName()); } } } } - if (bads.Any()) + if (frameworksMissingPlatformVersion.Any()) { - throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromIncludedFiles, string.Join(", ", bads))); + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromIncludedFiles, string.Join(", ", frameworksMissingPlatformVersion))); } } From f1a440232570dcd28955bb0156f1c20cbe9a0184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Tue, 20 Oct 2020 09:11:19 -0700 Subject: [PATCH 07/13] fixing tests :) --- .../NuGet.Packaging.Test/PackageBuilderTest.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs index d6770eda8cd..4caa432a261 100644 --- a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs +++ b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs @@ -139,7 +139,6 @@ public void CreatePackageWithNuspecIncludeExcludeAnyGroup() [InlineData(".NETFramework,Version=v4.7.2", ".NETFramework4.7.2")] [InlineData(".NETFramework,Version=v4.7.2,Profile=foo", ".NETFramework4.7.2-foo")] [InlineData("net5.0", "net5.0")] - [InlineData("net5.0-windows", "net5.0-windows")] [InlineData("net5.0-macos10.8", "net5.0-macos10.8")] [InlineData("net6.0", "net6.0")] public void CreatePackageTFMFormatting(string from, string to) @@ -214,8 +213,8 @@ public void CreatePackageWithNuspecIncludeExclude() var net45 = new PackageDependencyGroup(new NuGetFramework(".NETFramework", new Version(4, 5)), dependencies45); builder.DependencyGroups.Add(net45); - var net50win = new PackageDependencyGroup(new NuGetFramework(".NETCoreApp", new Version(5, 0), "windows", new Version(0, 0)), dependencies50); - builder.DependencyGroups.Add(net50win); + var net50win7 = new PackageDependencyGroup(new NuGetFramework(".NETCoreApp", new Version(5, 0), "windows", new Version(7, 0)), dependencies50); + builder.DependencyGroups.Add(net50win7); using (var ms = new MemoryStream()) { From 7cc39704b72d22e6773866bf5ebf91cff914a58f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Tue, 20 Oct 2020 09:37:25 -0700 Subject: [PATCH 08/13] missed a spot --- .../NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs index 4caa432a261..40e797a3858 100644 --- a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs +++ b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs @@ -239,7 +239,7 @@ public void CreatePackageWithNuspecIncludeExclude() - + From 1bc631c85397ceeb923d28cb7587d11037820da8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Tue, 20 Oct 2020 10:25:26 -0700 Subject: [PATCH 09/13] last test fix --- .../NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs index 88fe1c16db8..c0fca858e5a 100644 --- a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs +++ b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs @@ -687,7 +687,7 @@ private void ValidateIconFile(IEnumerable files, string iconPath) private static void ValidateFileFrameworks(IEnumerable files) { var set = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var file in files.Select(t => PathUtility.GetPathWithDirectorySeparator(t.Path))) + foreach (var file in files.Where(t => t.Path != null).Select(t => PathUtility.GetPathWithDirectorySeparator(t.Path))) { set.Add(file); } From 95de42afc86b9353fb761eb11b69124f4a594f71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Tue, 20 Oct 2020 14:33:53 -0700 Subject: [PATCH 10/13] add tests for validations --- .../Authoring/PackageBuilder.cs | 17 +++- .../NuGet.Packaging/Strings.Designer.cs | 9 ++ src/NuGet.Core/NuGet.Packaging/Strings.resx | 4 + .../PackageBuilderTest.cs | 90 +++++++++++++++++++ 4 files changed, 117 insertions(+), 3 deletions(-) diff --git a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs index c0fca858e5a..31beb7c1a98 100644 --- a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs +++ b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs @@ -380,7 +380,7 @@ public void Save(Stream stream) ValidateDependencies(Version, DependencyGroups); ValidateReferenceAssemblies(Files, PackageAssemblyReferences); - ValidateFrameworkAssemblies(FrameworkReferenceGroups); + ValidateFrameworkAssemblies(FrameworkReferences, FrameworkReferenceGroups); ValidateLicenseFile(Files, LicenseMetadata); ValidateIconFile(Files, Icon); ValidateFileFrameworks(Files); @@ -596,9 +596,20 @@ public static void ValidateReferenceAssemblies(IEnumerable files, } } - private static void ValidateFrameworkAssemblies(IEnumerable packageFrameworkAssemblies) + private static void ValidateFrameworkAssemblies(IEnumerable references, IEnumerable referenceGroups) { - var frameworksMissingPlatformVersion = new HashSet(packageFrameworkAssemblies + // Check standalone references + var frameworksMissingPlatformVersion = new HashSet(references + .SelectMany(reference => reference.SupportedFrameworks) + .Select(framework => framework.GetShortFolderName()) + ); + if (frameworksMissingPlatformVersion.Any()) + { + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyReferences, string.Join(", ", frameworksMissingPlatformVersion))); + } + + // Check reference groups too + frameworksMissingPlatformVersion = new HashSet(referenceGroups .Select(group => group.TargetFramework) .Where(groupFramework => groupFramework.HasPlatform && groupFramework.PlatformVersion == FrameworkConstants.EmptyVersion) .Select(framework => framework.GetShortFolderName())); diff --git a/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs b/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs index e0dcfa15546..df74cb33094 100644 --- a/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs @@ -807,6 +807,15 @@ internal static string MissingTargetPlatformVersionsFromFrameworkAssemblyGroups } } + /// + /// Looks up a localized string similar to Some framework assembly reference TFMs are missing a platform version: {0}. + /// + internal static string MissingTargetPlatformVersionsFromFrameworkAssemblyReferences { + get { + return ResourceManager.GetString("MissingTargetPlatformVersionsFromFrameworkAssemblyReferences", resourceCulture); + } + } + /// /// Looks up a localized string similar to Some included files are included under TFMs which are missing a platform version: {0}. /// diff --git a/src/NuGet.Core/NuGet.Packaging/Strings.resx b/src/NuGet.Core/NuGet.Packaging/Strings.resx index 5fe89dbc96e..846d2a39e37 100644 --- a/src/NuGet.Core/NuGet.Packaging/Strings.resx +++ b/src/NuGet.Core/NuGet.Packaging/Strings.resx @@ -855,4 +855,8 @@ Valid from: Some reference group TFMs are missing a platform version: {0} 0 - comma-separated list of reference group TFMs + + Some framework assembly reference TFMs are missing a platform version: {0} + 0 - comma-separated list of reference group TFMs + \ No newline at end of file diff --git a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs index 40e797a3858..24fc339d308 100644 --- a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs +++ b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs @@ -1317,6 +1317,96 @@ public void SavingPackageValidatesReferences() "Invalid assembly reference 'Bar.dll'. Ensure that a file named 'Bar.dll' exists in the lib directory."); } + [Fact] + public void SavingPackageValidatesMissingTPVInReferences() + { + // Arrange + var builder = new PackageBuilder + { + Id = "A", + Version = NuGetVersion.Parse("1.0"), + Description = "Test", + }; + builder.Authors.Add("Test"); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"lib\net5.0-windows\Foo.dll" }); + builder.PackageAssemblyReferences = new[] { new PackageReferenceSet(NuGetFramework.Parse("net5.0-windows"), new string[] { "Foo.dll" }) }; + + ExceptionAssert.Throws(() => builder.Save(new MemoryStream()), + "Some reference group TFMs are missing a platform version: net5.0-windows"); + } + + [Fact] + public void SavingPackageValidatesMissingTPVInFiles() + { + // Arrange + var builder = new PackageBuilder + { + Id = "A", + Version = NuGetVersion.Parse("1.0"), + Description = "Test", + }; + builder.Authors.Add("Test"); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"lib\net5.0-windows\Foo.dll" }); + + ExceptionAssert.Throws(() => builder.Save(new MemoryStream()), + "Some included files are included under TFMs which are missing a platform version: net5.0-windows"); + } + + [Fact] + public void SavingPackageValidatesMissingTPVInFrameworkReferences() + { + // Arrange + var builder = new PackageBuilder + { + Id = "A", + Version = NuGetVersion.Parse("1.0"), + Description = "Test", + }; + builder.Authors.Add("Test"); + builder.FrameworkReferences.Add(new FrameworkAssemblyReference("System.Web", new[] { NuGetFramework.Parse("net5.0-windows") })); + + ExceptionAssert.Throws(() => builder.Save(new MemoryStream()), + "Some framework assembly reference TFMs are missing a platform version: net5.0-windows"); + } + + [Fact] + public void SavingPackageValidatesMissingTPVInFrameworkReferenceGroups() + { + // Arrange + var builder = new PackageBuilder + { + Id = "A", + Version = NuGetVersion.Parse("1.0"), + Description = "Test", + }; + builder.Authors.Add("Test"); + builder.FrameworkReferenceGroups.Add(new FrameworkReferenceGroup(NuGetFramework.Parse("net5.0-windows"), new List())); + + ExceptionAssert.Throws(() => builder.Save(new MemoryStream()), + "Some reference assembly group TFMs are missing a platform version: net5.0-windows"); + } + + [Fact] + public void SavingPackageValidatesMissingTPVInDependencyGroups() + { + // Arrange + var builder = new PackageBuilder + { + Id = "A", + Version = NuGetVersion.Parse("1.0"), + Description = "Test", + }; + builder.Authors.Add("Test"); + var dependencySet = new PackageDependencyGroup(NuGetFramework.Parse("net5.0-windows"), new[] { + new PackageDependency("B", new VersionRange(NuGetVersion.Parse("2.0"), true, NuGetVersion.Parse("2.0"))) + }); + + builder.DependencyGroups.Add(dependencySet); + + ExceptionAssert.Throws(() => builder.Save(new MemoryStream()), + "Some dependency group TFMs are missing a platform version: net5.0-windows"); + } + [Fact] public void SavingPackageWithInvalidDependencyVersionMaxLessThanMinThrows() { From 86de1fa689d45a09c93d569ce6f590080c5419ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Tue, 20 Oct 2020 16:10:50 -0700 Subject: [PATCH 11/13] :clown: --- .../NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs index 31beb7c1a98..90f55cfc0ef 100644 --- a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs +++ b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs @@ -601,6 +601,7 @@ private static void ValidateFrameworkAssemblies(IEnumerable(references .SelectMany(reference => reference.SupportedFrameworks) + .Where(framework => framework.HasPlatform && framework.PlatformVersion == FrameworkConstants.EmptyVersion) .Select(framework => framework.GetShortFolderName()) ); if (frameworksMissingPlatformVersion.Any()) From 102ae445a21bb7eedf87037cc51848b37730220b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Wed, 21 Oct 2020 10:46:08 -0700 Subject: [PATCH 12/13] add tests for the rest of the cases --- .../PackageCreation/Authoring/PackageBuilder.cs | 10 +++++----- .../NuGet.Packaging.Test/PackageBuilderTest.cs | 10 +++++++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs index 90f55cfc0ef..a9f46c75114 100644 --- a/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs +++ b/src/NuGet.Core/NuGet.Packaging/PackageCreation/Authoring/PackageBuilder.cs @@ -554,7 +554,7 @@ private static void ValidateDependencies(SemanticVersion version, .Select(framework => framework.GetShortFolderName())); if (frameworksMissingPlatformVersion.Any()) { - throw new PackagingException(NuGetLogCode.NU1012, String.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromDependencyGroups, string.Join(", ", frameworksMissingPlatformVersion))); + throw new PackagingException(NuGetLogCode.NU1012, String.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromDependencyGroups, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str)))); } if (version == null) @@ -577,7 +577,7 @@ public static void ValidateReferenceAssemblies(IEnumerable files, .Select(framework => framework.GetShortFolderName())); if (frameworksMissingPlatformVersion.Any()) { - throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromReferenceGroups, string.Join(", ", frameworksMissingPlatformVersion))); + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromReferenceGroups, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str)))); } var libFiles = new HashSet(from file in files @@ -606,7 +606,7 @@ private static void ValidateFrameworkAssemblies(IEnumerable str)))); } // Check reference groups too @@ -616,7 +616,7 @@ private static void ValidateFrameworkAssemblies(IEnumerable framework.GetShortFolderName())); if (frameworksMissingPlatformVersion.Any()) { - throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyGroups, string.Join(", ", frameworksMissingPlatformVersion))); + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromFrameworkAssemblyGroups, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str)))); } } @@ -749,7 +749,7 @@ private static void ValidateFileFrameworks(IEnumerable files) if (frameworksMissingPlatformVersion.Any()) { - throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromIncludedFiles, string.Join(", ", frameworksMissingPlatformVersion))); + throw new PackagingException(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.MissingTargetPlatformVersionsFromIncludedFiles, string.Join(", ", frameworksMissingPlatformVersion.OrderBy(str => str)))); } } diff --git a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs index 24fc339d308..9f2c460d9bd 100644 --- a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs +++ b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs @@ -1347,9 +1347,17 @@ public void SavingPackageValidatesMissingTPVInFiles() }; builder.Authors.Add("Test"); builder.Files.Add(new PhysicalPackageFile { TargetPath = @"lib\net5.0-windows\Foo.dll" }); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"ref\net6.0-windows\Foo.dll" }); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"runtimes\win7-x64\lib\net7.0-windows\Foo.dll" }); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"runtimes\win7-x64\nativeassets\net8.0-windows\Foo.dll" }); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"build\net9.0-windows\foo.props" }); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"contentFiles\csharp\net10.0-windows\Foo.txt" }); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"tools\net11.0-windows\win7-x64\Foo.exe" }); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"embed\net12.0-windows\Foo.dll" }); + builder.Files.Add(new PhysicalPackageFile { TargetPath = @"buildTransitive\net13.0-windows\foo.props" }); ExceptionAssert.Throws(() => builder.Save(new MemoryStream()), - "Some included files are included under TFMs which are missing a platform version: net5.0-windows"); + "Some included files are included under TFMs which are missing a platform version: " + string.Join(", ", new string[] { "net5.0-windows", "net6.0-windows", "net7.0-windows", "net8.0-windows", "net9.0-windows", "net10.0-windows", "net11.0-windows", "net12.0-windows", "net13.0-windows"}.OrderBy(str => str))); } [Fact] From 57dee9a77f0095b082535ed519cb7eba1bde5d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Wed, 21 Oct 2020 10:58:30 -0700 Subject: [PATCH 13/13] uhhh --- .../NuGet.Packaging.Test/PackageBuilderTest.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs index 9f2c460d9bd..eccda8c3848 100644 --- a/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs +++ b/test/NuGet.Core.Tests/NuGet.Packaging.Test/PackageBuilderTest.cs @@ -1357,7 +1357,18 @@ public void SavingPackageValidatesMissingTPVInFiles() builder.Files.Add(new PhysicalPackageFile { TargetPath = @"buildTransitive\net13.0-windows\foo.props" }); ExceptionAssert.Throws(() => builder.Save(new MemoryStream()), - "Some included files are included under TFMs which are missing a platform version: " + string.Join(", ", new string[] { "net5.0-windows", "net6.0-windows", "net7.0-windows", "net8.0-windows", "net9.0-windows", "net10.0-windows", "net11.0-windows", "net12.0-windows", "net13.0-windows"}.OrderBy(str => str))); + "Some included files are included under TFMs which are missing a platform version: " + string.Join(", ", new string[] + { + "net5.0-windows", + "net6.0-windows", + "net7.0-windows", + "net8.0-windows", + "net9.0-windows", + "net10.0-windows", + "net11.0-windows", + "net12.0-windows", + "net13.0-windows" + }.OrderBy(str => str))); } [Fact]