Skip to content
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.

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

Add templates for 9.0 #6058

Merged
merged 25 commits into from
Oct 3, 2024
Merged

Conversation

radical
Copy link
Member

@radical radical commented Oct 1, 2024

Template changes

  • Rename Aspire.ProjectTemplates to Aspire.ProjectTemplates.9.0.net8
  • Add Aspire.ProjectTemplates.9.0.net8
    • duplicated from Aspire.ProjectTemplates
    • And the -f net8.0 option is replaced with -f net9.0 in the various template.json files.

Testing changes

  • 3 separate sdk installations are created for testing in artifacts:

    1. artifacts/bin/dotnet-tests - 9.0 sdk + 8.0 runtime (used by default)
    2. artifacts/bin/dotnet-9 - 9.0 sdk only
    3. artifacts/bin/dotnet-8 - 8.0 sdk only
  • 3 separate template installations (called custom hives) are created:

    1. templates nugets for net9+net8 installed
    2. templates nuget for net9 only installed
    3. templates nuget for net8 only installed
  • The existing workload tests are run against dotnet-tests. This is the same as previous behavior. These tests use the net8+net9 template set by default.

  • New tests are added that dotnet new + dotnet build the templates with the 3 sdks, and the 3 template installations.

  • The tests also allow running the workload tests against net8.0 by default, which can be enabled for internal pipeline to get extra testing for net8.0.

Fixes #5926
Fixes #5447

Microsoft Reviewers: Open in CodeFlow

@radical radical mentioned this pull request Oct 1, 2024
16 tasks
@radical
Copy link
Member Author

radical commented Oct 1, 2024

I'm still doing some cleanup work on the tests, but in general this can be review can be started, especially the templates part. The first two commits have the template changes.

cc @joperezr @eerhardt @DamianEdwards

@radical
Copy link
Member Author

radical commented Oct 2, 2024

Follow up work:

  • more workload -> template renaming in tests, as necessary
  • Add tests for aspire-servicedefaults, and aspire-apphost

@radical radical marked this pull request as ready for review October 2, 2024 07:36
@radical radical requested a review from eerhardt as a code owner October 2, 2024 07:36
"thirdPartyNotices": "https://aka.ms/dotnet/aspire/8.0-third-party-notices",
"groupIdentity": "Aspire.AppHost",
"groupIdentity": "Aspire.AppHost.9",
Copy link
Member

@DamianEdwards DamianEdwards Oct 2, 2024

Choose a reason for hiding this comment

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

If the package ID contains 9.0 I think we'd want the groupIdentity to be Aspire.AppHost.9.0 here (and the identity above Aspire.AppHost.CSharp.9.0.net8)? That means different 9.x versions can be installed side-by-side (the package IDs already have the major.minor AFAICT) and the template engine will group the templates by their groupIdentity for TFM selection.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that depends on whether we want major.minor to be SxS. FWIW, we could leave this .9 today, and if we release a 9.1 which we want to be SxS, make that groupid include the minor version.

Copy link
Member

Choose a reason for hiding this comment

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

fair point, can always defer that decision

"unsupportedHosts": [
{
"id": "vs",
"version": "(,17.9)"
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be bumped higher? Are we doing any testing of Aspire 9 with VS 17.10 or 17.11?

},
"precedence": "9000",
"identity": "Aspire.Empty.CSharp.9.net9",
"thirdPartyNotices": "https://aka.ms/dotnet/aspire/8.0-third-party-notices",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"thirdPartyNotices": "https://aka.ms/dotnet/aspire/8.0-third-party-notices",
"thirdPartyNotices": "https://aka.ms/dotnet/aspire/9.0-third-party-notices",

Copy link
Member

Choose a reason for hiding this comment

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

Actually, given this just links to the file in the repo, let's just make it:

Suggested change
"thirdPartyNotices": "https://aka.ms/dotnet/aspire/8.0-third-party-notices",
"thirdPartyNotices": "https://aka.ms/dotnet/aspire/third-party-notices",

Copy link
Member Author

Choose a reason for hiding this comment

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

Wouldn't this become a problem when we get to 10.0, and the default link would point to the 10.0 branch?

@radical radical mentioned this pull request Oct 2, 2024
8 tasks
…emplate.config/template.json

Co-authored-by: Damian Edwards <damian@damianedwards.com>
Copy link
Member

@DamianEdwards DamianEdwards left a comment

Choose a reason for hiding this comment

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

Looks good!

@davidfowl
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DamianEdwards DamianEdwards enabled auto-merge (squash) October 3, 2024 14:25
@DamianEdwards DamianEdwards merged commit 9086843 into dotnet:main Oct 3, 2024
9 checks passed
@radical radical deleted the templates-9-with-tests branch October 3, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants