From 3c5a259b43dc5325d9d9365b260237b8d26859e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Fri, 28 Aug 2020 14:24:05 -0700 Subject: [PATCH 1/7] error when TPV is missing from dependencies Fixes: https://github.com/nuget/home/issues/9441 --- .../RestoreCommand/RestoreCommand.cs | 21 ++++++ .../NuGet.Commands/Strings.Designer.cs | 9 +++ src/NuGet.Core/NuGet.Commands/Strings.resx | 60 +++++++++-------- .../NuGet.Common/Errors/NuGetLogCode.cs | 5 ++ .../PublicAPI/net45/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net472/PublicAPI.Unshipped.txt | 2 +- .../netstandard2.0/PublicAPI.Unshipped.txt | 2 +- .../RestoreCommandTests.cs | 66 +++++++++++++++++++ 8 files changed, 136 insertions(+), 30 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index c0b5b8bfe9b..f42d1ca8c8c 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -181,6 +181,12 @@ public async Task ExecuteAsync(CancellationToken token) _success = false; } + if (!CheckPlatformVersions()) + { + // the errors will be added to the assets file + _success = false; + } + // evaluate packages.lock.json file var packagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); var isLockFileValid = false; @@ -404,6 +410,21 @@ public async Task ExecuteAsync(CancellationToken token) } } + private bool CheckPlatformVersions() + { + IEnumerable badPlatforms = _request.Project.TargetFrameworks.Select(tfm => tfm.FrameworkName).Where(fw => !string.IsNullOrEmpty(fw.Platform) && (fw.PlatformVersion == FrameworkConstants.EmptyVersion)); + if (badPlatforms.Any()) + { + NuGetFramework fw = badPlatforms.First(); + _logger.Log(RestoreLogMessage.CreateError(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.Error_PlatformVersionNotPresent, fw.Framework, fw.Platform))); + return false; + } + else + { + return true; + } + } + private async Task AreCentralVersionRequirementsSatisfiedAsync() { // The dependencies should not have versions explicitelly defined if cpvm is enabled. diff --git a/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs b/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs index 488ba928f0c..56c946c3df4 100644 --- a/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs @@ -663,6 +663,15 @@ internal static string Error_PackFailed { } } + /// + /// Looks up a localized string similar to TargetFramework '{0}' has a platform '{1}', but a platform version could not be inferred. Specify the platform version explicitly.. + /// + 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 b07ba340cad..dbf02590817 100644 --- a/src/NuGet.Core/NuGet.Commands/Strings.resx +++ b/src/NuGet.Core/NuGet.Commands/Strings.resx @@ -1,17 +1,17 @@ - @@ -1111,4 +1111,8 @@ For more information, visit https://docs.nuget.org/docs/reference/command-line-r The original target frameworks value must match the aliases. Original target frameworks: {0}, aliases: {1}. - \ No newline at end of file + + TargetFramework '{0}' has a platform '{1}', but a platform version could not be inferred. Specify the platform version explicitly. + {0} should be the TargetFramework. {1} should be the TargetPlatformIdentifier + + diff --git a/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs b/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs index 5945bf8675d..9fd9510c143 100644 --- a/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs +++ b/src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs @@ -102,6 +102,11 @@ public enum NuGetLogCode /// NU1011 = 1011, + /// + /// Target Framework has a platform component, but the platform version could not be inferred. + /// + 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 980fe9f40fc..91d5e64fcbb 100644 --- a/src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt +++ b/src/NuGet.Core/NuGet.Common/PublicAPI/net45/PublicAPI.Unshipped.txt @@ -1,2 +1,3 @@ NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU1011 = 1011 -> 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 a70b68898e4..91d5e64fcbb 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,3 @@ NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU1011 = 1011 -> 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 a70b68898e4..91d5e64fcbb 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,3 @@ NuGet.Common.NuGetLogCode.NU1010 = 1010 -> NuGet.Common.NuGetLogCode NuGet.Common.NuGetLogCode.NU1011 = 1011 -> 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..96467493073 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 could not be inferred")); + 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 048ee0a0ace318b359baba97ffb20f91b1882777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Mon, 31 Aug 2020 16:35:55 -0700 Subject: [PATCH 2/7] add platform version check to pack, too --- .../NuGet.Build.Tasks.Pack/PackTaskLogic.cs | 20 ++++++++++--- .../Strings.Designer.cs | 14 ++++++++-- .../NuGet.Build.Tasks.Pack/Strings.resx | 6 +++- .../CommandRunners/PackCommandRunner.cs | 6 ++-- .../NuGet.Commands/GlobalSuppressions.cs | 2 +- .../PackTaskLogicTests.cs | 28 ++++++++++++++++--- 6 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs b/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs index aaf4479f778..0ee04dc8e66 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs @@ -410,7 +410,7 @@ private IEnumerable InitLibFiles(IMSBuildItem[] libFiles) var finalOutputPath = assembly.GetProperty("FinalOutputPath"); // Fallback to using Identity if FinalOutputPath is not set. - // See bug https://github.com/NuGet/Home/issues/5408 + // See bug https://github.com/NuGet/Home/issues/5408 if (string.IsNullOrEmpty(finalOutputPath)) { finalOutputPath = assembly.GetProperty(IdentityProperty); @@ -437,6 +437,18 @@ private IEnumerable InitLibFiles(IMSBuildItem[] libFiles) throw new PackagingException(NuGetLogCode.NU5027, string.Format(CultureInfo.CurrentCulture, Strings.InvalidTargetFramework, finalOutputPath)); } + if (!string.IsNullOrEmpty(targetFramework)) + { + var fw = NuGetFramework.Parse(targetFramework); + if (!string.IsNullOrEmpty(fw.Platform) && fw.PlatformVersion == FrameworkConstants.EmptyVersion) + { + throw new PackagingException( + NuGetLogCode.NU1012, + string.Format(CultureInfo.CurrentCulture, Strings.InvalidPlatformVersion, targetFramework) + ); + } + } + assemblies.Add(new OutputLibFile() { FinalOutputPath = finalOutputPath, @@ -655,7 +667,7 @@ private IEnumerable GetContentMetadata(IMSBuildItem packageFile var newTargetPath = Path.Combine(targetPath, identity); // We need to do this because evaluated identity in the above line of code can be an empty string // in the case when the original identity string was the absolute path to a file in project directory, and is in - // the same directory as the csproj file. + // the same directory as the csproj file. newTargetPath = PathUtility.EnsureTrailingSlash(newTargetPath); newTargetPaths.Add(newTargetPath); } @@ -813,7 +825,7 @@ private static void InitializeProjectDependencies( var versionToUse = new VersionRange(targetLibrary.Version); - // Use the project reference version obtained at build time if it exists, otherwise fallback to the one in assets file. + // Use the project reference version obtained at build time if it exists, otherwise fallback to the one in assets file. if (projectRefToVersionMap.TryGetValue(projectReference.ProjectPath, out var projectRefVersion)) { versionToUse = VersionRange.Parse(projectRefVersion, allowFloating: false); @@ -872,7 +884,7 @@ private static void InitializePackageDependencies( // Add each package dependency. foreach (var packageDependency in packageDependencies) { - // If we have a floating package dependency like 1.2.3-xyz-*, we + // If we have a floating package dependency like 1.2.3-xyz-*, we // use the version of the package that restore resolved it to. if (packageDependency.LibraryRange.VersionRange.IsFloating) { diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.Designer.cs b/src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.Designer.cs index fbedbeb9b1b..ff61f91d5f0 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.Designer.cs @@ -10,7 +10,6 @@ namespace NuGet.Build.Tasks.Pack { using System; - using System.Reflection; /// @@ -20,7 +19,7 @@ namespace NuGet.Build.Tasks.Pack { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "15.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "16.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Strings { @@ -40,7 +39,7 @@ internal Strings() { internal static global::System.Resources.ResourceManager ResourceManager { get { if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("NuGet.Build.Tasks.Pack.Strings", typeof(Strings).GetTypeInfo().Assembly); + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("NuGet.Build.Tasks.Pack.Strings", typeof(Strings).Assembly); resourceMan = temp; } return resourceMan; @@ -160,6 +159,15 @@ internal static string InvalidPackageVersion { } } + /// + /// Looks up a localized string similar to Platform version is missing from '{0}'. + /// + internal static string InvalidPlatformVersion { + get { + return ResourceManager.GetString("InvalidPlatformVersion", resourceCulture); + } + } + /// /// Looks up a localized string similar to Invalid target framework for the file '{0}'.. /// diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.resx b/src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.resx index f1a43c014ed..f4d1cb96b3e 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.resx +++ b/src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.resx @@ -159,6 +159,10 @@ PackageVersion string specified '{0}' is invalid. {0} is the version. + + Platform version is missing from '{0}' + {0} is the target framework string. + Invalid target framework for the file '{0}'. @@ -172,4 +176,4 @@ The PackageLicenseUrl is being deprecated and cannot be used in conjunction with the PackageLicenseFile or PackageLicenseExpression. Please don't localize PackageLicenseUrl, PackageLicenseFile and PackageLicenseExpression. - + \ No newline at end of file diff --git a/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs b/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs index 44054e640c6..af1aca20646 100644 --- a/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs +++ b/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs @@ -318,7 +318,7 @@ private void InitCommonPackageBuilderProperties(PackageBuilder builder) builder.MinClientVersion = _packArgs.MinClientVersion; } - CheckForUnsupportedFrameworks(builder); + CheckForBadFrameworks(builder); ExcludeFiles(builder.Files); } @@ -843,7 +843,7 @@ private bool BuildFromProjectFile(string path) return successful; } - private void CheckForUnsupportedFrameworks(PackageBuilder builder) + private void CheckForBadFrameworks(PackageBuilder builder) { foreach (FrameworkAssemblyReference reference in builder.FrameworkReferences) { @@ -992,7 +992,7 @@ private static string ResolvePath(IPackageFile packageFile, string basePath) private bool BuildSymbolsPackage(string path) { PackageBuilder symbolsBuilder = CreatePackageBuilderFromNuspec(path); - if (_packArgs.SymbolPackageFormat == SymbolPackageFormat.Snupkg) // Snupkgs can only have 1 PackageType. + if (_packArgs.SymbolPackageFormat == SymbolPackageFormat.Snupkg) // Snupkgs can only have 1 PackageType. { symbolsBuilder.PackageTypes.Clear(); symbolsBuilder.PackageTypes.Add(PackageType.SymbolsPackage); diff --git a/src/NuGet.Core/NuGet.Commands/GlobalSuppressions.cs b/src/NuGet.Core/NuGet.Commands/GlobalSuppressions.cs index a36117eb9ac..2e9442bdbad 100644 --- a/src/NuGet.Core/NuGet.Commands/GlobalSuppressions.cs +++ b/src/NuGet.Core/NuGet.Commands/GlobalSuppressions.cs @@ -148,7 +148,7 @@ [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void PackCommandRunner.AddLibraryDependency(LibraryDependency dependency, ISet list)', validate parameter 'list' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.AddLibraryDependency(NuGet.LibraryModel.LibraryDependency,System.Collections.Generic.ISet{NuGet.LibraryModel.LibraryDependency})")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void PackCommandRunner.AddPackageDependency(PackageDependency dependency, ISet set)', validate parameter 'set' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.AddPackageDependency(NuGet.Packaging.Core.PackageDependency,System.Collections.Generic.ISet{NuGet.Packaging.Core.PackageDependency})")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'PackageArchiveReader PackCommandRunner.BuildPackage(PackageBuilder builder, string outputPath = null)', validate parameter 'builder' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.BuildPackage(NuGet.Packaging.PackageBuilder,System.String)~NuGet.Packaging.PackageArchiveReader")] -[assembly: SuppressMessage("Build", "CA1822:Member CheckForUnsupportedFrameworks does not access instance data and can be marked as static (Shared in VisualBasic)", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.CheckForUnsupportedFrameworks(NuGet.Packaging.PackageBuilder)")] +[assembly: SuppressMessage("Build", "CA1822:Member CheckForUnsupportedFrameworks does not access instance data and can be marked as static (Shared in VisualBasic)", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.CheckForBadFrameworks(NuGet.Packaging.PackageBuilder)")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'string PackCommandRunner.GetInputFile(PackArgs packArgs)', validate parameter 'packArgs' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.GetInputFile(NuGet.Commands.PackArgs)~System.String")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'string PackCommandRunner.GetOutputFileName(string packageId, NuGetVersion version, bool isNupkg, bool symbols, SymbolPackageFormat symbolPackageFormat, bool excludeVersion = false)', validate parameter 'version' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.GetOutputFileName(System.String,NuGet.Versioning.NuGetVersion,System.Boolean,System.Boolean,NuGet.Commands.SymbolPackageFormat,System.Boolean)~System.String")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'string PackCommandRunner.GetOutputPath(PackageBuilder builder, PackArgs packArgs, bool symbols = false, NuGetVersion nugetVersion = null, string outputDirectory = null, bool isNupkg = true)', validate parameter 'packArgs' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.GetOutputPath(NuGet.Packaging.PackageBuilder,NuGet.Commands.PackArgs,System.Boolean,NuGet.Versioning.NuGetVersion,System.String,System.Boolean)~System.String")] diff --git a/test/NuGet.Core.Tests/NuGet.Build.Tasks.Pack.Test/PackTaskLogicTests.cs b/test/NuGet.Core.Tests/NuGet.Build.Tasks.Pack.Test/PackTaskLogicTests.cs index 00f99a1fef8..fa5fb1537e0 100644 --- a/test/NuGet.Core.Tests/NuGet.Build.Tasks.Pack.Test/PackTaskLogicTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Build.Tasks.Pack.Test/PackTaskLogicTests.cs @@ -9,6 +9,7 @@ using NuGet.Commands; using NuGet.Frameworks; using NuGet.Packaging; +using NuGet.Packaging.Core; using NuGet.Test.Utility; using Xunit; @@ -64,6 +65,20 @@ public void PackTaskLogic_ProducesBasicPackage() } } + [Fact] + public void PackTaskLogic_ErrorsOnBadFrameworkPlatform() + { + // This test uses the ...\NuGet.Build.Tasks.Pack.Test\compiler\resources\project.assets.json assets file. + // Arrange + using (var testDir = TestDirectory.Create()) + { + var tc = new TestContext(testDir, "net5.0-windows"); + + // Act & Assert + Assert.Throws(() => tc.BuildPackage()); + } + } + [Fact] public void PackTaskLogic_SplitsTags() { @@ -625,11 +640,16 @@ public void PackTaskLogic_EmbedInteropAssembly() private class TestContext { - public TestContext(TestDirectory testDir) + public TestContext(TestDirectory testdir) + : this(testdir, "net45") + { + } + + public TestContext(TestDirectory testDir, string tfm) { var fullPath = Path.Combine(testDir, "project.csproj"); var rootDir = Path.GetPathRoot(testDir); - var dllDir = Path.Combine(testDir, "bin", "Debug", "net45"); + var dllDir = Path.Combine(testDir, "bin", "Debug", tfm); var dllPath = Path.Combine(dllDir, "a.dll"); Directory.CreateDirectory(dllDir); @@ -664,11 +684,11 @@ public TestContext(TestDirectory testDir) IncludeBuildOutput = true, RestoreOutputPath = Path.Combine(testDir, "obj"), ContinuePackingAfterGeneratingNuspec = true, - TargetFrameworks = new[] { "net45" }, + TargetFrameworks = new[] { tfm }, BuildOutputInPackage = new[] { new MSBuildItem(dllPath, new Dictionary { {"FinalOutputPath", dllPath }, - {"TargetFramework", "net45" } + {"TargetFramework", tfm } })}, Logger = new TestLogger(), SymbolPackageFormat = "symbols.nupkg", From 35c3e0f428185f118c4ebb78067e7f668f6de377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Fri, 4 Sep 2020 13:34:38 -0700 Subject: [PATCH 3/7] show all bad platform errors --- .../NuGet.Commands/RestoreCommand/RestoreCommand.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index f42d1ca8c8c..95e0a6a1ccd 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -415,8 +415,10 @@ private bool CheckPlatformVersions() IEnumerable badPlatforms = _request.Project.TargetFrameworks.Select(tfm => tfm.FrameworkName).Where(fw => !string.IsNullOrEmpty(fw.Platform) && (fw.PlatformVersion == FrameworkConstants.EmptyVersion)); if (badPlatforms.Any()) { - NuGetFramework fw = badPlatforms.First(); - _logger.Log(RestoreLogMessage.CreateError(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.Error_PlatformVersionNotPresent, fw.Framework, fw.Platform))); + foreach (NuGetFramework fw in badPlatforms) + { + _logger.Log(RestoreLogMessage.CreateError(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.Error_PlatformVersionNotPresent, fw.Framework, fw.Platform))); + } return false; } else From 01be6c8978441276703de50f978da2582e6009b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Fri, 4 Sep 2020 13:35:37 -0700 Subject: [PATCH 4/7] format fixes --- .../NuGet.Commands/RestoreCommand/RestoreCommand.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 95e0a6a1ccd..5081126f7c5 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -650,7 +650,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; } @@ -750,7 +750,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) @@ -1131,7 +1131,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}"); From 8911618ab8a32a7ec3d69c65774aebfce456c7d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Fri, 4 Sep 2020 15:21:14 -0700 Subject: [PATCH 5/7] review --- .../CommandRunners/PackCommandRunner.cs | 4 +- .../NuGet.Commands/GlobalSuppressions.cs | 2 +- .../RestoreCommand/RestoreCommand.cs | 8 +-- .../NuGet.Commands/Strings.Designer.cs | 18 +++--- src/NuGet.Core/NuGet.Commands/Strings.resx | 60 +++++++++---------- 5 files changed, 44 insertions(+), 48 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs b/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs index af1aca20646..ef90301cc49 100644 --- a/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs +++ b/src/NuGet.Core/NuGet.Commands/CommandRunners/PackCommandRunner.cs @@ -318,7 +318,7 @@ private void InitCommonPackageBuilderProperties(PackageBuilder builder) builder.MinClientVersion = _packArgs.MinClientVersion; } - CheckForBadFrameworks(builder); + CheckForUnsupportedFrameworks(builder); ExcludeFiles(builder.Files); } @@ -843,7 +843,7 @@ private bool BuildFromProjectFile(string path) return successful; } - private void CheckForBadFrameworks(PackageBuilder builder) + private void CheckForUnsupportedFrameworks(PackageBuilder builder) { foreach (FrameworkAssemblyReference reference in builder.FrameworkReferences) { diff --git a/src/NuGet.Core/NuGet.Commands/GlobalSuppressions.cs b/src/NuGet.Core/NuGet.Commands/GlobalSuppressions.cs index 2e9442bdbad..a36117eb9ac 100644 --- a/src/NuGet.Core/NuGet.Commands/GlobalSuppressions.cs +++ b/src/NuGet.Core/NuGet.Commands/GlobalSuppressions.cs @@ -148,7 +148,7 @@ [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void PackCommandRunner.AddLibraryDependency(LibraryDependency dependency, ISet list)', validate parameter 'list' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.AddLibraryDependency(NuGet.LibraryModel.LibraryDependency,System.Collections.Generic.ISet{NuGet.LibraryModel.LibraryDependency})")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void PackCommandRunner.AddPackageDependency(PackageDependency dependency, ISet set)', validate parameter 'set' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.AddPackageDependency(NuGet.Packaging.Core.PackageDependency,System.Collections.Generic.ISet{NuGet.Packaging.Core.PackageDependency})")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'PackageArchiveReader PackCommandRunner.BuildPackage(PackageBuilder builder, string outputPath = null)', validate parameter 'builder' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.BuildPackage(NuGet.Packaging.PackageBuilder,System.String)~NuGet.Packaging.PackageArchiveReader")] -[assembly: SuppressMessage("Build", "CA1822:Member CheckForUnsupportedFrameworks does not access instance data and can be marked as static (Shared in VisualBasic)", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.CheckForBadFrameworks(NuGet.Packaging.PackageBuilder)")] +[assembly: SuppressMessage("Build", "CA1822:Member CheckForUnsupportedFrameworks does not access instance data and can be marked as static (Shared in VisualBasic)", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.CheckForUnsupportedFrameworks(NuGet.Packaging.PackageBuilder)")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'string PackCommandRunner.GetInputFile(PackArgs packArgs)', validate parameter 'packArgs' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.GetInputFile(NuGet.Commands.PackArgs)~System.String")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'string PackCommandRunner.GetOutputFileName(string packageId, NuGetVersion version, bool isNupkg, bool symbols, SymbolPackageFormat symbolPackageFormat, bool excludeVersion = false)', validate parameter 'version' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.GetOutputFileName(System.String,NuGet.Versioning.NuGetVersion,System.Boolean,System.Boolean,NuGet.Commands.SymbolPackageFormat,System.Boolean)~System.String")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'string PackCommandRunner.GetOutputPath(PackageBuilder builder, PackArgs packArgs, bool symbols = false, NuGetVersion nugetVersion = null, string outputDirectory = null, bool isNupkg = true)', validate parameter 'packArgs' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.Commands.PackCommandRunner.GetOutputPath(NuGet.Packaging.PackageBuilder,NuGet.Commands.PackArgs,System.Boolean,NuGet.Versioning.NuGetVersion,System.String,System.Boolean)~System.String")] diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 5081126f7c5..cb6114b7a06 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -181,11 +181,7 @@ public async Task ExecuteAsync(CancellationToken token) _success = false; } - if (!CheckPlatformVersions()) - { - // the errors will be added to the assets file - _success = false; - } + _success &= HasValidPlatformVersions(); // evaluate packages.lock.json file var packagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); @@ -410,7 +406,7 @@ public async Task ExecuteAsync(CancellationToken token) } } - private bool CheckPlatformVersions() + private bool HasValidPlatformVersions() { IEnumerable badPlatforms = _request.Project.TargetFrameworks.Select(tfm => tfm.FrameworkName).Where(fw => !string.IsNullOrEmpty(fw.Platform) && (fw.PlatformVersion == FrameworkConstants.EmptyVersion)); if (badPlatforms.Any()) diff --git a/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs b/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs index 56c946c3df4..8b8f4cf04bd 100644 --- a/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.Commands/Strings.Designer.cs @@ -1,4 +1,4 @@ -//------------------------------------------------------------------------------ +//------------------------------------------------------------------------------ // // This code was generated by a tool. // Runtime Version:4.0.30319.42000 @@ -268,20 +268,20 @@ internal static string Error_CentralPackageVersions_AutoreferencedReferencesNotA } /// - /// Looks up a localized string similar to The PackageReference items {0} do not have corresponding PackageVersion.. + /// Looks up a localized string similar to Centrally defined floating package versions are not allowed.. /// - internal static string Error_CentralPackageVersions_MissingPackageVersion { + internal static string Error_CentralPackageVersions_FloatingVersionsAreNotAllowed { get { - return ResourceManager.GetString("Error_CentralPackageVersions_MissingPackageVersion", resourceCulture); + return ResourceManager.GetString("Error_CentralPackageVersions_FloatingVersionsAreNotAllowed", resourceCulture); } } - + /// - /// Looks up a localized string similar to Centrally defined floating package versions are not allowed.. + /// Looks up a localized string similar to The PackageReference items {0} do not have corresponding PackageVersion.. /// - internal static string Error_CentralPackageVersions_FloatingVersionsAreNotAllowed { + internal static string Error_CentralPackageVersions_MissingPackageVersion { get { - return ResourceManager.GetString("Error_CentralPackageVersions_FloatingVersionsAreNotAllowed", resourceCulture); + return ResourceManager.GetString("Error_CentralPackageVersions_MissingPackageVersion", resourceCulture); } } @@ -664,7 +664,7 @@ internal static string Error_PackFailed { } /// - /// Looks up a localized string similar to TargetFramework '{0}' has a platform '{1}', but a platform version could not be inferred. Specify the platform version explicitly.. + /// Looks up a localized string similar to A platform version for '{0}' could not be inferred.. /// 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 dbf02590817..cb3b596b54c 100644 --- a/src/NuGet.Core/NuGet.Commands/Strings.resx +++ b/src/NuGet.Core/NuGet.Commands/Strings.resx @@ -1,17 +1,17 @@ - @@ -1112,7 +1112,7 @@ For more information, visit https://docs.nuget.org/docs/reference/command-line-r The original target frameworks value must match the aliases. Original target frameworks: {0}, aliases: {1}. - TargetFramework '{0}' has a platform '{1}', but a platform version could not be inferred. Specify the platform version explicitly. - {0} should be the TargetFramework. {1} should be the TargetPlatformIdentifier + A platform version for '{0}' could not be inferred. + {0} should be the TargetFramework - + \ No newline at end of file From d7b641a119ae12f5a4bf1718393d83d129799a8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Fri, 4 Sep 2020 16:06:20 -0700 Subject: [PATCH 6/7] missed one --- src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs b/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs index 0ee04dc8e66..1d72af410bc 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs @@ -440,7 +440,7 @@ private IEnumerable InitLibFiles(IMSBuildItem[] libFiles) if (!string.IsNullOrEmpty(targetFramework)) { var fw = NuGetFramework.Parse(targetFramework); - if (!string.IsNullOrEmpty(fw.Platform) && fw.PlatformVersion == FrameworkConstants.EmptyVersion) + if (fw.HasPlatform && fw.PlatformVersion == FrameworkConstants.EmptyVersion) { throw new PackagingException( NuGetLogCode.NU1012, From 27d89d0e0133cbc3db8bb530ec66bbf0e54066ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Tue, 8 Sep 2020 14:30:05 -0700 Subject: [PATCH 7/7] forgot to update tests >:( --- .../NuGet.Commands.FuncTest/RestoreCommandTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommandTests.cs index 96467493073..15ec779a484 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 could not be inferred")); + Assert.True(errorMessage.Contains("platform version")); var messagesForNU1012 = result.LockFile.LogMessages.Where(m => m.Code == NuGetLogCode.NU1012); Assert.Equal(1, messagesForNU1012.Count()); }