From 9ae354d2069bdf703cd69e7c59adc68faf9fcd0e Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 29 Sep 2020 14:05:19 -0300 Subject: [PATCH 1/9] Add separate scenario and tests for sourcelink --- .../empty_library_project.csproj | 1 - .../Scenarios/given_sourcelink/library.csproj | 12 +++++ src/NuGetizer.Tests/given_an_empty_library.cs | 36 ------------- src/NuGetizer.Tests/given_sourcelink.cs | 54 +++++++++++++++++++ 4 files changed, 66 insertions(+), 37 deletions(-) create mode 100644 src/NuGetizer.Tests/Scenarios/given_sourcelink/library.csproj create mode 100644 src/NuGetizer.Tests/given_sourcelink.cs diff --git a/src/NuGetizer.Tests/Scenarios/given_an_empty_library/empty_library_project.csproj b/src/NuGetizer.Tests/Scenarios/given_an_empty_library/empty_library_project.csproj index ba45f69b..ec537fe5 100644 --- a/src/NuGetizer.Tests/Scenarios/given_an_empty_library/empty_library_project.csproj +++ b/src/NuGetizer.Tests/Scenarios/given_an_empty_library/empty_library_project.csproj @@ -6,7 +6,6 @@ - diff --git a/src/NuGetizer.Tests/Scenarios/given_sourcelink/library.csproj b/src/NuGetizer.Tests/Scenarios/given_sourcelink/library.csproj new file mode 100644 index 00000000..12c0fa93 --- /dev/null +++ b/src/NuGetizer.Tests/Scenarios/given_sourcelink/library.csproj @@ -0,0 +1,12 @@ + + + + + netstandard2.0 + + + + + + + \ No newline at end of file diff --git a/src/NuGetizer.Tests/given_an_empty_library.cs b/src/NuGetizer.Tests/given_an_empty_library.cs index 8bb166e2..8ac6557b 100644 --- a/src/NuGetizer.Tests/given_an_empty_library.cs +++ b/src/NuGetizer.Tests/given_an_empty_library.cs @@ -165,42 +165,6 @@ public void when_include_framework_references_in_package_is_false_then_does_not_ })); } - [Fact] - public void when_getting_metadata_then_adds_repository_info() - { - var result = Builder.BuildScenario(nameof(given_an_empty_library), new - { - PackageId = "Foo", - PublishRepositoryUrl = "true", - }, target: "GetPackageMetadata", output: output); - - result.AssertSuccess(output); - - Assert.Single(result.Items); - var metadata = result.Items[0]; - - Assert.Equal("git", metadata.GetMetadata("RepositoryType")); - Assert.Equal("https://github.com/kzu/NuGetizer", metadata.GetMetadata("RepositoryUrl")); - Assert.NotEmpty(metadata.GetMetadata("RepositoryCommit")); - } - - [Fact] - public void when_getting_metadata_with_no_explicit_publish_repo_url_then_does_not_expose_it() - { - var result = Builder.BuildScenario(nameof(given_an_empty_library), new - { - PackageId = "Foo", - }, target: "GetPackageMetadata", output: output); - - result.AssertSuccess(output); - - Assert.Single(result.Items); - var metadata = result.Items[0]; - - Assert.Empty(metadata.GetMetadata("RepositoryUrl")); - Assert.NotEmpty(metadata.GetMetadata("RepositoryCommit")); - } - [Fact] public void when_updating_package_item_metadata_then_updates_metadata() { diff --git a/src/NuGetizer.Tests/given_sourcelink.cs b/src/NuGetizer.Tests/given_sourcelink.cs new file mode 100644 index 00000000..e61eccbe --- /dev/null +++ b/src/NuGetizer.Tests/given_sourcelink.cs @@ -0,0 +1,54 @@ +using Microsoft.Build.Execution; +using Xunit; +using Xunit.Abstractions; + +namespace NuGetizer +{ + public class given_sourcelink + { + ITestOutputHelper output; + + public given_sourcelink(ITestOutputHelper output) + { + this.output = output; + Builder.BuildScenario(nameof(given_sourcelink), target: "Restore") + .AssertSuccess(output); + } + + [Fact] + public void when_getting_metadata_then_adds_repository_info() + { + var result = Builder.BuildScenario(nameof(given_an_empty_library), new + { + PackageId = "Foo", + PublishRepositoryUrl = "true", + }, target: "GetPackageMetadata", output: output); + + result.AssertSuccess(output); + + Assert.Single(result.Items); + var metadata = result.Items[0]; + + Assert.Equal("git", metadata.GetMetadata("RepositoryType")); + Assert.Equal("https://github.com/kzu/NuGetizer", metadata.GetMetadata("RepositoryUrl")); + Assert.NotEmpty(metadata.GetMetadata("RepositoryCommit")); + } + + [Fact] + public void when_getting_metadata_with_no_explicit_publish_repo_url_then_does_not_expose_it() + { + var result = Builder.BuildScenario(nameof(given_an_empty_library), new + { + PackageId = "Foo", + }, target: "GetPackageMetadata", output: output); + + result.AssertSuccess(output); + + Assert.Single(result.Items); + var metadata = result.Items[0]; + + Assert.Empty(metadata.GetMetadata("RepositoryUrl")); + Assert.NotEmpty(metadata.GetMetadata("RepositoryCommit")); + } + } +} From 831873f948528a7fd33988e119d37dd5c0a05427 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 29 Sep 2020 16:29:21 -0300 Subject: [PATCH 2/9] Simplify condition when calculating BuildOutputFrameworkSpecific --- src/NuGetizer.Tasks/NuGetizer.Inference.targets | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/NuGetizer.Tasks/NuGetizer.Inference.targets b/src/NuGetizer.Tasks/NuGetizer.Inference.targets index 29b9040c..a7f3ee69 100644 --- a/src/NuGetizer.Tasks/NuGetizer.Inference.targets +++ b/src/NuGetizer.Tasks/NuGetizer.Inference.targets @@ -59,12 +59,12 @@ Copyright (c) .NET Foundation. All rights reserved. - + - + <_BuildOutputKindFrameworkSpecific Include="@(PackageItemKind->'%(FrameworkSpecific)')" Condition="'%(Identity)' == '$(BuildOutputKind)'" /> - + @(_BuildOutputKindFrameworkSpecific) @@ -191,17 +191,18 @@ Copyright (c) .NET Foundation. All rights reserved. <_PrivateAssets>@(_PrimaryPackageReference -> '%(PrivateAssets)') + <_ShouldPack>@(_PrimaryPackageReference -> '%(Pack)') <_ShouldIncludeAssetsRegex>$(_NuGetPackageId)\\.+\\$(_PrivateAssets)\\.* - + <_InferredPackageFile Include="@(_PrimaryOutputRelatedFile)" Condition="'%(_PrimaryOutputRelatedFile.FrameworkFile)' != 'true'"> $(BuildOutputKind) $(BuildOutputFrameworkSpecific) - + From 1cb6363032c31a8f8f348dac2cccf35667f19c10 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 29 Sep 2020 16:30:47 -0300 Subject: [PATCH 3/9] Add support for building inline project content This might make it easier for simple project scenarios, also making it easier to understand what files are used in which tests. Currently, this is a bit indirect via the test class name and the corresponding folder under Scenarios. --- src/NuGetizer.Tests/Builder.NuGetizer.cs | 325 +++++++++++++---------- src/NuGetizer.Tests/Utilities/Base62.cs | 53 ++++ 2 files changed, 238 insertions(+), 140 deletions(-) create mode 100644 src/NuGetizer.Tests/Utilities/Base62.cs diff --git a/src/NuGetizer.Tests/Builder.NuGetizer.cs b/src/NuGetizer.Tests/Builder.NuGetizer.cs index 2c4622ba..9f6f4c25 100644 --- a/src/NuGetizer.Tests/Builder.NuGetizer.cs +++ b/src/NuGetizer.Tests/Builder.NuGetizer.cs @@ -4,7 +4,11 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; +using System.Security.Cryptography; +using System.Text; using System.Threading; +using System.Xml.Linq; using Microsoft.Build.Execution; using Microsoft.Build.Framework; using Microsoft.Build.Logging.StructuredLogger; @@ -17,50 +21,84 @@ /// static partial class Builder { + public static TargetResult BuildProject( + string projectContent = null, + string target = "GetPackageContents", + ITestOutputHelper output = null, + LoggerVerbosity? verbosity = null) + { + using var sha = new SHA1Managed(); + var hash = Base62.Encode(BitConverter.ToInt64( + sha.ComputeHash(Encoding.UTF8.GetBytes(projectContent)), 0)); + + var scenarioName = hash; + var scenarioDir = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Scenarios", scenarioName); + Directory.CreateDirectory(scenarioDir); + var doc = XDocument.Parse(projectContent); + doc.Root.AddFirst(XElement + .Parse("")); + + doc.Save(Path.Combine(scenarioDir, "library.csproj")); + + var openLog = OpenBuildLogAttribute.IsActive; + try + { + OpenBuildLogAttribute.IsActive = false; + BuildScenario(scenarioName, target: "Restore", output: output, verbosity: verbosity) + .AssertSuccess(output); + } + finally + { + OpenBuildLogAttribute.IsActive = openLog; + } + + return BuildScenario(scenarioName, target: target, output: output, verbosity: verbosity); + } + public static TargetResult BuildScenario( - string scenarioName, - object properties = null, - string projectName = null, - string target = "GetPackageContents", - ITestOutputHelper output = null, - LoggerVerbosity? verbosity = null) - { - var scenarioDir = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Scenarios", scenarioName); - - if (projectName == null) - { - projectName = scenarioName; - if (scenarioName.StartsWith("given", StringComparison.OrdinalIgnoreCase)) - projectName = string.Join("_", scenarioName.Split('_').Skip(2)); - } - else if (!File.Exists(Path.Combine(scenarioDir, $"{projectName}.csproj"))) - { - throw new FileNotFoundException($"Project '{projectName}' was not found under scenario path '{scenarioDir}'.", projectName); - } - - string projectOrSolution; - - if (File.Exists(Path.Combine(scenarioDir, $"{projectName}.csproj"))) - projectOrSolution = Path.Combine(scenarioDir, $"{projectName}.csproj"); - else - projectOrSolution = Directory.EnumerateFiles(scenarioDir, "*.*proj").FirstOrDefault(); - - var logger = default(TestOutputLogger); - if (output != null) - { - if (verbosity == null) - { - var data = Thread.GetNamedDataSlot(nameof(LoggerVerbosity)); - if (data != null) - verbosity = (LoggerVerbosity?)Thread.GetData(data); - } - - logger = new TestOutputLogger(output, verbosity); - } - else - { - logger = new TestOutputLogger(null); - } + string scenarioName, + object properties = null, + string projectName = null, + string target = "GetPackageContents", + ITestOutputHelper output = null, + LoggerVerbosity? verbosity = null) + { + var scenarioDir = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Scenarios", scenarioName); + + if (projectName == null) + { + projectName = scenarioName; + if (scenarioName.StartsWith("given", StringComparison.OrdinalIgnoreCase)) + projectName = string.Join("_", scenarioName.Split('_').Skip(2)); + } + else if (!File.Exists(Path.Combine(scenarioDir, $"{projectName}.csproj"))) + { + throw new FileNotFoundException($"Project '{projectName}' was not found under scenario path '{scenarioDir}'.", projectName); + } + + string projectOrSolution; + + if (File.Exists(Path.Combine(scenarioDir, $"{projectName}.csproj"))) + projectOrSolution = Path.Combine(scenarioDir, $"{projectName}.csproj"); + else + projectOrSolution = Directory.EnumerateFiles(scenarioDir, "*.*proj").FirstOrDefault(); + + var logger = default(TestOutputLogger); + if (output != null) + { + if (verbosity == null) + { + var data = Thread.GetNamedDataSlot(nameof(LoggerVerbosity)); + if (data != null) + verbosity = (LoggerVerbosity?)Thread.GetData(data); + } + + logger = new TestOutputLogger(output, verbosity); + } + else + { + logger = new TestOutputLogger(null); + } var logFile = scenarioName + ".binlog"; @@ -74,66 +112,66 @@ public static TargetResult BuildScenario( } var loggers = OpenBuildLogAttribute.IsActive || Environment.GetEnvironmentVariable("SYSTEM_DEBUG") == "true" ? - new ILogger[] { logger, new StructuredLogger - { + new ILogger[] { logger, new StructuredLogger + { Verbosity = LoggerVerbosity.Diagnostic, Parameters = logFile } } : new ILogger[] { logger }; - var buildProps = properties?.GetType().GetProperties() - .ToDictionary(prop => prop.Name, prop => prop.GetValue(properties).ToString()) - ?? new Dictionary(); + var buildProps = properties?.GetType().GetProperties() + .ToDictionary(prop => prop.Name, prop => prop.GetValue(properties).ToString()) + ?? new Dictionary(); - buildProps[nameof(ThisAssembly.Project.NuGetRestoreTargets)] = ThisAssembly.Project.NuGetRestoreTargets; - buildProps[nameof(ThisAssembly.Project.NuGetTargets)] = ThisAssembly.Project.NuGetTargets; + buildProps[nameof(ThisAssembly.Project.NuGetRestoreTargets)] = ThisAssembly.Project.NuGetRestoreTargets; + buildProps[nameof(ThisAssembly.Project.NuGetTargets)] = ThisAssembly.Project.NuGetTargets; - var result = Build(projectOrSolution, target, buildProps, loggers); - if (OpenBuildLogAttribute.IsActive) - Process.Start(scenarioName + ".binlog"); + var result = Build(projectOrSolution, target, buildProps, loggers); + if (OpenBuildLogAttribute.IsActive) + Process.Start(scenarioName + ".binlog"); - return new TargetResult(projectOrSolution, result, target, logger); - } + return new TargetResult(projectOrSolution, result, target, logger); + } - public class TargetResult : ITargetResult - { - public TargetResult(string projectOrSolutionFile, BuildResult result, string target, TestOutputLogger logger) - { - ProjectOrSolutionFile = projectOrSolutionFile; - BuildResult = result; - Target = target; - Logger = logger; - } + public class TargetResult : ITargetResult + { + public TargetResult(string projectOrSolutionFile, BuildResult result, string target, TestOutputLogger logger) + { + ProjectOrSolutionFile = projectOrSolutionFile; + BuildResult = result; + Target = target; + Logger = logger; + } - public string ProjectOrSolutionFile { get; private set; } + public string ProjectOrSolutionFile { get; private set; } - public BuildResult BuildResult { get; private set; } + public BuildResult BuildResult { get; private set; } - public TestOutputLogger Logger { get; private set; } + public TestOutputLogger Logger { get; private set; } - public string Target { get; private set; } + public string Target { get; private set; } - public Exception Exception => BuildResult[Target].Exception; + public Exception Exception => BuildResult[Target].Exception; - public ITaskItem[] Items => BuildResult[Target].Items; + public ITaskItem[] Items => BuildResult[Target].Items; - public TargetResultCode ResultCode => BuildResult[Target].ResultCode; + public TargetResultCode ResultCode => BuildResult[Target].ResultCode; - public void AssertSuccess(ITestOutputHelper output) - { - if (!BuildResult.ResultsByTarget.ContainsKey(Target)) - { - output.WriteLine(this.ToString()); - Assert.False(true, "Build results do not contain output for target " + Target); - } + public void AssertSuccess(ITestOutputHelper output) + { + if (!BuildResult.ResultsByTarget.ContainsKey(Target)) + { + output.WriteLine(this.ToString()); + Assert.False(true, "Build results do not contain output for target " + Target); + } - if (ResultCode != TargetResultCode.Success) - output.WriteLine(this.ToString()); + if (ResultCode != TargetResultCode.Success) + output.WriteLine(this.ToString()); - Assert.Equal(TargetResultCode.Success, ResultCode); - } + Assert.Equal(TargetResultCode.Success, ResultCode); + } - public override string ToString() + public override string ToString() { return string.Join(Environment.NewLine, Logger.Warnings .Select(e => e.Message) @@ -148,50 +186,57 @@ public override string ToString() /// internal class OpenBuildLogAttribute : BeforeAfterTestAttribute { - /// - /// Whether the attribute is active for the current test. - /// - public static bool IsActive - { - get - { + /// + /// Whether the attribute is active for the current test. + /// + public static bool IsActive + { + get + { #if DEBUG - var data = Thread.GetNamedDataSlot(nameof(OpenBuildLogAttribute)); - if (data == null) - return false; + var data = Thread.GetNamedDataSlot(nameof(OpenBuildLogAttribute)); + if (data == null) + return false; - return Thread.GetData(data) != null; + return Thread.GetData(data) != null; #else return false; #endif - } - } - - public override void Before(MethodInfo methodUnderTest) - { - // Don't ever set this flag on release builds just in case - // we forget the attribute in a commit ;) + } + set + { + // Don't ever set this flag on release builds just in case + // we forget the attribute in a commit ;) #if DEBUG - var data = Thread.GetNamedDataSlot(nameof(OpenBuildLogAttribute)); - if (data == null) - data = Thread.AllocateNamedDataSlot(nameof(OpenBuildLogAttribute)); - - Thread.SetData(data, new object()); + if (value) + { + var data = Thread.GetNamedDataSlot(nameof(OpenBuildLogAttribute)); + if (data == null) + data = Thread.AllocateNamedDataSlot(nameof(OpenBuildLogAttribute)); + + Thread.SetData(data, new object()); + } + else + { + var data = Thread.GetNamedDataSlot(nameof(OpenBuildLogAttribute)); + if (data != null) + Thread.SetData(data, null); #endif + } + } + } - base.Before(methodUnderTest); - } - - public override void After(MethodInfo methodUnderTest) - { -#if DEBUG - var data = Thread.GetNamedDataSlot(nameof(OpenBuildLogAttribute)); - if (data != null) - Thread.SetData(data, null); -#endif + public override void Before(MethodInfo methodUnderTest) + { + IsActive = true; + base.Before(methodUnderTest); + } - base.After(methodUnderTest); - } + public override void After(MethodInfo methodUnderTest) + { + IsActive = false; + base.After(methodUnderTest); + } } /// @@ -201,36 +246,36 @@ public override void After(MethodInfo methodUnderTest) /// internal class VerbosityAttribute : BeforeAfterTestAttribute { - public VerbosityAttribute(LoggerVerbosity verbosity) - { - Verbosity = verbosity; - } + public VerbosityAttribute(LoggerVerbosity verbosity) + { + Verbosity = verbosity; + } - public LoggerVerbosity? Verbosity { get; } + public LoggerVerbosity? Verbosity { get; } - public override void Before(MethodInfo methodUnderTest) - { - // Don't ever set the verbosity on release builds just in case - // we forget the attribute in a commit ;) + public override void Before(MethodInfo methodUnderTest) + { + // Don't ever set the verbosity on release builds just in case + // we forget the attribute in a commit ;) #if DEBUG - var data = Thread.GetNamedDataSlot(nameof(LoggerVerbosity)); - if (data == null) - data = Thread.AllocateNamedDataSlot(nameof(LoggerVerbosity)); + var data = Thread.GetNamedDataSlot(nameof(LoggerVerbosity)); + if (data == null) + data = Thread.AllocateNamedDataSlot(nameof(LoggerVerbosity)); - Thread.SetData(data, Verbosity); + Thread.SetData(data, Verbosity); #endif - base.Before(methodUnderTest); - } + base.Before(methodUnderTest); + } - public override void After(MethodInfo methodUnderTest) - { + public override void After(MethodInfo methodUnderTest) + { #if DEBUG - var data = Thread.GetNamedDataSlot(nameof(LoggerVerbosity)); - if (data != null) - Thread.SetData(data, null); + var data = Thread.GetNamedDataSlot(nameof(LoggerVerbosity)); + if (data != null) + Thread.SetData(data, null); #endif - base.After(methodUnderTest); - } + base.After(methodUnderTest); + } } diff --git a/src/NuGetizer.Tests/Utilities/Base62.cs b/src/NuGetizer.Tests/Utilities/Base62.cs new file mode 100644 index 00000000..c988b993 --- /dev/null +++ b/src/NuGetizer.Tests/Utilities/Base62.cs @@ -0,0 +1,53 @@ +using System; +using System.Linq; +using System.Numerics; +using System.Text; + +/// +/// Inspired in a bunch of searches, samples and snippets on various languages +/// and blogs and what-not on doing URL shortering :), heavily tweaked to make +/// it fully idiomatic in C#. +/// +public static class Base62 +{ + /// + /// Encodes a numeric value into a base62 string. + /// + public static string Encode(BigInteger value) + { + // TODO: I'm almost sure there's a more succint way + // of doing this with LINQ and Aggregate, but just + // can't figure it out... + var sb = new StringBuilder(); + + while (value != 0) + { + sb = sb.Append(ToBase62(value % 62)); + value /= 62; + } + + return new string(sb.ToString().Reverse().ToArray()); + } + + /// + /// Decodes a base62 string into its original numeric value. + /// + public static BigInteger Decode(string value) + => value.Aggregate(new BigInteger(0), (current, c) => current * 62 + FromBase62(c)); + + static char ToBase62(BigInteger d) => d switch + { + BigInteger v when v < 10 => (char)('0' + d), + BigInteger v when v < 36 => (char)('A' + d - 10), + BigInteger v when v < 62 => (char)('a' + d - 36), + _ => throw new ArgumentException($"Cannot encode digit {d} to base 62.", nameof(d)), + }; + + static BigInteger FromBase62(char c) => c switch + { + char v when c >= 'a' && v <= 'z' => 36 + c - 'a', + char v when c >= 'A' && v <= 'Z' => 10 + c - 'A', + char v when c >= '0' && v <= '9' => c - '0', + _ => throw new ArgumentException($"Cannot decode char '{c}' from base 62.", nameof(c)), + }; +} From f9eafc0229558fda025018cca5c5348145add73e Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 29 Sep 2020 16:35:28 -0300 Subject: [PATCH 4/9] Convert SourceLink tests to inline projects --- .../Scenarios/given_sourcelink/library.csproj | 12 ------ src/NuGetizer.Tests/given_sourcelink.cs | 42 +++++++++++-------- 2 files changed, 25 insertions(+), 29 deletions(-) delete mode 100644 src/NuGetizer.Tests/Scenarios/given_sourcelink/library.csproj diff --git a/src/NuGetizer.Tests/Scenarios/given_sourcelink/library.csproj b/src/NuGetizer.Tests/Scenarios/given_sourcelink/library.csproj deleted file mode 100644 index 12c0fa93..00000000 --- a/src/NuGetizer.Tests/Scenarios/given_sourcelink/library.csproj +++ /dev/null @@ -1,12 +0,0 @@ - - - - - netstandard2.0 - - - - - - - \ No newline at end of file diff --git a/src/NuGetizer.Tests/given_sourcelink.cs b/src/NuGetizer.Tests/given_sourcelink.cs index e61eccbe..8d3ec070 100644 --- a/src/NuGetizer.Tests/given_sourcelink.cs +++ b/src/NuGetizer.Tests/given_sourcelink.cs @@ -1,5 +1,4 @@ -using Microsoft.Build.Execution; -using Xunit; +using Xunit; using Xunit.Abstractions; namespace NuGetizer @@ -8,21 +7,23 @@ public class given_sourcelink { ITestOutputHelper output; - public given_sourcelink(ITestOutputHelper output) - { - this.output = output; - Builder.BuildScenario(nameof(given_sourcelink), target: "Restore") - .AssertSuccess(output); - } + public given_sourcelink(ITestOutputHelper output) => this.output = output; [Fact] public void when_getting_metadata_then_adds_repository_info() { - var result = Builder.BuildScenario(nameof(given_an_empty_library), new - { - PackageId = "Foo", - PublishRepositoryUrl = "true", - }, target: "GetPackageMetadata", output: output); + var result = Builder.BuildProject(@" + + + Library + netstandard2.0 + true + + + + +", + "GetPackageMetadata", output); result.AssertSuccess(output); @@ -37,10 +38,17 @@ public void when_getting_metadata_then_adds_repository_info() [Fact] public void when_getting_metadata_with_no_explicit_publish_repo_url_then_does_not_expose_it() { - var result = Builder.BuildScenario(nameof(given_an_empty_library), new - { - PackageId = "Foo", - }, target: "GetPackageMetadata", output: output); + var result = Builder.BuildProject(@" + + + Library + netstandard2.0 + + + + +", + "GetPackageMetadata", output); result.AssertSuccess(output); From 56de45ebb18cbd7b0c93f0d1386f88e37fb00e77 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 29 Sep 2020 16:35:58 -0300 Subject: [PATCH 5/9] Add specific tests for package reference inference. --- .../given_packagereferences.cs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 src/NuGetizer.Tests/given_packagereferences.cs diff --git a/src/NuGetizer.Tests/given_packagereferences.cs b/src/NuGetizer.Tests/given_packagereferences.cs new file mode 100644 index 00000000..0b2cd4ba --- /dev/null +++ b/src/NuGetizer.Tests/given_packagereferences.cs @@ -0,0 +1,57 @@ +using Microsoft.Build.Execution; +using Xunit; +using Xunit.Abstractions; + +namespace NuGetizer +{ + public class given_packagereferences + { + ITestOutputHelper output; + + public given_packagereferences(ITestOutputHelper output) => this.output = output; + + [Fact] + public void when_privateassets_all_then_packs_library() + { + var result = Builder.BuildProject(@" + + + Library + netstandard2.0 + + + + +", + "GetPackageContents", output); + + result.AssertSuccess(output); + Assert.Contains(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Newtonsoft.Json.dll", + })); + } + + [Fact] + public void when_privateassets_pack_false_then_does_not_pack() + { + var result = Builder.BuildProject(@" + + + Library + netstandard2.0 + + + + +", + "GetPackageContents", output); + + result.AssertSuccess(output); + Assert.DoesNotContain(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Newtonsoft.Json.dll", + })); + } + } +} From c3551f89e51839212def33eaaf5fae5d3ed4ac22 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 29 Sep 2020 19:54:51 -0300 Subject: [PATCH 6/9] Sort root items in package first This is easier to visualize than the current ordering where they end up last. --- src/dotnet-nugetize/Program.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dotnet-nugetize/Program.cs b/src/dotnet-nugetize/Program.cs index b3eed1b3..90d1dd7b 100644 --- a/src/dotnet-nugetize/Program.cs +++ b/src/dotnet-nugetize/Program.cs @@ -104,7 +104,8 @@ static int Execute(bool binlog, bool debug) x.Element("PackageId")?.Value == packageId) .Select(x => x.Element("PackagePath").Value) .Distinct() - .OrderBy(x => x); + .OrderBy(x => Path.GetDirectoryName(x)) + .ThenBy(x => x); RenderContents(contents.ToList(), 0, 0, ""); Console.WriteLine(); From 437392f263873c946be47d5b95513a9ca0e73be1 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 29 Sep 2020 20:01:41 -0300 Subject: [PATCH 7/9] Default build package types to not include MSBuild deps Since the PrivateAssets attribute is not a good fit in this scenario (see https://github.com/NuGet/Home/issues/6041), use the common Pack metadata attribute and default it to false for MSBuild dependencies by default for build package kind. Fixes https://github.com/NuGet/Home/issues/6041 --- .../NuGetizer.Inference.targets | 11 ++++- src/NuGetizer.Tests/Builder.NuGetizer.cs | 4 +- .../given_packagereferences.cs | 49 +++++++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/NuGetizer.Tasks/NuGetizer.Inference.targets b/src/NuGetizer.Tasks/NuGetizer.Inference.targets index a7f3ee69..3043f848 100644 --- a/src/NuGetizer.Tasks/NuGetizer.Inference.targets +++ b/src/NuGetizer.Tasks/NuGetizer.Inference.targets @@ -36,6 +36,7 @@ Copyright (c) .NET Foundation. All rights reserved. $(GetPackageContentsDependsOn); _BuildOutputFrameworkSpecific; + _SetDefaultPackageReferencePack; InferPackageContents @@ -68,6 +69,14 @@ Copyright (c) .NET Foundation. All rights reserved. @(_BuildOutputKindFrameworkSpecific) + + + + + + @@ -132,7 +141,7 @@ Copyright (c) .NET Foundation. All rights reserved. Condition="'%(_NoneToInfer.CopyToOutputDirectory)' == '' or '%(_NoneToInfer.CopyToOutputDirectory)' == 'Never'"> None - + <_InferredPackageFile Include="@(PackageReference)" Condition="'%(PackageReference.Identity)' != 'NuGetizer' and '%(PackageReference.Identity)' != 'NETStandard.Library' and diff --git a/src/NuGetizer.Tests/Builder.NuGetizer.cs b/src/NuGetizer.Tests/Builder.NuGetizer.cs index 9f6f4c25..172388b8 100644 --- a/src/NuGetizer.Tests/Builder.NuGetizer.cs +++ b/src/NuGetizer.Tests/Builder.NuGetizer.cs @@ -28,8 +28,8 @@ public static TargetResult BuildProject( LoggerVerbosity? verbosity = null) { using var sha = new SHA1Managed(); - var hash = Base62.Encode(BitConverter.ToInt64( - sha.ComputeHash(Encoding.UTF8.GetBytes(projectContent)), 0)); + var hash = Base62.Encode(Math.Abs(BitConverter.ToInt64( + sha.ComputeHash(Encoding.UTF8.GetBytes(projectContent)), 0))); var scenarioName = hash; var scenarioDir = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Scenarios", scenarioName); diff --git a/src/NuGetizer.Tests/given_packagereferences.cs b/src/NuGetizer.Tests/given_packagereferences.cs index 0b2cd4ba..3d90ad3c 100644 --- a/src/NuGetizer.Tests/given_packagereferences.cs +++ b/src/NuGetizer.Tests/given_packagereferences.cs @@ -53,5 +53,54 @@ public void when_privateassets_pack_false_then_does_not_pack() PackagePath = @"lib\netstandard2.0\Newtonsoft.Json.dll", })); } + + [Fact] + public void when_build_kind_then_does_not_pack_msbuild() + { + var result = Builder.BuildProject(@" + + + Library + netstandard2.0 + build + + + + +", + "GetPackageContents", output); + + result.AssertSuccess(output); + Assert.DoesNotContain(result.Items, item => item.Matches(new + { + Identity = "Microsoft.Build.Tasks.Core", + Kind = PackageItemKind.Dependency, + })); + } + + [Fact] + public void when_build_kind_and_explicit_pack_then_packs_msbuild() + { + var result = Builder.BuildProject(@" + + + Library + netstandard2.0 + build + + + + +", + "GetPackageContents", output); + + result.AssertSuccess(output); + Assert.Contains(result.Items, item => item.Matches(new + { + Identity = "Microsoft.Build.Tasks.Core", + Kind = PackageItemKind.Dependency, + })); + } + } } From d9f32b81b01a3ee2adec2b4ffa5fbccbd56fbf98 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 29 Sep 2020 20:47:56 -0300 Subject: [PATCH 8/9] Don't add unnecessary empty item groups to contents dump --- src/NuGetizer.Tasks/dotnet-nugetize.targets | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NuGetizer.Tasks/dotnet-nugetize.targets b/src/NuGetizer.Tasks/dotnet-nugetize.targets index b75930cc..9cc79f80 100644 --- a/src/NuGetizer.Tasks/dotnet-nugetize.targets +++ b/src/NuGetizer.Tasks/dotnet-nugetize.targets @@ -23,8 +23,8 @@ Copyright (c) .NET Foundation. All rights reserved. - - + + \ No newline at end of file From 12b3cd7b49c6865624d8d95f9292c3e173363775 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Wed, 30 Sep 2020 01:08:48 -0300 Subject: [PATCH 9/9] Automatically pack transitive assets when PrivateAssets=all When using `parent list. We use that to add the concept of "implicit package references" that basically share the PrivateAssets=all that brought them in. The inference target then just considers both `@(PackageReference)` as well as `@(ImplicitPackageReference)` to determine the primary output dependencies to pack, but otherwise the existing logic remains unchanged. This behavior still honors the `Pack=false` on the PackageReference, but also skips the implificly defined packages like NETStandard.Library and Microsoft.NETCore. Fixes https://github.com/NuGet/Home/issues/5103. --- .../InferImplicitPackageReference.cs | 105 ++++++++++++++++++ .../NuGetizer.Inference.targets | 28 ++++- src/NuGetizer.Tasks/NuGetizer.Tasks.csproj | 7 +- .../given_packagereferences.cs | 76 +++++++++++++ 4 files changed, 204 insertions(+), 12 deletions(-) create mode 100644 src/NuGetizer.Tasks/InferImplicitPackageReference.cs diff --git a/src/NuGetizer.Tasks/InferImplicitPackageReference.cs b/src/NuGetizer.Tasks/InferImplicitPackageReference.cs new file mode 100644 index 00000000..f0f1ce54 --- /dev/null +++ b/src/NuGetizer.Tasks/InferImplicitPackageReference.cs @@ -0,0 +1,105 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; + +namespace NuGetizer.Tasks +{ + public class InferImplicitPackageReference : Task + { + [Required] + public ITaskItem[] PackageReferences { get; set; } = Array.Empty(); + + [Required] + public ITaskItem[] PackageDependencies { get; set; } = Array.Empty(); + + [Output] + public ITaskItem[] ImplicitPackageReferences { get; set; } = Array.Empty(); + + public override bool Execute() + { + var packages = new ConcurrentDictionary>(); + Func parse = value => + { + var parts = value.Split('/'); + return new PackageIdentity(parts[0], parts[1]); + }; + + // Build the list of parent>child relationships. + foreach (var dependency in PackageDependencies.Where(x => x.ItemSpec.Contains('/'))) + { + var identity = parse(dependency.ItemSpec); + var parent = dependency.GetMetadata("ParentPackage"); + if (!string.IsNullOrEmpty(parent)) + { + packages.GetOrAdd(parse(parent), _ => new List()) + .Add(identity); + } + } + + var inferred = new HashSet(); + + foreach (var reference in PackageReferences.Where(x => + "all".Equals(x.GetMetadata("PrivateAssets"), StringComparison.OrdinalIgnoreCase) && + // Unless explicitly set to Pack=false + (!x.TryGetBoolMetadata("Pack", out var pack) || pack != false) && + // NETCore/NETStandard are implicitly defined, we never need to bring them as deps. + !(bool.TryParse(x.GetMetadata("IsImplicitlyDefined"), out var isImplicit) && isImplicit))) + { + var identity = new PackageIdentity(reference.ItemSpec, reference.GetMetadata("Version")); + foreach (var dependency in FindDependencies(identity, packages)) + { + inferred.Add(dependency); + } + } + + ImplicitPackageReferences = inferred + .Select(x => new TaskItem( + x.Id, + new Dictionary + { + { "Version", x.Version } , + { "PrivateAssets", "all" }, + })) + .ToArray(); + + return true; + } + + IEnumerable FindDependencies(PackageIdentity identity, IDictionary> packages) + { + if (packages.TryGetValue(identity, out var dependencies)) + { + foreach (var dependency in dependencies) + { + yield return dependency; + foreach (var child in FindDependencies(dependency, packages)) + { + yield return child; + } + } + } + } + + class PackageIdentity + { + public PackageIdentity(string id, string version) + => (Id, Version) + = (id, version); + + public string Id { get; } + public string Version { get; } + + public override bool Equals(object obj) + => obj is PackageIdentity dependency && + dependency.Id == Id && + dependency.Version == Version; + + public override int GetHashCode() => Tuple.Create(Id, Version).GetHashCode(); + + public override string ToString() => Id + "/" + Version; + } + } +} diff --git a/src/NuGetizer.Tasks/NuGetizer.Inference.targets b/src/NuGetizer.Tasks/NuGetizer.Inference.targets index 3043f848..5e92e52c 100644 --- a/src/NuGetizer.Tasks/NuGetizer.Inference.targets +++ b/src/NuGetizer.Tasks/NuGetizer.Inference.targets @@ -10,6 +10,7 @@ Copyright (c) .NET Foundation. All rights reserved. *********************************************************************************************** --> + @@ -70,7 +71,8 @@ Copyright (c) .NET Foundation. All rights reserved. - + <_InferredPackageFile Include="@(ReferencePath->'%(OriginalItemSpec)')" - Condition="'$(PackFrameworkReferences)' == 'true' and '%(ReferencePath.ResolvedFrom)' == '{TargetFrameworkDirectory}' and '%(ReferencePath.Pack)' != 'false'"> + Condition="'$(PackFrameworkReferences)' == 'true' and + '%(ReferencePath.ResolvedFrom)' == '{TargetFrameworkDirectory}' and + '%(ReferencePath.Pack)' != 'false'"> FrameworkReference @@ -174,21 +178,31 @@ Copyright (c) .NET Foundation. All rights reserved. - + <_PrimaryOutputRelatedFile Include="@(ReferencePath);@(_ReferenceRelatedPaths)" Condition="'%(NuGetPackageId)' != 'NETStandard.Library' and '%(Facade)' != 'true' and '%(FrameworkFile)' != 'true' and '%(Pack)' != 'false'"/> + <_PrivateAssetsPackageReference Include="@(PackageReference -> WithMetadataValue('PrivateAssets', 'all'))" + Condition="'%(PackageReference.IsImplicitlyDefined)' != 'true' and '%(PackageReference.Pack)' != 'false'"/> + + + + + + DependsOnTargets="_ResolvePackageDependencies;_CollectPrimaryOutputDependencies"> + <_NuGetPackageId Include="@(_PrimaryOutputRelatedFile -> '%(NuGetPackageId)')" Condition="'%(NuGetPackageId)' != 'NETStandard.Library'" /> @@ -196,8 +210,9 @@ Copyright (c) .NET Foundation. All rights reserved. <_NuGetPackageId>@(_NuGetPackageId -> Distinct()) - <_PrimaryPackageReference Include="@(PackageReference)" Condition="'$(_NuGetPackageId)' != '' and '%(Identity)' == '$(_NuGetPackageId)'" /> + <_PrimaryPackageReference Include="@(PackageReference);@(ImplicitPackageReference)" Condition="'$(_NuGetPackageId)' != '' and '%(Identity)' == '$(_NuGetPackageId)'" /> + <_PrivateAssets>@(_PrimaryPackageReference -> '%(PrivateAssets)') <_ShouldPack>@(_PrimaryPackageReference -> '%(Pack)') @@ -215,7 +230,8 @@ Copyright (c) .NET Foundation. All rights reserved. - <_InferredPackageFile Include="@(_PrimaryOutputRelatedFile)" Condition="$([System.Text.RegularExpressions.Regex]::IsMatch('%(_PrimaryOutputRelatedFile.FullPath)', '$(_ShouldIncludeAssetsRegex)', 'RegexOptions.IgnoreCase')) == 'true'"> + <_InferredPackageFile Include="@(_PrimaryOutputRelatedFile)" + Condition="$([System.Text.RegularExpressions.Regex]::IsMatch('%(_PrimaryOutputRelatedFile.FullPath)', '$(_ShouldIncludeAssetsRegex)', 'RegexOptions.IgnoreCase')) == 'true'"> $(BuildOutputKind) $(BuildOutputFrameworkSpecific) diff --git a/src/NuGetizer.Tasks/NuGetizer.Tasks.csproj b/src/NuGetizer.Tasks/NuGetizer.Tasks.csproj index 5ef44a62..eba95674 100644 --- a/src/NuGetizer.Tasks/NuGetizer.Tasks.csproj +++ b/src/NuGetizer.Tasks/NuGetizer.Tasks.csproj @@ -10,11 +10,8 @@ - - - @@ -26,9 +23,7 @@ - + diff --git a/src/NuGetizer.Tests/given_packagereferences.cs b/src/NuGetizer.Tests/given_packagereferences.cs index 3d90ad3c..5db39fbc 100644 --- a/src/NuGetizer.Tests/given_packagereferences.cs +++ b/src/NuGetizer.Tests/given_packagereferences.cs @@ -32,6 +32,82 @@ public void when_privateassets_all_then_packs_library() })); } + [Fact] + public void when_privateassets_all_then_packs_transitive_libraries() + { + var result = Builder.BuildProject(@" + + + Library + netstandard2.0 + + + + +", + "GetPackageContents", output); + + result.AssertSuccess(output); + Assert.Contains(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Prism.dll", + })); + Assert.Contains(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Prism.Forms.dll", + })); + Assert.Contains(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Core.dll", + })); + Assert.Contains(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Platform.dll", + })); + Assert.Contains(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Xaml.dll", + })); + } + + [Fact] + public void when_privateassets_all_and_pack_false_then_does_not_pack_transitively() + { + var result = Builder.BuildProject(@" + + + Library + netstandard2.0 + + + + +", + "GetPackageContents", output); + + result.AssertSuccess(output); + Assert.DoesNotContain(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Prism.dll", + })); + Assert.DoesNotContain(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Prism.Forms.dll", + })); + Assert.DoesNotContain(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Core.dll", + })); + Assert.DoesNotContain(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Platform.dll", + })); + Assert.DoesNotContain(result.Items, item => item.Matches(new + { + PackagePath = @"lib\netstandard2.0\Xamarin.Forms.Xaml.dll", + })); + } + [Fact] public void when_privateassets_pack_false_then_does_not_pack() {