Skip to content

Allow referencing older version of AppHost package for backward compatibility #5556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 12, 2024
Merged

Allow referencing older version of AppHost package for backward compatibility #5556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df138fd
Allow referencing older version of AppHost package for backward compa…
joperezr Sep 5, 2024
fc8f6dd
Remove unnecessary using statement
joperezr Sep 5, 2024
b9dd125
Rename test
joperezr Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Aspire.Hosting.Sdk/Aspire.Hosting.Sdk.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<None Include="..\Aspire.Hosting\build\*.targets" Link="SDK\%(Filename)%(Extension)" Pack="true" PackagePath="Sdk\%(Filename)%(Extension)" CopyToOutputDirectory="PreserveNewest" />
<None Update="SDK\AutoImport.props;SDK\*.targets" Pack="true" PackagePath="Sdk\%(Filename)%(Extension)" CopyToOutputDirectory="PreserveNewest" />
<None Update="SDK\Sdk.in.props" Pack="true" PerformTextReplacement="True" PackagePath="Sdk\Sdk.props" CopyToOutputDirectory="PreserveNewest" />
<None Update="SDK\Sdk.targets" Pack="true" PackagePath="Sdk\Sdk.targets" />
<None Update="SDK\Sdk.in.targets" Pack="true" PerformTextReplacement="True" PackagePath="Sdk\Sdk.targets" />
</ItemGroup>

</Project>
12 changes: 8 additions & 4 deletions src/Aspire.Hosting.Sdk/SDK/Sdk.targets → src/Aspire.Hosting.Sdk/SDK/Sdk.in.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,17 @@
<_AppHostVersion>%(_AppHostPackageReference.Version)</_AppHostVersion>
</PropertyGroup>

<PropertyGroup Condition="'$(_AppHostVersion)' != ''">
<__CurrentAppHostVersionMessage> You are using version $(_AppHostVersion).</__CurrentAppHostVersionMessage>
<PropertyGroup Condition="'$(_AppHostVersion)' != '' and $([MSBuild]::VersionLessThan('$(_AppHostVersion)', '8.2.0'))">
<!-- If we find the version to Aspire.Hosting.AppHost package but it is lower than 8.2.0, then we fall back
to use the Dashboard and DCP packages that match the version of the installed workload for backwards compatibility.
This results in the same behavior that we had before moving Dashboard and DCP out of the workload, since the version
is again just matching to the one the workload has. -->
<_AppHostVersion>@VERSION@</_AppHostVersion>
</PropertyGroup>

<!-- At this point, we should have the version either by CPM or PackageReference, so we fail if not. -->
<Error Condition="'$(_AppHostVersion)' == '' or $([MSBuild]::VersionLessThan('$(_AppHostVersion)', '8.2.0'))"
Text="$(MSBuildProjectName) is a .NET Aspire AppHost project that needs a package reference to Aspire.Hosting.AppHost version 8.2.0 or above to work correctly.$(__CurrentAppHostVersionMessage)" />
<Error Condition="'$(_AppHostVersion)' == ''"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test for this would be good too. It doesn't have to be in this PR.
Also, the same for both cases but with CPM?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add more tests to this whole scenario with the changes on the Aspire Workload for 9.0

Text="$(MSBuildProjectName) is a .NET Aspire AppHost project that needs a package reference to Aspire.Hosting.AppHost version 8.2.0 or above to work correctly." />

<!-- Now that we have the version, we add the package references -->
<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion tests/Aspire.Hosting.Testing.Tests/Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<!-- NOTE: These lines are only required because we are using P2P references, not NuGet. They will not exist in real apps. -->
<Import Project="..\..\src\Aspire.Hosting.AppHost\build\Aspire.Hosting.AppHost.targets" />
<Import Project="..\..\src\Aspire.Hosting.Sdk\SDK\Sdk.targets" />
<Import Project="..\..\src\Aspire.Hosting.Sdk\SDK\Sdk.in.targets" />

<PropertyGroup>
<!-- This is for in-repo testing and required for Aspire.Hosting.AppHost targets loading correctly. On real projects, this comes from SDK.props in Aspire.Hosting.SDK. -->
Expand Down
2 changes: 1 addition & 1 deletion tests/Aspire.Hosting.Tests/MSBuildTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ the Aspire.Hosting.SDK targets that will automatically add these references to p
File.WriteAllText(Path.Combine(appHostDirectory, "Directory.Build.targets"), $"""
<Project>
<Import Project="{repoRoot}\src\Aspire.Hosting.AppHost\build\Aspire.Hosting.AppHost.targets" />
<Import Project="{repoRoot}\src\Aspire.Hosting.Sdk\SDK\Sdk.targets" />
<Import Project="{repoRoot}\src\Aspire.Hosting.Sdk\SDK\Sdk.in.targets" />
</Project>
""");

