From 367ae05e55e31c67bbbf1efe82630b22a50d2246 Mon Sep 17 00:00:00 2001 From: Alex Panov Date: Fri, 20 Jan 2017 18:10:06 -0800 Subject: [PATCH] PackageVersion property for .NET Core projects Resolves NuGet/Home#3901. Acquire package version property for a .NET Core project from the `IVsProjectRestoreInfo` as provided via the Nominate API call. Although the `$(PackageVersion)` property shouldn't differ between different TFMs current API design assumes all project properties will be evaluated per each TFM. As a result `SolutionRestoreService` expects to get the same value of package version defined in each TFM property bag. Otherwise `InvalidOperationException` will be thrown. --- .../VsSolutionRestoreService.cs | 26 ++++ .../ProjectRestoreInfoBuilder.cs | 95 ++++++++++---- .../VsProjectProperties.cs | 5 + .../VsProjectRestoreInfo.cs | 2 +- .../VsReferenceProperties.cs | 5 + .../VsSolutionRestoreServiceTests.cs | 119 ++++++++++++------ .../VsTargetFrameworkInfo.cs | 15 +-- 7 files changed, 194 insertions(+), 73 deletions(-) diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/VsSolutionRestoreService.cs b/src/NuGet.Clients/NuGet.SolutionRestoreManager/VsSolutionRestoreService.cs index 67818d54514..f96bbe14c1b 100644 --- a/src/NuGet.Clients/NuGet.SolutionRestoreManager/VsSolutionRestoreService.cs +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/VsSolutionRestoreService.cs @@ -29,6 +29,8 @@ namespace NuGet.SolutionRestoreManager [Export(typeof(IVsSolutionRestoreService))] public sealed class VsSolutionRestoreService : IVsSolutionRestoreService { + private const string PackageVersion = nameof(PackageVersion); + private const string Version = nameof(Version); private const string IncludeAssets = "IncludeAssets"; private const string ExcludeAssets = "ExcludeAssets"; private const string PrivateAssets = "PrivateAssets"; @@ -196,6 +198,7 @@ private static PackageSpec ToPackageSpec(ProjectNames projectNames, IVsProjectRe var packageSpec = new PackageSpec(tfis) { Name = projectNames.ShortName, + Version = GetPackageVersion(projectRestoreInfo.TargetFrameworks), FilePath = projectFullPath, RestoreMetadata = new ProjectRestoreMetadata { @@ -220,6 +223,29 @@ private static PackageSpec ToPackageSpec(ProjectNames projectNames, IVsProjectRe return packageSpec; } + private static NuGetVersion GetPackageVersion(IVsTargetFrameworks tfms) + { + // $(PackageVersion) property if set overrides the $(Version) + var versionPropertyValue = + GetNonEvaluatedPropertyOrNull(tfms, PackageVersion) + ?? GetNonEvaluatedPropertyOrNull(tfms, Version); + + return versionPropertyValue != null + ? NuGetVersion.Parse(versionPropertyValue) + : PackageSpec.DefaultVersion; + } + + // Trying to fetch a property value from tfm property bags. + // If defined the property should have identical values in all of the occurances. + private static string GetNonEvaluatedPropertyOrNull(IVsTargetFrameworks tfms, string propertyName) + { + return tfms + .Cast() + .Select(tfm => GetPropertyValueOrNull(tfm.Properties, propertyName)) + .Distinct() + .SingleOrDefault(); + } + private static RuntimeGraph GetRuntimeGraph(IVsProjectRestoreInfo projectRestoreInfo) { var runtimes = projectRestoreInfo diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/ProjectRestoreInfoBuilder.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/ProjectRestoreInfoBuilder.cs index 99403b760a6..f134625b008 100644 --- a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/ProjectRestoreInfoBuilder.cs +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/ProjectRestoreInfoBuilder.cs @@ -13,18 +13,24 @@ namespace NuGet.SolutionRestoreManager.Test /// Helper class providing a method of building /// out of . /// - internal static class ProjectRestoreInfoBuilder + internal class ProjectRestoreInfoBuilder { + private readonly VsProjectRestoreInfo _pri; + + private ProjectRestoreInfoBuilder(VsProjectRestoreInfo pri) + { + _pri = pri; + } + /// /// Creates project restore info object to be consumed by . /// /// Source project restore object /// Desired project restore object - public static VsProjectRestoreInfo Build( + public static ProjectRestoreInfoBuilder FromPackageSpec( PackageSpec packageSpec, string baseIntermediatePath, - bool crossTargeting, - IEnumerable tools) + bool crossTargeting) { if (packageSpec == null) { @@ -36,18 +42,24 @@ public static VsProjectRestoreInfo Build( return null; } + var projectProperties = new VsProjectProperties { }; + + if (packageSpec.Version != null) + { + projectProperties = new VsProjectProperties + { + { "PackageVersion", packageSpec.Version.ToString() } + }; + } + var targetFrameworks = new VsTargetFrameworks( packageSpec .TargetFrameworks - .Select(ToTargetFrameworkInfo)); + .Select(tfm => ToTargetFrameworkInfo(tfm, projectProperties))); var pri = new VsProjectRestoreInfo( baseIntermediatePath, - targetFrameworks) - { - ToolReferences = new VsReferenceItems( - (tools ?? Enumerable.Empty()).Select(ToToolReference)) - }; + targetFrameworks); if (crossTargeting) { @@ -57,32 +69,61 @@ public static VsProjectRestoreInfo Build( .Select(tfm => tfm.FrameworkName.GetShortFolderName())); } - return pri; + return new ProjectRestoreInfoBuilder(pri); } - private static VsTargetFrameworkInfo ToTargetFrameworkInfo(TargetFrameworkInformation tfm) + public ProjectRestoreInfoBuilder WithTool(string name, string version) { - var packageReferences = new VsReferenceItems( - tfm.Dependencies - .Where(d => d.LibraryRange.TypeConstraint == LibraryDependencyTarget.Package) - .Select(ToPackageReference)); - - var projectReferences = new VsReferenceItems( - tfm.Dependencies - .Where(d => d.LibraryRange.TypeConstraint == LibraryDependencyTarget.ExternalProject) - .Select(ToProjectReference)); - - var projectProperties = new VsProjectProperties( - new VsProjectProperty( + var properties = new VsReferenceProperties + { + { "Version", version } + }; + + _pri.ToolReferences = new VsReferenceItems + { + new VsReferenceItem(name, properties) + }; + + return this; + } + + public ProjectRestoreInfoBuilder WithTargetFrameworkInfo( + IVsTargetFrameworkInfo tfi) + { + (_pri.TargetFrameworks as VsTargetFrameworks).Add(tfi); + + return this; + } + + public VsProjectRestoreInfo Build() => _pri; + + private static VsTargetFrameworkInfo ToTargetFrameworkInfo( + TargetFrameworkInformation tfm, + IEnumerable globalProperties) + { + var packageReferences = tfm + .Dependencies + .Where(d => d.LibraryRange.TypeConstraint == LibraryDependencyTarget.Package) + .Select(ToPackageReference); + + var projectReferences = tfm + .Dependencies + .Where(d => d.LibraryRange.TypeConstraint == LibraryDependencyTarget.ExternalProject) + .Select(ToProjectReference); + + var projectProperties = new VsProjectProperties + { + { "PackageTargetFallback", - string.Join(";", tfm.Imports.Select(x => x.GetShortFolderName()))) - ); + string.Join(";", tfm.Imports.Select(x => x.GetShortFolderName())) + } + }; return new VsTargetFrameworkInfo( tfm.FrameworkName.ToString(), packageReferences, projectReferences, - projectProperties); + projectProperties.Concat(globalProperties)); } private static IVsReferenceItem ToPackageReference(LibraryDependency library) diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsProjectProperties.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsProjectProperties.cs index 4450b9ec2c9..88e1263518a 100644 --- a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsProjectProperties.cs +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsProjectProperties.cs @@ -15,5 +15,10 @@ public VsProjectProperties(IEnumerable collection) : base(co public VsProjectProperties(params IVsProjectProperty[] collection) : base(collection) { } protected override string GetKeyForItem(IVsProjectProperty value) => value.Name; + + public void Add(string propertyName, string propertyValue) + { + Add(new VsProjectProperty(propertyName, propertyValue)); + } } } diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsProjectRestoreInfo.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsProjectRestoreInfo.cs index 875a7cac476..982b08ef017 100644 --- a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsProjectRestoreInfo.cs +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsProjectRestoreInfo.cs @@ -12,7 +12,7 @@ namespace NuGet.SolutionRestoreManager.Test /// internal class VsProjectRestoreInfo : IVsProjectRestoreInfo { - public String BaseIntermediatePath { get; } + public string BaseIntermediatePath { get; } public string OriginalTargetFrameworks { get; set; } diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsReferenceProperties.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsReferenceProperties.cs index c2cadb2084d..ed38cbd22d0 100644 --- a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsReferenceProperties.cs +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsReferenceProperties.cs @@ -13,5 +13,10 @@ public VsReferenceProperties() : base() { } public VsReferenceProperties(IEnumerable collection) : base(collection) { } protected override String GetKeyForItem(IVsReferenceProperty value) => value.Name; + + public void Add(string propertyName, string propertyValue) + { + Add(new VsReferenceProperty(propertyName, propertyValue)); + } } } diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsSolutionRestoreServiceTests.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsSolutionRestoreServiceTests.cs index 70bc3791716..3bb43626407 100644 --- a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsSolutionRestoreServiceTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsSolutionRestoreServiceTests.cs @@ -59,7 +59,7 @@ public void NominateProjectAsync_Always_SchedulesAutoRestore() cache, restoreWorker, NuGet.Common.NullLogger.Instance); // Act - var actualRestoreTask = service.NominateProjectAsync(cps.Item1, cps.Item3, CancellationToken.None); + var actualRestoreTask = service.NominateProjectAsync(cps.ProjectFullPath, cps.ProjectRestoreInfo, CancellationToken.None); Assert.Same(completedRestoreTask, actualRestoreTask); @@ -90,9 +90,9 @@ public async Task NominateProjectAsync_ConsoleAppTemplate() } }"; var projectName = "ConsoleApp1"; - var cps = NewCpsProject(projectName, consoleAppProjectJson); - var pri = cps.Item3; - var projectFullPath = cps.Item1; + var cps = NewCpsProject(consoleAppProjectJson, projectName); + var pri = cps.ProjectRestoreInfo; + var projectFullPath = cps.ProjectFullPath; // Act var actualRestoreSpec = await CaptureNominateResultAsync(projectFullPath, pri); @@ -102,13 +102,14 @@ public async Task NominateProjectAsync_ConsoleAppTemplate() var actualProjectSpec = actualRestoreSpec.GetProjectSpec(projectFullPath); Assert.NotNull(actualProjectSpec); + Assert.Equal("1.0.0", actualProjectSpec.Version.ToString()); var actualMetadata = actualProjectSpec.RestoreMetadata; Assert.NotNull(actualMetadata); Assert.Equal(projectFullPath, actualMetadata.ProjectPath); Assert.Equal(projectName, actualMetadata.ProjectName); Assert.Equal(ProjectStyle.PackageReference, actualMetadata.ProjectStyle); - Assert.Equal(cps.Item2, actualMetadata.OutputPath); + Assert.Equal(pri.BaseIntermediatePath, actualMetadata.OutputPath); Assert.Single(actualProjectSpec.TargetFrameworks); var actualTfi = actualProjectSpec.TargetFrameworks.Single(); @@ -129,17 +130,9 @@ public async Task NominateProjectAsync_ProjectWithTools() ""netcoreapp1.0"": { } } }"; - var cps = NewCpsProject( - projectJson: toolProjectJson, - tools: new[] - { - new LibraryRange( - "Foo.Test.Tools", - VersionRange.Parse("2.0.0"), - LibraryDependencyTarget.Package) - }); - var pri = cps.Item3; - var projectFullPath = cps.Item1; + var cps = NewCpsProject(toolProjectJson); + var pri = cps.Builder.WithTool("Foo.Test.Tools", "2.0.0").Build(); + var projectFullPath = cps.ProjectFullPath; // Act var actualRestoreSpec = await CaptureNominateResultAsync(projectFullPath, pri); @@ -195,8 +188,8 @@ public async Task NominateProjectAsync_CrossTargeting( var cps = NewCpsProject( projectJson: projectJson, crossTargeting: true); - var pri = cps.Item3; - var projectFullPath = cps.Item1; + var projectFullPath = cps.ProjectFullPath; + var pri = cps.ProjectRestoreInfo; pri.OriginalTargetFrameworks = rawOriginalTargetFrameworks; // Act @@ -228,13 +221,11 @@ public async Task NominateProjectAsync_Imports() } } }"; - var cps = NewCpsProject( - projectJson: projectJson); - var pri = cps.Item3; - var projectFullPath = cps.Item1; + var cps = NewCpsProject(projectJson); + var projectFullPath = cps.ProjectFullPath; // Act - var actualRestoreSpec = await CaptureNominateResultAsync(projectFullPath, pri); + var actualRestoreSpec = await CaptureNominateResultAsync(projectFullPath, cps.ProjectRestoreInfo); // Assert SpecValidationUtility.ValidateDependencySpec(actualRestoreSpec); @@ -247,6 +238,55 @@ public async Task NominateProjectAsync_Imports() Assert.Equal("dotnet5.3;portable-net452+win81", actualImports); } + [Fact] + public async Task NominateProjectAsync_WithValidPackageVersion_Passes() + { + const string projectJson = @"{ + ""version"": ""1.2.0-beta1"", + ""frameworks"": { + ""netcoreapp1.0"": { } + } +}"; + var cps = NewCpsProject(projectJson); + var projectFullPath = cps.ProjectFullPath; + + // Act + var actualRestoreSpec = await CaptureNominateResultAsync(projectFullPath, cps.ProjectRestoreInfo); + + // Assert + SpecValidationUtility.ValidateDependencySpec(actualRestoreSpec); + + var actualProjectSpec = actualRestoreSpec.GetProjectSpec(projectFullPath); + Assert.NotNull(actualProjectSpec); + Assert.Equal("1.2.0-beta1", actualProjectSpec.Version.ToString()); + } + + [Fact] + public async Task NominateProjectAsync_WithMultiplePackageVersions_Throws() + { + const string projectJson = @"{ + ""version"": ""1.2.0-beta1"", + ""frameworks"": { + ""netcoreapp1.0"": { } + } +}"; + var cps = NewCpsProject(projectJson); + var projectFullPath = cps.ProjectFullPath; + var pri = cps.Builder + .WithTargetFrameworkInfo( + new VsTargetFrameworkInfo( + "net46", + Enumerable.Empty(), + Enumerable.Empty(), + new[] { new VsProjectProperty("PackageVersion", "1.0.0") })) + .Build(); + + // Act & Assert + var ex = await Assert.ThrowsAsync(() => CaptureNominateResultAsync(projectFullPath, pri)); + + Assert.Equal("Sequence contains more than one element", ex.Message); + } + private async Task CaptureNominateResultAsync( string projectFullPath, IVsProjectRestoreInfo pri) { @@ -279,11 +319,10 @@ private async Task CaptureNominateResultAsync( return capturedRestoreSpec; } - private Tuple NewCpsProject( - string projectName = null, + private TestContext NewCpsProject( string projectJson = null, - bool crossTargeting = false, - IEnumerable tools = null) + string projectName = null, + bool crossTargeting = false) { const string DefaultProjectJson = @"{ ""frameworks"": { @@ -303,20 +342,16 @@ private Tuple NewCpsProject( Directory.CreateDirectory(baseIntermediatePath); var spec = JsonPackageSpecReader.GetPackageSpec(projectJson ?? DefaultProjectJson, projectName, projectFullPath); - var pri = ProjectRestoreInfoBuilder.Build( + var builder = ProjectRestoreInfoBuilder.FromPackageSpec( spec, baseIntermediatePath, - crossTargeting, - tools); - return Tuple.Create(projectFullPath, baseIntermediatePath, pri); - } + crossTargeting); - private static IVsReferenceItem ToReferenceItem(string itemName, string versionRange) - { - var properties = new VsReferenceProperties( - new[] { new VsReferenceProperty("Version", versionRange) } - ); - return new VsReferenceItem(itemName, properties); + return new TestContext + { + ProjectFullPath = projectFullPath, + Builder = builder + }; } private static void AssertPackages(TargetFrameworkInformation actualTfi, params string[] expectedPackages) @@ -328,5 +363,13 @@ private static void AssertPackages(TargetFrameworkInformation actualTfi, params Assert.Equal(expectedPackages, actualPackages); } + + private class TestContext + { + public string ProjectFullPath { get; set; } + public ProjectRestoreInfoBuilder Builder { get; set; } + + public VsProjectRestoreInfo ProjectRestoreInfo => Builder.Build(); + } } } diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsTargetFrameworkInfo.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsTargetFrameworkInfo.cs index dd76e26f5a2..3ff7265e3f9 100644 --- a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsTargetFrameworkInfo.cs +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsTargetFrameworkInfo.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; namespace NuGet.SolutionRestoreManager.Test { @@ -13,13 +14,13 @@ internal class VsTargetFrameworkInfo : IVsTargetFrameworkInfo public IVsProjectProperties Properties { get; } - public String TargetFrameworkMoniker { get; } + public string TargetFrameworkMoniker { get; } public VsTargetFrameworkInfo( string targetFrameworkMoniker, - IVsReferenceItems packageReferences, - IVsReferenceItems projectReferences, - IVsProjectProperties projectProperties) + IEnumerable packageReferences, + IEnumerable projectReferences, + IEnumerable projectProperties) { if (string.IsNullOrEmpty(targetFrameworkMoniker)) { @@ -42,9 +43,9 @@ public VsTargetFrameworkInfo( } TargetFrameworkMoniker = targetFrameworkMoniker; - PackageReferences = packageReferences; - ProjectReferences = projectReferences; - Properties = projectProperties; + PackageReferences = new VsReferenceItems(packageReferences); + ProjectReferences = new VsReferenceItems(projectReferences); + Properties = new VsProjectProperties(projectProperties); } } }