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
[workload] create $(MauiVersion) property #2282
Conversation
5a40377
to
8881431
Compare
666c733
to
887e65e
Compare
<PackageVersion>0.0.1-alpha1</PackageVersion> | ||
<PackageVersion>6.0.100-dev</PackageVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you build dotnet/installer locally, it has this version number.
I thought we could put the same here, so this check would pass:
maui/.nuspec/Microsoft.Maui.Controls.targets
Lines 45 to 47 in 887e65e
<Error Code="MAUI004" | |
Text="At least version '$(_MinimumMauiWorkloadVersion)' of the .NET MAUI workload is required to use <MauiVersion>$(MauiVersion)</MauiVersion>." | |
Condition="'$(MauiWorkloadVersion)' != '' and '$(MauiVersion)' != '' and $([MSBuild]::VersionLessThan($(MauiWorkloadVersion), $(_MinimumMauiWorkloadVersion)))" /> |
NOTE: [MSBuild]::VersionLessThan()
ignores semver, so we'd need to tweak this if it ever matters.
@@ -1,9 +1,11 @@ | |||
<!-- NOTE: Resizetizer.csproj is temporarily replacing usage of this file --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is not currently used in this PR anymore, but I left it here if we want to go back in the future? I think there is still some uncertainty here.
<ItemGroup Condition=" '$(UseMaui)' == 'true' "> | ||
<PackageReference Include="Microsoft.Maui.Controls.Build.Tasks" Version="$(MauiVersion)" IsImplicitlyDefined="true" /> | ||
</ItemGroup> | ||
<ItemGroup Condition=" '$(UseMaui)' == 'true' or '$(UseMauiAssets)' == 'true' "> | ||
<PackageReference Include="Microsoft.Maui.Resizetizer.Sdk" Version="$(MauiVersion)" IsImplicitlyDefined="true" /> | ||
</ItemGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the major change here -- these two packs are library-packs
and an implicit @(PackageReference)
is brought into Maui projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonathanpeppers can $(MauiVersion)
be a floating version? is there a way this could be discoverable from NuGet package manager?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is a way for the NuGet package manager to show this... I guess we could make some package that you add to your project that could do it? Some Microsoft.Maui package could set MauiVersion
in its MSBuild targets inside.
I asked about floating versions here: dotnet/docs#25813 (comment)
I could try and just see if it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Floating numbers could be a reasonable answer. I do like the idea of some other meta package that we publish too that just sets the version, that's a simple enough work around that would surface in the package manager updates UI.
887e65e
to
acc7949
Compare
acc7949
to
954b6a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like all things are right, CI passed. Now for the real test by updating my local machine and pretend I am in the real world!
<Import | ||
Condition=" '$(UseMaui)' == 'true' or '$(UseMauiAssets)' == 'true' " | ||
Project="Sdk.targets" Sdk="Microsoft.Maui.Resizetizer.Sdk" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonathanpeppers I see this was removed so no $(UseMauiAssets)
no longer applies. However, since this is now a dependency of the Microsoft.Maui.Controls.Sdk
, we might also need to add the condition to the line above?
It doesn't seem to pull in anything else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, it should be:
<Import
Condition=" '$(UseMaui)' == 'true' or '$(UseMauiCore)' == 'true' or '$(UseMauiEssentials)' == 'true' or '$(UseMauiAssets)' == 'true' "
Project="Sdk.targets" Sdk="Microsoft.Maui.Controls.Sdk"
/>
One confusing thing is the name of this SDK, it has the name "Controls", but it's imported even when you're not using Controls.
Should we rename this one to Microsoft.Maui.Sdk? I would probably do that in a future PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we can do that in a new PR so we can merge this one now.
Context: https://github.com/jonathanpeppers/maui-workload-pinning We currently have a `$(MicrosoftMauiSdkVersion)` property you can change that allows you to grab newer targeting and runtime packs for .NET MAUI. Unfortunately, there is not a way to do this for MAUI's MSBuild tasks for things like XamlG, XamlC, Resizetizer, etc. Some improvements here: 1. Rename `$(MicrosoftMauiSdkVersion)` to `$(MauiVersion)`. If this is going to be a thing people use, let's choose a shorter name. 2. Move `Microsoft.Maui.Resizetizer.Sdk` into a `library-pack`. Bring it in via an implicit `@(PackageReference)`. 3. Split up parts of `Microsoft.Maui.Controls.Sdk` into a new `library-pack` named `Microsoft.Maui.Controls.Build.Tasks`. Bring it in via an implicit `@(PackageReference)`. Now users will be able to have some minimum version of the .NET MAUI workload installed. They can independently change `$(MauiVersion)` in different projects to use different versions of .NET MAUI. This behavior *may* be temporary, so I left some code in place if we want to revert this behavior and turn these new `library-packs` into regular workload packs. At some point, workload installation may be the preferred (and only way) for installing .NET MAUI.
954b6a2
to
d41e649
Compare
Description of Change
Context: https://github.com/jonathanpeppers/maui-workload-pinning
We currently have a
$(MicrosoftMauiSdkVersion)
property you canchange that allows you to grab newer targeting and runtime packs for
.NET MAUI. Unfortunately, there is not a way to do this for MAUI's
MSBuild tasks for things like XamlG, XamlC, Resizetizer, etc.
Some improvements here:
Rename
$(MicrosoftMauiSdkVersion)
to$(MauiVersion)
. If this isgoing to be a thing people use, let's choose a shorter name.
Move
Microsoft.Maui.Resizetizer.Sdk
into alibrary-pack
. Bringit in via an implicit
@(PackageReference)
.Split up parts of
Microsoft.Maui.Controls.Sdk
into a newlibrary-pack
namedMicrosoft.Maui.Controls.Build.Tasks
. Bringit in via an implicit
@(PackageReference)
.Now users will be able to have some minimum version of the .NET MAUI
workload installed. They can independently change
$(MauiVersion)
indifferent projects to use different versions of .NET MAUI.
This behavior may be temporary, so I left some code in place if we
want to revert this behavior and turn these new
library-packs
intoregular workload packs. At some point, workload installation may be
the preferred (and only way) for installing .NET MAUI.
PR Checklist
Does this PR touch anything that might affect accessibility?
Nope