From 78a36c3fb06557609e32206bcad9ce9841756f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20P=C3=A4nk=C3=A4l=C3=A4inen?= Date: Thu, 9 Feb 2023 11:32:37 +0200 Subject: [PATCH 1/6] Added support for conditionals in NuGet package references --- Sharpmake/PackageReferences.Template.cs | 2 +- Sharpmake/PackageReferences.cs | 26 ++++++++++++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/Sharpmake/PackageReferences.Template.cs b/Sharpmake/PackageReferences.Template.cs index 5a24e7e4a..edb4ccaf4 100644 --- a/Sharpmake/PackageReferences.Template.cs +++ b/Sharpmake/PackageReferences.Template.cs @@ -8,7 +8,7 @@ public partial class PackageReferences /// /// See : https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#controlling-dependency-assets /// - private const string TemplateBeginPackageReference = " { - internal PackageReference(string name, string version, string dotNetHint, AssetsDependency privateAssets, string referenceType) + internal PackageReference(string name, string version, string dotNetHint, AssetsDependency privateAssets, string referenceType, string condition ) { Name = name; Version = version; DotNetHint = dotNetHint; PrivateAssets = privateAssets; ReferenceType = referenceType; + Condition = condition; } internal PackageReference(string name, string version, string dotNetHint, AssetsDependency privateAssets) - : this(name, version, dotNetHint, privateAssets, null) + : this(name, version, dotNetHint, privateAssets, null, null ) { } @@ -36,6 +37,7 @@ internal PackageReference(string name, string version, string dotNetHint, Assets public string Version { get; internal set; } public string DotNetHint { get; internal set; } public string ReferenceType { get; internal set; } + public string Condition { get; internal set; } public AssetsDependency PrivateAssets { get; internal set; } @@ -43,6 +45,7 @@ public string Resolve(Resolver resolver) { using (resolver.NewScopedParameter("packageName", Name)) using (resolver.NewScopedParameter("packageVersion", Version)) + using (resolver.NewScopedParameter("packageCondition", GetConditionParameter() )) { if (PrivateAssets == DefaultPrivateAssets) { @@ -60,11 +63,17 @@ public string Resolve(Resolver resolver, string customTemplate) { using (resolver.NewScopedParameter("packageName", Name)) using (resolver.NewScopedParameter("packageVersion", Version)) + using (resolver.NewScopedParameter("packageCondition", GetConditionParameter())) { return resolver.Resolve(customTemplate); } } + private string GetConditionParameter() + { + return string.IsNullOrWhiteSpace(Condition) ? string.Empty : $"Condition=\"{Condition}\""; + } + public int CompareTo(PackageReference other) { if (ReferenceEquals(this, other)) @@ -141,13 +150,13 @@ internal static IEnumerable GetFormatedAssetsDependency(AssetsDependency private readonly UniqueList _packageReferences = new UniqueList(); - public void Add(string packageName, string version, string dotNetHint = null, AssetsDependency privateAssets = DefaultPrivateAssets, string referenceType = null) + public void Add(string packageName, string version, string dotNetHint = null, AssetsDependency privateAssets = DefaultPrivateAssets, string referenceType = null, string condition = null ) { // check package unicity var existingPackage = _packageReferences.FirstOrDefault(pr => pr.Name == packageName); if (existingPackage == null) { - _packageReferences.Add(new PackageReference(packageName, version, null, privateAssets, referenceType)); + _packageReferences.Add(new PackageReference(packageName, version, null, privateAssets, referenceType, condition )); return; } @@ -162,11 +171,18 @@ public void Add(string packageName, string version, string dotNetHint = null, As existingPackage.PrivateAssets &= privateAssets; Builder.Instance.LogWarningLine($"Package {packageName} was added twice with different private assets. Kept assets are {string.Join(",", PackageReference.GetFormatedAssetsDependency(existingPackage.PrivateAssets))}."); } + + if ( !string.Equals( condition, existingPackage.Condition, StringComparison.Ordinal )) + { + existingPackage.Condition = condition; + Builder.Instance.LogWarningLine( + $"Package {packageName} was added twice with different conditions. Condition '{condition}' will be used."); + } } public void Add(string packageName, string version, string dotNetHint, AssetsDependency privateAssets) { - Add(packageName, version, dotNetHint, privateAssets, null); + Add(packageName, version, dotNetHint, privateAssets, null, null); } public int Count => _packageReferences.Count; From 248be28d98ac98747c0b8b399a851307b7410f15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20P=C3=A4nk=C3=A4l=C3=A4inen?= Date: Thu, 9 Feb 2023 15:18:32 +0200 Subject: [PATCH 2/6] Added a unit test for the conditions in PackageReferences --- Sharpmake.UnitTests/PackageReferencesTest.cs | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/Sharpmake.UnitTests/PackageReferencesTest.cs b/Sharpmake.UnitTests/PackageReferencesTest.cs index 246f55561..9ef1fc7dc 100644 --- a/Sharpmake.UnitTests/PackageReferencesTest.cs +++ b/Sharpmake.UnitTests/PackageReferencesTest.cs @@ -61,6 +61,27 @@ public void InheritedPrivatePackageReferencesToPublicBecomePublic() Assert.True(configuration.ReferencesByNuGetPackage.SortedValues.Any(item => item.PrivateAssets == PackageReferences.DefaultPrivateAssets && item.Name == "NUnit")); } } + + [Test] + public void PackageReferencesSupportConditions() + { + var project = GetProject(); + Assert.IsNotNull(project); + foreach (var configuration in project.Configurations) + { + Assert.AreEqual(2, configuration.ReferencesByNuGetPackage.Count); + CheckForPackageCondition("NUnit", "$(Configuration)='Release'"); + CheckForPackageCondition("NUnit-debug", "$(Configuration)='Debug'"); + + void CheckForPackageCondition( string packageName, string expectedCondition ) + { + Assert.AreEqual( + configuration.ReferencesByNuGetPackage.SortedValues + .First(v => string.Equals(v.Name, packageName, StringComparison.Ordinal)).Condition, + expectedCondition); + } + } + } } namespace PackageReferencesTestProjects @@ -127,5 +148,16 @@ public override void ConfigureAll(Configuration conf, Target target) conf.ReferencesByNuGetPackage.Add("NUnit", "3.4.1"); } } + + [Generate] + public class ConditionalNuGetPackage : CSharpUnitTestCommonProject + { + [Configure()] + public virtual void ConfigureAll(Configuration conf, Target target) + { + conf.ReferencesByNuGetPackage.Add("NUnit", "3.4.1", condition: "$(Configuration)='Release'" ); + conf.ReferencesByNuGetPackage.Add("NUnit-debug", "3.4.1", condition: "$(Configuration)='Debug'" ); + } + } } } From 2c5caf7c0539f8e965b8768f04f79ef6f4535fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20P=C3=A4nk=C3=A4l=C3=A4inen?= Date: Mon, 27 Mar 2023 11:05:06 +0300 Subject: [PATCH 3/6] Fixed unit test compilation --- Sharpmake.UnitTests/PackageReferencesTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Sharpmake.UnitTests/PackageReferencesTest.cs b/Sharpmake.UnitTests/PackageReferencesTest.cs index 9ef1fc7dc..60655604f 100644 --- a/Sharpmake.UnitTests/PackageReferencesTest.cs +++ b/Sharpmake.UnitTests/PackageReferencesTest.cs @@ -1,6 +1,7 @@ // Copyright (c) Ubisoft. All Rights Reserved. // Licensed under the Apache 2.0 License. See LICENSE.md in the project root for license information. +using System; using System.Linq; using NUnit.Framework; From 9f2eb8c6d3915a58c8fd73770db61d5d0e2aadaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20P=C3=A4nk=C3=A4l=C3=A4inen?= Date: Mon, 27 Mar 2023 11:06:48 +0300 Subject: [PATCH 4/6] Moved the private function to end of class --- Sharpmake/PackageReferences.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Sharpmake/PackageReferences.cs b/Sharpmake/PackageReferences.cs index f5bcc6a95..72896ea04 100644 --- a/Sharpmake/PackageReferences.cs +++ b/Sharpmake/PackageReferences.cs @@ -69,11 +69,6 @@ public string Resolve(Resolver resolver, string customTemplate) } } - private string GetConditionParameter() - { - return string.IsNullOrWhiteSpace(Condition) ? string.Empty : $"Condition=\"{Condition}\""; - } - public int CompareTo(PackageReference other) { if (ReferenceEquals(this, other)) @@ -146,6 +141,11 @@ internal static IEnumerable GetFormatedAssetsDependency(AssetsDependency yield return "buildTransitive"; } } + + private string GetConditionParameter() + { + return string.IsNullOrWhiteSpace(Condition) ? string.Empty : $"Condition=\"{Condition}\""; + } } private readonly UniqueList _packageReferences = new UniqueList(); From 33c7a719468052d859ba2c56b93ebef6766101c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20P=C3=A4nk=C3=A4l=C3=A4inen?= Date: Mon, 27 Mar 2023 11:10:22 +0300 Subject: [PATCH 5/6] Fixed formatting --- Sharpmake.UnitTests/PackageReferencesTest.cs | 2 +- Sharpmake/PackageReferences.cs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Sharpmake.UnitTests/PackageReferencesTest.cs b/Sharpmake.UnitTests/PackageReferencesTest.cs index 60655604f..357420983 100644 --- a/Sharpmake.UnitTests/PackageReferencesTest.cs +++ b/Sharpmake.UnitTests/PackageReferencesTest.cs @@ -74,7 +74,7 @@ public void PackageReferencesSupportConditions() CheckForPackageCondition("NUnit", "$(Configuration)='Release'"); CheckForPackageCondition("NUnit-debug", "$(Configuration)='Debug'"); - void CheckForPackageCondition( string packageName, string expectedCondition ) + void CheckForPackageCondition(string packageName, string expectedCondition) { Assert.AreEqual( configuration.ReferencesByNuGetPackage.SortedValues diff --git a/Sharpmake/PackageReferences.cs b/Sharpmake/PackageReferences.cs index 72896ea04..a128b1c34 100644 --- a/Sharpmake/PackageReferences.cs +++ b/Sharpmake/PackageReferences.cs @@ -18,7 +18,7 @@ public partial class PackageReferences [DebuggerDisplay("{Name} {Version}")] public class PackageReference : IResolverHelper, IComparable { - internal PackageReference(string name, string version, string dotNetHint, AssetsDependency privateAssets, string referenceType, string condition ) + internal PackageReference(string name, string version, string dotNetHint, AssetsDependency privateAssets, string referenceType, string condition) { Name = name; Version = version; @@ -29,7 +29,7 @@ internal PackageReference(string name, string version, string dotNetHint, Assets } internal PackageReference(string name, string version, string dotNetHint, AssetsDependency privateAssets) - : this(name, version, dotNetHint, privateAssets, null, null ) + : this(name, version, dotNetHint, privateAssets, null, null) { } @@ -150,7 +150,7 @@ private string GetConditionParameter() private readonly UniqueList _packageReferences = new UniqueList(); - public void Add(string packageName, string version, string dotNetHint = null, AssetsDependency privateAssets = DefaultPrivateAssets, string referenceType = null, string condition = null ) + public void Add(string packageName, string version, string dotNetHint = null, AssetsDependency privateAssets = DefaultPrivateAssets, string referenceType = null, string condition = null) { // check package unicity var existingPackage = _packageReferences.FirstOrDefault(pr => pr.Name == packageName); @@ -172,7 +172,7 @@ public void Add(string packageName, string version, string dotNetHint = null, As Builder.Instance.LogWarningLine($"Package {packageName} was added twice with different private assets. Kept assets are {string.Join(",", PackageReference.GetFormatedAssetsDependency(existingPackage.PrivateAssets))}."); } - if ( !string.Equals( condition, existingPackage.Condition, StringComparison.Ordinal )) + if (!string.Equals(condition, existingPackage.Condition, StringComparison.Ordinal)) { existingPackage.Condition = condition; Builder.Instance.LogWarningLine( From 52efb13467fa07c345e420bdb0f52589b5f921e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20P=C3=A4nk=C3=A4l=C3=A4inen?= Date: Thu, 30 Mar 2023 11:22:43 +0300 Subject: [PATCH 6/6] Fixed an extra whitespace when no conditionals were specified for a package reference --- Sharpmake/PackageReferences.Template.cs | 2 +- Sharpmake/PackageReferences.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sharpmake/PackageReferences.Template.cs b/Sharpmake/PackageReferences.Template.cs index edb4ccaf4..72b760fea 100644 --- a/Sharpmake/PackageReferences.Template.cs +++ b/Sharpmake/PackageReferences.Template.cs @@ -8,7 +8,7 @@ public partial class PackageReferences /// /// See : https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#controlling-dependency-assets /// - private const string TemplateBeginPackageReference = " GetFormatedAssetsDependency(AssetsDependency private string GetConditionParameter() { - return string.IsNullOrWhiteSpace(Condition) ? string.Empty : $"Condition=\"{Condition}\""; + return string.IsNullOrWhiteSpace(Condition) ? string.Empty : $" Condition=\"{Condition}\""; } }