From 5bf3be8c5ca2e5c6eb47d1d49180ae5abb130a5d Mon Sep 17 00:00:00 2001 From: William Li Date: Fri, 26 Jul 2019 08:51:49 -0700 Subject: [PATCH] Fix Publish with GeneratePackageOnBuild=true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What I did is essentially write a different dependency graph in GeneratePackageOnBuild = true to avoid circular dependency . Original bug. I added the https://github.com/dotnet/sdk/pull/3255/files and essentially regressed https://github.com/NuGet/Home/issues/7801. Instead of the logic is in nuget, it is in SDK now. Publish should not skip Build if GeneratePackageOnBuild is true. Or it will mean no build even use Publish verb. But revert 3255 means, https://github.com/dotnet/sdk/issues/2114 will be back. The solution is to break Pack’s dependency on Build in GeneratePackageOnBuild. But it still need Publish. So I need to let Pack depend directly on no build version of publish. > Why cannot directly use “_PublishNoBuildAlternative”? We want to exclude Build step regardless NoBuild flag is true or not. --- .../targets/Microsoft.NET.PackTool.targets | 4 +- .../targets/Microsoft.NET.Publish.targets | 8 +- ...WithGeneratePackageOnBuildAndPackAsTool.cs | 53 ++++++++++++ ...WithGeneratePackageOnBuildAndPackAsTool.cs | 82 +++++++++++++++++++ ...kAToolProjectWithGeneratePackageOnBuild.cs | 29 +++++++ 5 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool.cs create mode 100644 src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool.cs diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets index df28cc69c426..ec25df80b56f 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets @@ -27,9 +27,11 @@ Copyright (c) .NET Foundation. All rights reserved. <_ToolsSettingsFilePath>$(BaseIntermediateOutputPath)DotnetToolSettings.xml true + <_PackToolPublishDependency Condition=" ('$(GeneratePackageOnBuild)' != 'true' and '$(NoBuild)' != 'true') and $(IsPublishable) == 'true' ">_PublishBuildAlternative + <_PackToolPublishDependency Condition=" ('$(GeneratePackageOnBuild)' == 'true' or '$(NoBuild)' == 'true') and $(IsPublishable) == 'true' ">$(_PublishNoBuildAlternativeDependsOn) - + <_GeneratedFiles Include="$(PublishDepsFilePath)"/> <_GeneratedFiles Include="$(PublishRuntimeConfigFilePath)"/> diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets index 443f8d07db57..8329f8f42297 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets @@ -50,15 +50,17 @@ Copyright (c) .NET Foundation. All rights reserved. GeneratePublishDependencyFile; BundlePublishDirectory; + + <_PublishNoBuildAlternativeDependsOn>$(_BeforePublishNoBuildTargets);$(_CorePublishTargets) + Condition="'$(NoBuild)' == 'true'" + DependsOnTargets="$(_PublishNoBuildAlternativeDependsOn)" /> + { + XNamespace ns = project.Root.Name.Namespace; + XElement propertyGroup = project.Root.Elements(ns + "PropertyGroup").First(); + propertyGroup.Add(new XElement(ns + "GeneratePackageOnBuild", generatePackageOnBuild.ToString())); + propertyGroup.Add(new XElement(ns + "PackAsTool", packAsTool.ToString())); + }); + + var appProjectDirectory = Path.Combine(testAsset.TestRoot); + var buildCommand = new BuildCommand(Log, appProjectDirectory); + + CommandResult result = buildCommand.Execute("/restore"); + + result.Should() + .Pass(); + } + } +} diff --git a/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool.cs b/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool.cs new file mode 100644 index 000000000000..19a8d751a45e --- /dev/null +++ b/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool.cs @@ -0,0 +1,82 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Xml.Linq; +using FluentAssertions; +using Microsoft.DotNet.Cli.Utils; +using Microsoft.NET.TestFramework; +using Microsoft.NET.TestFramework.Commands; +using NuGet.Packaging; +using Xunit; +using Xunit.Abstractions; +using Microsoft.NET.TestFramework.Assertions; + +namespace Microsoft.NET.ToolPack.Tests +{ + public class GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool : SdkTest + { + public GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool(ITestOutputHelper log) : base(log) + {} + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void It_publishes_successfully(bool generatePackageOnBuild, bool packAsTool) + { + Console.WriteLine(generatePackageOnBuild.ToString() + packAsTool.ToString()); + + TestAsset testAsset = _testAssetsManager + .CopyTestAsset("HelloWorld", identifier: generatePackageOnBuild.ToString() + packAsTool.ToString()) + .WithSource() + .WithProjectChanges((projectPath, project) => + { + XNamespace ns = project.Root.Name.Namespace; + XElement propertyGroup = project.Root.Elements(ns + "PropertyGroup").First(); + propertyGroup.Add(new XElement(ns + "GeneratePackageOnBuild", generatePackageOnBuild.ToString())); + propertyGroup.Add(new XElement(ns + "PackAsTool", packAsTool.ToString())); + }); + + var appProjectDirectory = Path.Combine(testAsset.TestRoot); + var publishCommand = new PublishCommand(Log, appProjectDirectory); + + CommandResult result = publishCommand.Execute("/restore"); + + result.Should() + .Pass(); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void It_builds_successfully(bool generatePackageOnBuild, bool packAsTool) + { + TestAsset testAsset = _testAssetsManager + .CopyTestAsset("HelloWorld", identifier: generatePackageOnBuild.ToString() + packAsTool.ToString()) + .WithSource() + .WithProjectChanges((projectPath, project) => + { + XNamespace ns = project.Root.Name.Namespace; + XElement propertyGroup = project.Root.Elements(ns + "PropertyGroup").First(); + propertyGroup.Add(new XElement(ns + "GeneratePackageOnBuild", generatePackageOnBuild.ToString())); + propertyGroup.Add(new XElement(ns + "PackAsTool", packAsTool.ToString())); + }); + + var appProjectDirectory = Path.Combine(testAsset.TestRoot); + var buildCommand = new BuildCommand(Log, appProjectDirectory); + + CommandResult result = buildCommand.Execute("/restore"); + + result.Should() + .Pass(); + } + } +} diff --git a/src/Tests/Microsoft.NET.ToolPack.Tests/GivenThatWeWantToPackAToolProjectWithGeneratePackageOnBuild.cs b/src/Tests/Microsoft.NET.ToolPack.Tests/GivenThatWeWantToPackAToolProjectWithGeneratePackageOnBuild.cs index 0a866969230a..efd6ad0a6395 100644 --- a/src/Tests/Microsoft.NET.ToolPack.Tests/GivenThatWeWantToPackAToolProjectWithGeneratePackageOnBuild.cs +++ b/src/Tests/Microsoft.NET.ToolPack.Tests/GivenThatWeWantToPackAToolProjectWithGeneratePackageOnBuild.cs @@ -86,6 +86,35 @@ public void It_builds_and_result_contains_dependencies_dll() } } + [Theory(Skip = "https://github.com/dotnet/sdk/issues/3471")] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void It_packs_successfully(bool generatePackageOnBuild, bool packAsTool) + { + Console.WriteLine(generatePackageOnBuild.ToString() + packAsTool.ToString()); + + TestAsset testAsset = _testAssetsManager + .CopyTestAsset("HelloWorld", identifier: generatePackageOnBuild.ToString() + packAsTool.ToString()) + .WithSource() + .WithProjectChanges((projectPath, project) => + { + XNamespace ns = project.Root.Name.Namespace; + XElement propertyGroup = project.Root.Elements(ns + "PropertyGroup").First(); + propertyGroup.Add(new XElement(ns + "GeneratePackageOnBuild", generatePackageOnBuild.ToString())); + propertyGroup.Add(new XElement(ns + "PackAsTool", packAsTool.ToString())); + }); + + var appProjectDirectory = Path.Combine(testAsset.TestRoot); + var packCommand = new PackCommand(Log, appProjectDirectory); + + CommandResult result = packCommand.Execute("/restore"); + + result.Should() + .Pass(); + } + private bool IsAppProject(string projectPath) { return Path.GetFileNameWithoutExtension(projectPath).Equals(AppName, StringComparison.OrdinalIgnoreCase);