Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error when TPV is missing from target frameworks #3616

Merged
merged 7 commits into from
Sep 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions src/NuGet.Core/NuGet.Build.Tasks.Pack/PackTaskLogic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ private IEnumerable<OutputLibFile> 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);
Expand All @@ -437,6 +437,18 @@ private IEnumerable<OutputLibFile> 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 (fw.HasPlatform && fw.PlatformVersion == FrameworkConstants.EmptyVersion)
{
throw new PackagingException(
NuGetLogCode.NU1012,
string.Format(CultureInfo.CurrentCulture, Strings.InvalidPlatformVersion, targetFramework)
);
}
}

assemblies.Add(new OutputLibFile()
{
FinalOutputPath = finalOutputPath,
Expand Down Expand Up @@ -655,7 +667,7 @@ private IEnumerable<ContentMetadata> 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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
{
Expand Down
14 changes: 11 additions & 3 deletions src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion src/NuGet.Core/NuGet.Build.Tasks.Pack/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@
<value>PackageVersion string specified '{0}' is invalid.</value>
<comment>{0} is the version.</comment>
</data>
<data name="InvalidPlatformVersion" xml:space="preserve">
<value>Platform version is missing from '{0}'</value>
<comment>{0} is the target framework string.</comment>
</data>
<data name="InvalidTargetFramework" xml:space="preserve">
<value>Invalid target framework for the file '{0}'.</value>
</data>
Expand All @@ -172,4 +176,4 @@
<value>The PackageLicenseUrl is being deprecated and cannot be used in conjunction with the PackageLicenseFile or PackageLicenseExpression.</value>
<comment>Please don't localize PackageLicenseUrl, PackageLicenseFile and PackageLicenseExpression.</comment>
</data>
</root>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
25 changes: 22 additions & 3 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ public async Task<RestoreResult> ExecuteAsync(CancellationToken token)
_success = false;
}

_success &= HasValidPlatformVersions();

// evaluate packages.lock.json file
var packagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project);
var isLockFileValid = false;
Expand Down Expand Up @@ -404,6 +406,23 @@ public async Task<RestoreResult> ExecuteAsync(CancellationToken token)
}
}

private bool HasValidPlatformVersions()
{
IEnumerable<NuGetFramework> badPlatforms = _request.Project.TargetFrameworks.Select(tfm => tfm.FrameworkName).Where(fw => !string.IsNullOrEmpty(fw.Platform) && (fw.PlatformVersion == FrameworkConstants.EmptyVersion));
if (badPlatforms.Any())
{
foreach (NuGetFramework fw in badPlatforms)
{
_logger.Log(RestoreLogMessage.CreateError(NuGetLogCode.NU1012, string.Format(CultureInfo.CurrentCulture, Strings.Error_PlatformVersionNotPresent, fw.Framework, fw.Platform)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: In an ideal world, I'd recommend these get merged so that a user gets only 1 instead of N errors, but I simply don't see this scenario getting hit too often (actually I'd only expect it during development), so I would not bother too much :)

}
return false;
}
else
{
return true;
}
}

private async Task<bool> AreCentralVersionRequirementsSatisfiedAsync()
{
// The dependencies should not have versions explicitelly defined if cpvm is enabled.
Expand Down Expand Up @@ -627,7 +646,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;
}
Expand Down Expand Up @@ -727,7 +746,7 @@ private LockFile BuildAssetsFile(
/// <summary>
/// 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.
/// </summary>
private async Task<bool> ValidateRestoreGraphsAsync(IEnumerable<RestoreTargetGraph> graphs, ILogger logger)
Expand Down Expand Up @@ -1108,7 +1127,7 @@ private List<ExternalProjectReference> 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}");
Expand Down
25 changes: 17 additions & 8 deletions src/NuGet.Core/NuGet.Commands/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/NuGet.Core/NuGet.Commands/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1111,4 +1111,8 @@ For more information, visit https://docs.nuget.org/docs/reference/command-line-r
<data name="SpecValidation_OriginalTargetFrameworksMustMatchAliases" xml:space="preserve">
<value>The original target frameworks value must match the aliases. Original target frameworks: {0}, aliases: {1}.</value>
</data>
<data name="Error_PlatformVersionNotPresent" xml:space="preserve">
<value>A platform version for '{0}' could not be inferred.</value>
<comment>{0} should be the TargetFramework</comment>
</data>
</root>
5 changes: 5 additions & 0 deletions src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ public enum NuGetLogCode
/// </summary>
NU1011 = 1011,

/// <summary>
/// Target Framework has a platform component, but the platform version could not be inferred.
/// </summary>
NU1012 = 1012,

/// <summary>
/// Unable to resolve package, generic message for unknown type constraints.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<PackageSource>
{
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(
Expand Down
Loading