Skip to content

Commit 09101a5

Browse files
author
William Li
committed
Fix Publish with GeneratePackageOnBuild=true
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 NuGet/Home#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, #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.
1 parent 527e0e0 commit 09101a5

File tree

5 files changed

+172
-4
lines changed

5 files changed

+172
-4
lines changed

‎src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@ Copyright (c) .NET Foundation. All rights reserved.
2727
<PropertyGroup>
2828
<_ToolsSettingsFilePath>$(BaseIntermediateOutputPath)DotnetToolSettings.xml</_ToolsSettingsFilePath>
2929
<SuppressDependenciesWhenPacking Condition=" '$(PackAsTool)' == 'true' ">true</SuppressDependenciesWhenPacking>
30+
<_PackToolPublishDependency Condition=" ('$(GeneratePackageOnBuild)' != 'true' and '$(NoBuild)' != 'true') and $(IsPublishable) == 'true' ">_PublishBuildAlternative</_PackToolPublishDependency>
31+
<_PackToolPublishDependency Condition=" ('$(GeneratePackageOnBuild)' == 'true' or '$(NoBuild)' == 'true') and $(IsPublishable) == 'true' ">$(_PublishNoBuildAlternativeDependsOn)</_PackToolPublishDependency>
3032
</PropertyGroup>
3133

32-
<Target Name="PackTool" DependsOnTargets="GenerateToolsSettingsFileFromBuildProperty;Publish;_PackToolValidation" Condition=" '$(PackAsTool)' == 'true' ">
34+
<Target Name="PackTool" DependsOnTargets="GenerateToolsSettingsFileFromBuildProperty;$(_PackToolPublishDependency);_PackToolValidation" Condition=" '$(PackAsTool)' == 'true' ">
3335
<ItemGroup>
3436
<_GeneratedFiles Include="$(PublishDepsFilePath)"/>
3537
<_GeneratedFiles Include="$(PublishRuntimeConfigFilePath)"/>

‎src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,17 @@ Copyright (c) .NET Foundation. All rights reserved.
5050
GeneratePublishDependencyFile;
5151
BundlePublishDirectory;
5252
</_CorePublishTargets>
53+
54+
<_PublishNoBuildAlternativeDependsOn>$(_BeforePublishNoBuildTargets);$(_CorePublishTargets)</_PublishNoBuildAlternativeDependsOn>
5355
</PropertyGroup>
5456

5557
<Target Name="_PublishBuildAlternative"
56-
Condition="'$(NoBuild)' != 'true' and '$(GeneratePackageOnBuild)' != 'true'"
58+
Condition="'$(NoBuild)' != 'true'"
5759
DependsOnTargets="Build;$(_CorePublishTargets)" />
5860

5961
<Target Name="_PublishNoBuildAlternative"
60-
Condition="'$(NoBuild)' == 'true' or '$(GeneratePackageOnBuild)' == 'true'"
61-
DependsOnTargets="$(_BeforePublishNoBuildTargets);$(_CorePublishTargets)" />
62+
Condition="'$(NoBuild)' == 'true'"
63+
DependsOnTargets="$(_PublishNoBuildAlternativeDependsOn)" />
6264

6365
<Target Name="Publish"
6466
Condition="$(IsPublishable) == 'true'"

‎src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool.cs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.IO;
7+
using System.Linq;
8+
using System.Runtime.CompilerServices;
9+
using System.Xml.Linq;
10+
using FluentAssertions;
11+
using Microsoft.DotNet.Cli.Utils;
12+
using Microsoft.NET.TestFramework;
13+
using Microsoft.NET.TestFramework.Commands;
14+
using NuGet.Packaging;
15+
using Xunit;
16+
using Xunit.Abstractions;
17+
using Microsoft.NET.TestFramework.Assertions;
18+
19+
namespace Microsoft.NET.ToolPack.Tests
20+
{
21+
public class GivenThatWeWantToBuildWithGeneratePackageOnBuildAndPackAsTool : SdkTest
22+
{
23+
public GivenThatWeWantToBuildWithGeneratePackageOnBuildAndPackAsTool(ITestOutputHelper log) : base(log)
24+
{}
25+
26+
[Theory]
27+
[InlineData(false, false)]
28+
[InlineData(false, true)]
29+
[InlineData(true, false)]
30+
[InlineData(true, true)]
31+
public void It_builds_successfully(bool generatePackageOnBuild, bool packAsTool)
32+
{
33+
TestAsset testAsset = _testAssetsManager
34+
.CopyTestAsset("HelloWorld", identifier: generatePackageOnBuild.ToString() + packAsTool.ToString())
35+
.WithSource()
36+
.WithProjectChanges((projectPath, project) =>
37+
{
38+
XNamespace ns = project.Root.Name.Namespace;
39+
XElement propertyGroup = project.Root.Elements(ns + "PropertyGroup").First();
40+
propertyGroup.Add(new XElement(ns + "GeneratePackageOnBuild", generatePackageOnBuild.ToString()));
41+
propertyGroup.Add(new XElement(ns + "PackAsTool", packAsTool.ToString()));
42+
});
43+
44+
var appProjectDirectory = Path.Combine(testAsset.TestRoot);
45+
var buildCommand = new BuildCommand(Log, appProjectDirectory);
46+
47+
CommandResult result = buildCommand.Execute("/restore");
48+
49+
result.Should()
50+
.Pass();
51+
}
52+
}
53+
}

