Skip to content

Commit d3d494e

Browse files
baronfelbaronfel
authored
Fix tool projects so they don't remove apphosts when publishing (not packing) (#49818)
1 parent e9fb1d8 commit d3d494e

File tree

6 files changed

+76
-24
lines changed

6 files changed

+76
-24
lines changed

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

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,6 @@ NOTE: This file is imported from the following contexts, so be aware when writin
7474
<_InnerToolsPublishAot Condition="$(_HasRIDSpecificTools) and '$(PublishAot)' == 'true'">true</_InnerToolsPublishAot>
7575
<PublishAot Condition="!$(_IsRidSpecific) and '$(MSBuildIsRestoring)' != 'true'">false</PublishAot>
7676

77-
<!-- we want to ensure that we don't publish any AppHosts for the 'outer', RID-agnostic builds -->
78-
<UseAppHost Condition="!$(_IsRidSpecific)">false</UseAppHost>
79-
<!-- we want to ensure that we _do_ publish any AppHosts for the 'inner', RID-specific builds -->
80-
<UseAppHost Condition="$(_IsRidSpecific)">true</UseAppHost>
81-
8277
<!-- Tool implementation files are not included in the primary package when the tool has RID-specific packages. So only pack the tool implementation
8378
(and only depend on publish) if there are no RID-specific packages, or if the RuntimeIdentifier is set. -->
8479
<_ToolPackageShouldIncludeImplementation Condition=" '$(PackAsTool)' == 'true' And
@@ -132,14 +127,27 @@ NOTE: This file is imported from the following contexts, so be aware when writin
132127
<Target Name="SetPackToolProperties"
133128
BeforeTargets="_GenerateRestoreProjectSpec;_GenerateToolsSettingsFileInputCache;_GenerateShimInputCache;_GetOutputItemsFromPack">
134129

130+
<PropertyGroup>
131+
<!-- Fun fact - UseAppHost is false for NativeAot because _RuntimeIdentifierUsesAppHost gets set to false.
132+
So we need to check if PublishAot is set as part of this too. As far as I can tell there is no 'common'
133+
property for 'you should expect a native binary here' -->
134+
<_ToolUsesPlatformSpecificExecutable Condition="$(_IsRidSpecific) and ('$(UseAppHost)' == 'true' or '$(PublishAot)' == 'true')">true</_ToolUsesPlatformSpecificExecutable>
135+
<_ToolUsesPlatformSpecificExecutable Condition="'$(_ToolUsesPlatformSpecificExecutable)' == ''">false</_ToolUsesPlatformSpecificExecutable>
136+
</PropertyGroup>
137+
138+
<PropertyGroup Condition="'$(ToolCommandRunner)' == ''">
139+
<ToolCommandRunner Condition="!$(_ToolUsesPlatformSpecificExecutable)">dotnet</ToolCommandRunner>
140+
<ToolCommandRunner Condition="$(_ToolUsesPlatformSpecificExecutable)">executable</ToolCommandRunner>
141+
</PropertyGroup>
142+
135143
<!-- Needs to be in a target so we don't need to worry about evaluation order with NativeBinary property -->
136144
<PropertyGroup Condition="'$(ToolEntryPoint)' == ''">
137-
<ToolEntryPoint>$(TargetFileName)</ToolEntryPoint>
138-
<ToolEntryPoint Condition="$(_IsRidSpecific) and '$(UseAppHost)' == 'true' ">$(AssemblyName)$(_NativeExecutableExtension)</ToolEntryPoint>
145+
<ToolEntryPoint Condition="!$(_ToolUsesPlatformSpecificExecutable)">$(TargetFileName)</ToolEntryPoint>
146+
<ToolEntryPoint Condition="$(_ToolUsesPlatformSpecificExecutable)">$(AssemblyName)$(_NativeExecutableExtension)</ToolEntryPoint>
139147
</PropertyGroup>
140148

141149
<!-- inner-build tool packages get a RID suffix -->
142-
<PropertyGroup Condition="'$(_HasRIDSpecificTools)' != '' And $(RuntimeIdentifier) != ''">
150+
<PropertyGroup Condition="'$(_HasRIDSpecificTools)' != '' And '$(RuntimeIdentifier)' != ''">
143151
<PackageId>$(PackageId).$(RuntimeIdentifier)</PackageId>
144152
</PropertyGroup>
145153
</Target>
@@ -157,7 +165,7 @@ NOTE: This file is imported from the following contexts, so be aware when writin
157165
<Target Name="PackToPublishDependencyIndirection"
158166
DependsOnTargets="$(_PackToolPublishDependency)"/>
159167