Expand Down
42 changes: 42 additions & 0 deletions tests/Aspire.Workload.Tests/AppHostTemplateTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Xunit;
using Xunit.Abstractions;
using System.Text.RegularExpressions;

namespace Aspire.Workload.Tests;

public partial class AppHostTemplateTests : WorkloadTestsBase
{
public AppHostTemplateTests(ITestOutputHelper testOutput)
: base(testOutput)
{
}

[Fact]
public async Task EnsureProjectsReferencing8_1_0AppHostWithNewerWorkloadCanBuild()
{
string projectId = "aspire-can-reference-8.1.0";
await using var project = await AspireProject.CreateNewTemplateProjectAsync(
projectId,
"aspire-apphost",
_testOutput,
BuildEnvironment.ForDefaultFramework,
string.Empty,
false);

var projectPath = Path.Combine(project.RootDir, $"{projectId}.csproj");

// Replace the reference to Aspire.Hosting.AppHost with version 8.1.0
var newContents = AppHostPackageReferenceRegex().Replace(File.ReadAllText(projectPath), @"$1""8.1.0""");

File.WriteAllText(projectPath, newContents);

// Ensure project builds successfully
await project.BuildAsync(workingDirectory: project.RootDir);
}

[GeneratedRegex(@"(PackageReference\s.*""Aspire\.Hosting\.AppHost.*Version=)""[^""]+""")]
private static partial Regex AppHostPackageReferenceRegex();
}
2 changes: 1 addition & 1 deletion tests/Shared/RepoTesting/Aspire.RepoTesting.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@

<ImportGroup Condition="'$(RepoRoot)' != 'null' and '$(TestsRunningOutsideOfRepo)' != 'true' and '$(IsAspireHost)' == 'true'">
<Import Project="$(RepoRoot)src\Aspire.Hosting.AppHost\build\Aspire.Hosting.AppHost.targets" Condition="Exists('$(RepoRoot)src\Aspire.Hosting.AppHost\build\Aspire.Hosting.AppHost.targets')" />
<Import Project="$(RepoRoot)src\Aspire.Hosting.Sdk\SDK\Sdk.targets" Condition="Exists('$(RepoRoot)src\Aspire.Hosting.Sdk\SDK\Sdk.targets')" />
<Import Project="$(RepoRoot)src\Aspire.Hosting.Sdk\SDK\Sdk.in.targets" Condition="Exists('$(RepoRoot)src\Aspire.Hosting.Sdk\SDK\Sdk.in.targets')" />
</ImportGroup>

<PropertyGroup Condition="'$(RepoRoot)' != 'null' and '$(TestsRunningOutsideOfRepo)' != 'true' and '$(IsAspireHost)' == 'true'">
Expand Down
8 changes: 5 additions & 3 deletions tests/Shared/WorkloadTesting/AspireProject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,12 @@ public async Task StartAppHostAsync(string[]? extraArgs = default, Action<Proces
_testOutput.WriteLine($"-- Ready to run tests --");
}

public async Task BuildAsync(string[]? extraBuildArgs = default, CancellationToken token = default)
public async Task BuildAsync(string[]? extraBuildArgs = default, CancellationToken token = default, string? workingDirectory = null)
{
workingDirectory ??= Path.Combine(RootDir, $"{Id}.AppHost");

using var restoreCmd = new DotNetCommand(_testOutput, buildEnv: _buildEnv, label: "restore")
.WithWorkingDirectory(Path.Combine(RootDir, $"{Id}.AppHost"));
.WithWorkingDirectory(workingDirectory);
var res = await restoreCmd.ExecuteAsync($"restore \"-bl:{Path.Combine(LogPath!, $"{Id}-restore.binlog")}\" /p:TreatWarningsAsErrors=true");
res.EnsureSuccessful();

Expand All @@ -274,7 +276,7 @@ public async Task BuildAsync(string[]? extraBuildArgs = default, CancellationTok
buildArgs += " " + string.Join(" ", extraBuildArgs);
}
using var buildCmd = new DotNetCommand(_testOutput, buildEnv: _buildEnv, label: "build")
.WithWorkingDirectory(Path.Combine(RootDir, $"{Id}.AppHost"));
.WithWorkingDirectory(workingDirectory);
res = await buildCmd.ExecuteAsync(buildArgs);
res.EnsureSuccessful();
}
Expand Down