-
Notifications
You must be signed in to change notification settings - Fork 53
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
[build] Target net7.0
.
#988
Changes from 4 commits
1fa26b3
145ab21
2a2a60c
0d93608
03beb35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFrameworks>net472;net6.0</TargetFrameworks> | ||
<TargetFrameworks>net472;$(DotNetStableTargetFramework)</TargetFrameworks> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made a thing like We can also wait and update these later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I figured There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with whatever is easiest for now, but we should endeavor to get everything we ship running on .NET 7 previews since that is how our customers will eventually receive it. We need to find any issues caused by .NET 7 before they do. 😁 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't look too closely, but I think the AndroidDotnetToolTask that runs tools like generator and class parse only uses the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there must be some logic when you There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not quite sure, previous XA PR builds testing these changes had issues running class-parse and other tools because the XA test environments no longer have NET 7 installed system wide. That blob also mentions an old unresolved issue that seems related. I put up dotnet/android#7038 for now, we can discuss this further over there. I'm going to revert the |
||
<OutputType>Exe</OutputType> | ||
</PropertyGroup> | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFrameworks>net472;net6.0</TargetFrameworks> | ||
<TargetFrameworks>net472;$(DotNetStableTargetFramework)</TargetFrameworks> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same with this one, should it build for 7? |
||
<OutputType>Exe</OutputType> | ||
<DefineConstants>$(DefineConstants);GENERATOR;HAVE_CECIL;JCW_ONLY_TYPE_NAMES</DefineConstants> | ||
<LangVersion>8.0</LangVersion> | ||
|
@@ -43,7 +43,7 @@ | |
|
||
<ItemGroup> | ||
<None Condition=" '$(TargetFramework)' == 'net472' " Include="$(PkgMono_Options)\lib\net40\Mono.Options.pdb" CopyToOutputDirectory="PreserveNewest" /> | ||
<None Condition=" '$(TargetFramework)' == 'net6.0' " Include="$(PkgMono_Options)\lib\netstandard2.0\Mono.Options.pdb" CopyToOutputDirectory="PreserveNewest" /> | ||
<None Condition=" '$(TargetFramework)' != 'net472' " Include="$(PkgMono_Options)\lib\netstandard2.0\Mono.Options.pdb" CopyToOutputDirectory="PreserveNewest" /> | ||
</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.
This "confuses" me, and I'm not sure what's going on.
$(DotNetTargetFramework)
isnet7.0
, thus I would expectbin/Debug-net7.0/Xamarin.Android.Tools.Bytecode.dll
to be a file that exists after a build.However, it doesn't exist; instead, I have
bin/Debug-net6.0/Xamarin.Android.Tools.Bytecode.dll
, which is a copy of the .NET Standard 2.0 build, which is weird.Meaning, as a consequence of this change, e.g. the net6.0
class-parse.dll
now references the .NET Standard 2.0 build ofXamarin.Android.Tools.Bytecode.dll
(v0.2.0.11), whereas before this change it referenced the net6.0 build, 7.0.0.7.In this case that likely doesn't matter, as there's no
#if NET
inXamarin.Android.Tools.Bytecode.dll
, but if that ever changed…I suspect what we actually want/need is to instead add a 3rd
$(TargetFramework)
value in many of these places, instead of replacingnet6.0
withnet7.0
, a'la: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.
Or this is a good reason to make everything target
net7.0
instead of some things still targetingnet6.0
.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've moved everything back to target net7.0.