160-
<Target Name="PackTool" DependsOnTargets="SetPackToolProperties;GenerateToolsSettingsFileFromBuildProperty;PackToPublishDependencyIndirection;_PackToolValidation;PackToolImplementation"
168+
<Target Name="PackTool" DependsOnTargets="SetPackToolProperties;GenerateToolsSettingsFileFromBuildProperty;PackToPublishDependencyIndirection;_PackToolValidation;PackToolImplementation"
161169
Condition=" '$(PackAsTool)' == 'true' "
162170
Returns="@(TfmSpecificPackageFile)">
163171
<ItemGroup>
@@ -166,8 +174,15 @@ NOTE: This file is imported from the following contexts, so be aware when writin
166174
recreate the publish layout, but under a TFM- or RID-specific root path. Because we're globbing from the PublishDir,
167175
the MSBuild Items will have the RecursiveDir metadata - this is used by the PackLogic to combine with the PackagePath
168176
we set below to ensure we have the correct layout in the end. If the PackLogic didn't try to use RecursiveDir, then
169-
we could just explicitly set everything ourselves. -->
177+
we could just explicitly set everything ourselves.
178+
179+
We _also_ have to filter out apphosts here from platform-agnostic builds, because we don't want to include them in the tool package.
180+
This is because we try not to influence the selection of UseAppHost (so that we do not impact 'normal' publishing and end up
181+
influencing the contents of the 'normal' publish directory), so it's possible that the 'publish' of an otherwise RID-agnostic
182+
tool may include an apphost.
183+
-->
170184
<_PublishFiles Condition="'$(_ToolPackageShouldIncludeImplementation)' == 'true'" Include="$(PublishDir)/**/*" />
185+
<_PublishFiles Condition="'$(UseAppHost)' == 'true' and '$(RuntimeIdentifier)' == ''" Remove="$(PublishDir)$(AssemblyName)$(_NativeExecutableExtension)"/>
171186
</ItemGroup>
172187

173188
<PropertyGroup>
@@ -195,11 +210,6 @@ NOTE: This file is imported from the following contexts, so be aware when writin
195210
<_GenerateToolsSettingsFileCacheFile>$([MSBuild]::NormalizePath($(MSBuildProjectDirectory), $(_GenerateToolsSettingsFileCacheFile)))</_GenerateToolsSettingsFileCacheFile>
196211
</PropertyGroup>
197212

198-
<PropertyGroup Condition="'$(ToolCommandRunner)' == ''">
199-
<ToolCommandRunner Condition="!$(UseAppHost)">dotnet</ToolCommandRunner>
200-
<ToolCommandRunner Condition="$(UseAppHost)">executable</ToolCommandRunner>
201-
</PropertyGroup>
202-
203213
<Target Name="_GenerateToolsSettingsFileInputCache">
204214
<ItemGroup>
205215
<_ToolPackageRuntimeIdentifier Include="$(_UserSpecifiedToolPackageRids)" />

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ Copyright (c) .NET Foundation. All rights reserved.
184184
$([MSBuild]::VersionLessThan($(_TargetFrameworkVersionWithoutV), '8.0'))">true</SelfContained>
185185

186186
<SelfContained Condition="'$(SelfContained)' == ''">false</SelfContained>
187-
<_RuntimeIdentifierUsesAppHost Condition="$(RuntimeIdentifier.StartsWith('ios')) or $(RuntimeIdentifier.StartsWith('tvos')) or $(RuntimeIdentifier.StartsWith('maccatalyst')) or $(RuntimeIdentifier.StartsWith('android')) or $(RuntimeIdentifier.StartsWith('browser')) or $(RuntimeIdentifier.StartsWith('wasi'))">false</_RuntimeIdentifierUsesAppHost>
187+
<_RuntimeIdentifierUsesAppHost Condition="$(RuntimeIdentifier.StartsWith('ios')) or $(RuntimeIdentifier.StartsWith('tvos')) or $(RuntimeIdentifier.StartsWith('maccatalyst')) or $(RuntimeIdentifier.StartsWith('android')) or $(RuntimeIdentifier.StartsWith('browser')) or $(RuntimeIdentifier.StartsWith('wasi')) or $(RuntimeIdentifier) == 'any'">false</_RuntimeIdentifierUsesAppHost>
188188
<_RuntimeIdentifierUsesAppHost Condition="'$(_IsPublishing)' == 'true' and '$(PublishAot)' == 'true'">false</_RuntimeIdentifierUsesAppHost>
189189
<_RuntimeIdentifierUsesAppHost Condition="'$(_RuntimeIdentifierUsesAppHost)' == ''">true</_RuntimeIdentifierUsesAppHost>
190190
<UseAppHost Condition="'$(UseAppHost)' == '' and

‎test/Microsoft.DotNet.PackageInstall.Tests/EndToEndToolTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public EndToEndToolTests(ITestOutputHelper log, TestToolBuilder toolBuilder) : b
2222
public void InstallAndRunToolGlobal()
2323
{
2424
var toolSettings = new TestToolBuilder.TestToolSettings();
25-
string toolPackagesPath = ToolBuilder.CreateTestTool(Log, toolSettings);
25+
string toolPackagesPath = ToolBuilder.CreateTestTool(Log, toolSettings, collectBinlogs: true);
2626

2727
var testDirectory = _testAssetsManager.CreateTestDirectory();
2828
var homeFolder = Path.Combine(testDirectory.Path, "home");

‎test/Microsoft.DotNet.PackageInstall.Tests/TestToolBuilder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public string GetIdentifier() {
8282
}
8383

8484

85-
public string CreateTestTool(ITestOutputHelper log, TestToolSettings toolSettings, bool collectBinlogs = false)
85+
public string CreateTestTool(ITestOutputHelper log, TestToolSettings toolSettings, bool collectBinlogs = true)
8686
{
8787
var targetDirectory = Path.Combine(TestContext.Current.TestExecutionDirectory, "TestTools", toolSettings.GetIdentifier());
8888

‎test/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAToolProject.cs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
#nullable disable
5+
6+
using System.Runtime.CompilerServices;
7+
using Microsoft.DotNet.Cli.Utils;
8+
9+
namespace Microsoft.NET.Publish.Tests
10+
{
11+
public class GivenThatWeWantToPublishAToolProject : SdkTest
12+
{
13+
public GivenThatWeWantToPublishAToolProject(ITestOutputHelper log) : base(log)
14+
{
15+
}
16+
17+
private TestAsset SetupTestAsset([CallerMemberName] string callingMethod = "")
18+
{
19+
TestAsset helloWorldAsset = _testAssetsManager
20+
.CopyTestAsset("PortableTool", callingMethod)
21+
.WithSource();
22+
23+
24+
return helloWorldAsset;
25+
}
26+
27+
[Fact]
28+
// this test verifies that we don't regress the 'normal' publish experience accidentally in the
29+
// PackTool.targets
30+
public void It_can_publish_and_has_apphost()
31+
{
32+
var testAsset = SetupTestAsset();
33+
var publishCommand = new PublishCommand(testAsset);
34+
35+
publishCommand.Execute();
36+
37+
publishCommand.GetOutputDirectory(targetFramework: ToolsetInfo.CurrentTargetFramework)
38+
.Should().HaveFile("consoledemo" + Constants.ExeSuffix);
39+
}
40+
}
41+
}

‎test/Microsoft.NET.ToolPack.Tests/GivenThatWeWantToPackAToolProject.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#nullable disable
55

66
using System.Runtime.CompilerServices;
7+
using Microsoft.DotNet.Cli.Utils;
78
using NuGet.Packaging;
89
using NuGet.Packaging.Core;
910

@@ -20,9 +21,9 @@ public GivenThatWeWantToPackAToolProject(ITestOutputHelper log) : base(log)
2021

2122
private string SetupNuGetPackage(bool multiTarget, string packageType = null, [CallerMemberName] string callingMethod = "")
2223
{
23-
24+
string id = $"{callingMethod}-{_targetFrameworkOrFrameworks}";
2425
TestAsset helloWorldAsset = _testAssetsManager
25-
.CopyTestAsset("PortableTool", callingMethod + multiTarget + (packageType ?? ""))
26+
.CopyTestAsset("PortableTool", id)
2627
.WithSource()
2728
.WithProjectChanges(project =>
2829
{
@@ -39,7 +40,7 @@ private string SetupNuGetPackage(bool multiTarget, string packageType = null, [C
3940

4041
var packCommand = new PackCommand(helloWorldAsset);
4142

42-
var result = packCommand.Execute();
43+
var result = packCommand.Execute($"/bl:{id}-{{}}.binlog");
4344
result.Should().Pass();
4445

4546
return packCommand.GetNuGetPackage();
@@ -192,9 +193,9 @@ public void It_does_not_contain_apphost_exe(bool multiTarget)
192193
string[] args = multiTarget ? new[] { $"/p:TargetFramework={_targetFrameworkOrFrameworks}" } : Array.Empty<string>();
193194
getValuesCommand.Execute(args)
194195
.Should().Pass();
195-
string runCommandPath = getValuesCommand.GetValues().Single();
196-
runCommandPath
197-
.Should().Be("dotnet", because: "The RunCommand should recognize that this is a non-AppHost tool and should use the muxer to launch it");
196+
var runCommand = new FileInfo(getValuesCommand.GetValues().Single());
197+
runCommand.Name
198+
.Should().Be("consoledemo" + Constants.ExeSuffix, because: "The RunCommand should recognize that this is an AppHost-using project and should use the AppHost for non-tool use cases.");
198199
}
199200
}
200201

0 commit comments

Comments
 (0)