‎src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool.cs‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.IO;
7+
using System.Linq;
8+
using System.Runtime.CompilerServices;
9+
using System.Xml.Linq;
10+
using FluentAssertions;
11+
using Microsoft.DotNet.Cli.Utils;
12+
using Microsoft.NET.TestFramework;
13+
using Microsoft.NET.TestFramework.Commands;
14+
using NuGet.Packaging;
15+
using Xunit;
16+
using Xunit.Abstractions;
17+
using Microsoft.NET.TestFramework.Assertions;
18+
19+
namespace Microsoft.NET.ToolPack.Tests
20+
{
21+
public class GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool : SdkTest
22+
{
23+
public GivenThatWeWantToPublishWithGeneratePackageOnBuildAndPackAsTool(ITestOutputHelper log) : base(log)
24+
{}
25+
26+
[Theory]
27+
[InlineData(false, false)]
28+
[InlineData(false, true)]
29+
[InlineData(true, false)]
30+
[InlineData(true, true)]
31+
public void It_publishes_successfully(bool generatePackageOnBuild, bool packAsTool)
32+
{
33+
Console.WriteLine(generatePackageOnBuild.ToString() + packAsTool.ToString());
34+
35+
TestAsset testAsset = _testAssetsManager
36+
.CopyTestAsset("HelloWorld", identifier: generatePackageOnBuild.ToString() + packAsTool.ToString())
37+
.WithSource()
38+
.WithProjectChanges((projectPath, project) =>
39+
{
40+
XNamespace ns = project.Root.Name.Namespace;
41+
XElement propertyGroup = project.Root.Elements(ns + "PropertyGroup").First();
42+
propertyGroup.Add(new XElement(ns + "GeneratePackageOnBuild", generatePackageOnBuild.ToString()));
43+
propertyGroup.Add(new XElement(ns + "PackAsTool", packAsTool.ToString()));
44+
});
45+
46+
var appProjectDirectory = Path.Combine(testAsset.TestRoot);
47+
var publishCommand = new PublishCommand(Log, appProjectDirectory);
48+
49+
CommandResult result = publishCommand.Execute("/restore");
50+
51+
result.Should()
52+
.Pass();
53+
}
54+
55+
[Theory]
56+
[InlineData(false, false)]
57+
[InlineData(false, true)]
58+
[InlineData(true, false)]
59+
[InlineData(true, true)]
60+
public void It_builds_successfully(bool generatePackageOnBuild, bool packAsTool)
61+
{
62+
TestAsset testAsset = _testAssetsManager
63+
.CopyTestAsset("HelloWorld", identifier: generatePackageOnBuild.ToString() + packAsTool.ToString())
64+
.WithSource()
65+
.WithProjectChanges((projectPath, project) =>
66+
{
67+
XNamespace ns = project.Root.Name.Namespace;
68+
XElement propertyGroup = project.Root.Elements(ns + "PropertyGroup").First();
69+
propertyGroup.Add(new XElement(ns + "GeneratePackageOnBuild", generatePackageOnBuild.ToString()));
70+
propertyGroup.Add(new XElement(ns + "PackAsTool", packAsTool.ToString()));
71+
});
72+
73+
var appProjectDirectory = Path.Combine(testAsset.TestRoot);
74+
var buildCommand = new BuildCommand(Log, appProjectDirectory);
75+
76+
CommandResult result = buildCommand.Execute("/restore");
77+
78+
result.Should()
79+
.Pass();
80+
}
81+
}
82+
}

‎src/Tests/Microsoft.NET.ToolPack.Tests/GivenThatWeWantToPackAToolProjectWithGeneratePackageOnBuild.cs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,35 @@ public void It_builds_and_result_contains_dependencies_dll()
8686
}
8787
}
8888

89+
[Theory(Skip = "https://github.com/dotnet/sdk/issues/3471")]
90+
[InlineData(false, false)]
91+
[InlineData(false, true)]
92+
[InlineData(true, false)]
93+
[InlineData(true, true)]
94+
public void It_packs_successfully(bool generatePackageOnBuild, bool packAsTool)
95+
{
96+
Console.WriteLine(generatePackageOnBuild.ToString() + packAsTool.ToString());
97+
98+
TestAsset testAsset = _testAssetsManager
99+
.CopyTestAsset("HelloWorld", identifier: generatePackageOnBuild.ToString() + packAsTool.ToString())
100+
.WithSource()
101+
.WithProjectChanges((projectPath, project) =>
102+
{
103+
XNamespace ns = project.Root.Name.Namespace;
104+
XElement propertyGroup = project.Root.Elements(ns + "PropertyGroup").First();
105+
propertyGroup.Add(new XElement(ns + "GeneratePackageOnBuild", generatePackageOnBuild.ToString()));
106+
propertyGroup.Add(new XElement(ns + "PackAsTool", packAsTool.ToString()));
107+
});
108+
109+
var appProjectDirectory = Path.Combine(testAsset.TestRoot);
110+
var packCommand = new PackCommand(Log, appProjectDirectory);
111+
112+
CommandResult result = packCommand.Execute("/restore");
113+
114+
result.Should()
115+
.Pass();
116+
}
117+
89118
private bool IsAppProject(string projectPath)
90119
{
91120
return Path.GetFileNameWithoutExtension(projectPath).Equals(AppName, StringComparison.OrdinalIgnoreCase);

0 commit comments

Comments
 (0)