-
Notifications
You must be signed in to change notification settings - Fork 21
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
Bump .NET version to 7.0 #807
Conversation
…red unsafe block for AddressOf
<Folder Include="SwiftMarshal\" /> | ||
</ItemGroup> | ||
<ItemGroup> | ||
<Reference Include="Xamarin.Mac" /> |
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 doesn't look right, you shouldn't be referencing Xamarin.Mac.dll anymore (you should automatically get a reference to Microsoft.macOS.dll because TargetFramework=net7.0-macos).
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 got managed in a later commit.
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.
Only reviewed the csroj files so far, but I'll come back for the rest later. Also check to make sure your Before* and After* targets are actually running. IIRC, those targets do not run if defined in an SDK style project, you have to configure them differently. See https://learn.microsoft.com/en-us/visualstudio/msbuild/how-to-extend-the-visual-studio-build-process?view=vs-2022#example-aftertargets-and-beforetargets
<RuntimeIdentifiers>ios-arm64</RuntimeIdentifiers> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<Reference Include="SwiftRuntimeLibrary.iOS"> |
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.
Why use a binary reference instead of a project reference?
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.
Likely because this was set up by vsmac years ago and was hit by the migration tool. I wouldn't worry too much about PigLatin - it's likely going to go away/get rebuilt from scratch as a sample.
<ItemGroup> | ||
<Compile Remove="Properties\AssemblyInfo.cs" /> | ||
</ItemGroup> | ||
<ProjectExtensions> |
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 should be converted to a .editorconfig
and removed from here.
<Platforms>x86;AnyCPU</Platforms> | ||
</PropertyGroup> | ||
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|x86' "> | ||
<OutputPath>bin\Debug</OutputPath> |
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 a default and can be removed
<ExternalConsole>true</ExternalConsole> | ||
</PropertyGroup> | ||
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|x86' "> | ||
<OutputPath>bin\Release</OutputPath> |
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 a default and can be removed
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|x86' "> | ||
<OutputPath>bin\Debug</OutputPath> | ||
<DefineConstants>DEBUG;</DefineConstants> | ||
<ExternalConsole>true</ExternalConsole> |
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 duplicated for both configurations, move it to a common PropertyGroup
</ItemGroup> | ||
<Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" /> | ||
<ItemGroup> | ||
<Reference Include="System" /> | ||
</ItemGroup> | ||
<ItemGroup> | ||
<Folder Include="SwiftMarshal\" /> |
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 can probably be removed
</Properties> | ||
</MonoDevelop> | ||
</ProjectExtensions> | ||
<Project Sdk="Microsoft.NET.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.
You may want to consider migrating this to match the MAUI project templates over the migrated Xamarin.Forms template:
`
<PropertyGroup>
<TargetFrameworks>net9.0-android;net9.0-ios;net9.0-maccatalyst</TargetFrameworks>
<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('windows'))">$(TargetFrameworks);net9.0-windows10.0.19041.0</TargetFrameworks>
<!-- Uncomment to also build the tizen app. You will need to install tizen by following this: https://github.com/Samsung/Tizen.NET -->
<!-- <TargetFrameworks>$(TargetFrameworks);net6.0-tizen</TargetFrameworks> -->
<OutputType>Exe</OutputType>
<RootNamespace>MauiTest</RootNamespace>
<UseMaui>true</UseMaui>
<SingleProject>true</SingleProject>
<ImplicitUsings>enable</ImplicitUsings>
<!-- Display name -->
<ApplicationTitle>MauiTest</ApplicationTitle>
<!-- App Identifier -->
<ApplicationId>com.companyname.mauitest</ApplicationId>
<ApplicationIdGuid>c8937955-7050-4e07-9013-1bda386aa558</ApplicationIdGuid>
<!-- Versions -->
<ApplicationDisplayVersion>1.0</ApplicationDisplayVersion>
<ApplicationVersion>1</ApplicationVersion>
<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'ios'">14.2</SupportedOSPlatformVersion>
<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'maccatalyst'">14.0</SupportedOSPlatformVersion>
<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'android'">21.0</SupportedOSPlatformVersion>
<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'windows'">10.0.17763.0</SupportedOSPlatformVersion>
<TargetPlatformMinVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'windows'">10.0.17763.0</TargetPlatformMinVersion>
<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'tizen'">6.5</SupportedOSPlatformVersion>
</PropertyGroup>
<PropertyGroup>
<MtouchLink>SdkOnly</MtouchLink>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Maui.Controls" Version="9.0.0-ci.net9.9888" />
</ItemGroup>
<ItemGroup>
<!-- App Icon -->
<MauiIcon Include="Resources\AppIcon\appicon.svg" ForegroundFile="Resources\AppIcon\appiconfg.svg" Color="#512BD4" />
<!-- Splash Screen -->
<MauiSplashScreen Include="Resources\Splash\splash.svg" Color="#512BD4" BaseSize="128,128" />
<!-- Images -->
<MauiImage Include="Resources\Images\*" />
<MauiImage Update="Resources\Images\dotnet_bot.svg" BaseSize="168,208" />
<!-- Custom Fonts -->
<MauiFont Include="Resources\Fonts\*" />
<!-- Raw Assets (also remove the "Resources\Raw" prefix) -->
<MauiAsset Include="Resources\Raw\**" LogicalName="%(RecursiveDir)%(Filename)%(Extension)" />
</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.
I expect the samples to be redone entirely.
<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" /> | ||
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" /> | ||
</ItemGroup> | ||
<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.
Not sure what this next ItemGroup accomplishes. Why are all these files being removed from the project? If it is correct, please convert to globbing patterns to reduce the noise.
<PropertyGroup> | ||
<OutputType>Exe</OutputType> | ||
<TargetFramework>net7.0</TargetFramework> | ||
<DefaultItemExcludes>$(DefaultItemExcludes);*.sh;*.md;*.props</DefaultItemExcludes> |
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.
💯
type-o-matic/type-o-matic.csproj
Outdated
</ItemGroup> | ||
<ItemGroup> | ||
<Compile Remove="TypeMetadataSet.cs" /> |
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.
Why is this file being removed? Add a comment, or if it is no longer used entirely, delete the file.
Yes, this is a large PR. I tried to make each commit its own story, so in reviewing this you should probably go through each commit rather than trying to look at the whole.
This PR moves Binding Tools for Swift from Mono to .NET 7.0
The changes fall into a few categories:
dotnet
to build thingsdotnet
to compile C# code[UnmanagedCallersOnly]
[UnmanagedCallersOnly]
delegate *unmanaged<...>
syntaxIn its current state 1416 tests pass and 175 fail. Those that fail fall into the following categories:
[UnmanagedCallersOnly]
can't appear in generic typesNativeHandle
forClassHandle
In addition iPhone/iPad tests don't happen (yet). At this point, I want to get this PR in so I focus on getting things running so I don't hold up @kotlarmilos