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

[Housekeeping] Add MSBuild Task to Verify .NET MAUI Workload Installed on Local Machine #2266

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

brminnick
Copy link
Collaborator

Description of Change

This PR creates a new project, CommunityToolkit.Maui.BuildTasks, where we can create MSBuild Tasks to run verification steps after developers have installed our NuGet Package.

In CommunityToolkit.Maui.BuildTasks, I've created CommunityToolkit.Maui.BuildTasks that verifies the user has the minimum required version of .NET MAUI installed on their local machine.

To Do

  • Add MSBuild Task to NuGet Package

PR Checklist

Additional information

This PR is a work-around until Microsoft improves the .NET Workload tooling. Currently, there is no way for us to require a minimum .NET Workload version before a user installs our app.

Each time we bump our minimum required version of .NET, we subsequently bump our minimum required version of .NET MAUI. However, there are no tools in the chain to notify the user they must update their .NET MAUI Workload, leading to users opening many Issues on our repo where we repeat our recommendations for them to update the .NET SDK and update the .NET MAUI workload.

This PR should help us avoid users opening these "Issues".

@brminnick brminnick added the needs discussion Discuss it on the next Monthly standup label Oct 8, 2024
Copy link
Member

@pictos pictos left a comment

Choose a reason for hiding this comment

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

I really like this idea and can see value on it. I added some comments around the current implementation and I want to add.

I tried on my end and inspect the nuget, and right now the target isn't packaged correctly, I did some changes locally and could make it pack as expected for the BuildTasks.csproj but couldn't if we want this targets to be built with Core project.

I suggest that we ship it as a separated nuget package and add a reference to that package in our core project.

After appling that on my end I could see it during the build phase
image

Directory.Build.props Outdated Show resolved Hide resolved
@@ -44,6 +44,12 @@
<Configurations>Debug;Release</Configurations>
</PropertyGroup>

<UsingTask TaskName="VerifyMauiWorkloadVersionTask" AssemblyFile="..\CommunityToolkit.Maui.BuildTasks\bin\$(Configuration)\$(NetVersion)\CommunityToolkit.Maui.BuildTasks.dll" />
Copy link
Member

Choose a reason for hiding this comment

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

I believe this will not work the way you expect...
If I understood correctly you want to run the MSBuild task when the users build the project. The way it's coded here, this will run during the build of Core project and not on the MauiApp project.

If you want it to run on the way that I mentioned, in the app project, this should be moved to a target file inside the BuildTasks folder and pack from there

<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(NetVersion)</TargetFramework>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if you want to use the net8 here... It's recommended to use netstandard for MsBuild tasks, because the project may run on IDEs and/or tools that doesn't support net8. For example, Visual Studio runs on top of .net framework, and using net 8 may cause errors

brminnick and others added 2 commits October 7, 2024 21:45
Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>
Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>
<UsingTask TaskName="VerifyMauiWorkloadVersionTask" AssemblyFile="..\CommunityToolkit.Maui.BuildTasks\bin\$(Configuration)\$(NetVersion)\CommunityToolkit.Maui.BuildTasks.dll" />

<Target Name="VerifyMauiWorkloadVersionTask" BeforeTargets="Build">
<VerifyMauiWorkloadVersionTask MinimumRequiredMauiVersion="$(RequiredMauiWorkloadVersion)" />
Copy link
Contributor

@filipnavara filipnavara Oct 8, 2024

Choose a reason for hiding this comment

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

The property defined above is named _RequiredMauiWorkloadVersion, with underscore.

@pictos
Copy link
Member

pictos commented Oct 8, 2024

@filipnavara by any chance, do you know how we can add the BuildTask project as a dependency but without executing it? If on Core's csproj I do:

<PackageReference Include="CommunityToolkit.Maui.BuildTasks" Version="1.0.5" /> or even reference the project itself, the Target will run during dotnet pack, but the Target should run only during the build from projects that consumes the MCT

@filipnavara
Copy link
Contributor

While I appreciate the intention there seems to be a misunderstanding about how the whole SDK and workload versioning works.

It's an officially supported scenario to use .NET SDK version X to build apps targeting .NET <= X. The workloads are bound to the SDK that is used for the build, not the TFM that the application targets. .NET 9 workloads can build both net8.0-iosX.Y and net9.0-iosX.Y TFMs and that's very much a supported scenario.

dotnet workload list lists the workloads that are bound the the build SDK (which can be forced through global.json), not to the TFM. Notably, you will never see the version 8.0.402.1 when building with .NET 9 SDK, and even if you got some version and compared it as a version number the check is going to pass despite the original issue still happening (net8.0 taken from NuGet while net8.0-ios18.0 is not understood by the installed .NET 9 workload).

If you really want a reliable check you should be shipping MSBuild script inside the NuGet that verifies on the actual build that OS-specific assets were picked by NuGet...

@filipnavara
Copy link
Contributor

filipnavara commented Oct 8, 2024

Furthermore, there are two distinct workload update modes, configurable with dotnet workload config --update-mode. The manifests mode was the default and only supported mode up to 8.0.3xx SDKs. The workload-set mode was added in 8.0.4xx SDK band. You seem to be relying on the output with the workload-set mode. While that's the new default, it's not something you should be forcing on the users. There are many legitimate reasons not to use that mode, including existing CI build scenarios with fixed rollback manifests and remote iOS builds from Windows machines.

For example, this is the output I get with 8.0.402 SDK with the manifests mode with the iOS 18 workloads:

Installed Workload Id      Manifest Version       Installation Source
---------------------------------------------------------------------
macos                      15.0.8303/8.0.100      SDK 8.0.400        
maui-ios                   8.0.82/8.0.100         SDK 8.0.400        
ios                        18.0.8303/8.0.100      SDK 8.0.400        
android                    34.0.138/8.0.100       SDK 8.0.400        

Use `dotnet workload search` to find additional workloads to install.

Updates are available for the following workload(s): macos maui-ios ios android. Run `dotnet workload update` to get the latest.

This is .NET 9 RC1 SDK with the workload-set mode (and no support for -ios18.0 TFMs!):

Workload version: 9.0.100-rc.1.24452.12

Installed Workload Id      Installation Source
----------------------------------------------
ios                        SDK 9.0.100-rc.1   

Use `dotnet workload search` to find additional workloads to install.

Updates are available for the following workload(s): ios. Run `dotnet workload update` to get the latest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Discuss it on the next Monthly standup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants