-
Notifications
You must be signed in to change notification settings - Fork 58
[Java.Interop.Tools.JavaCallableWrappers] fix net8.0 targeting in XA #1197
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
Conversation
xamarin/xamarin-android's build began failing with: (CoreCompile target) -> Xamarin.Android.Build.Tasks\Utilities\MamJsonParser.cs(92,43): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level [Xamarin.Android.Build.Tasks\Xamarin.Android.Build.Tasks.csproj] Xamarin.Android.Build.Tasks\Utilities\MamJsonParser.cs(92,81): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level [Xamarin.Android.Build.Tasks\Xamarin.Android.Build.Tasks.csproj] Xamarin.Android.Build.Tasks\Utilities\MavenExtensions.cs(26,32): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level [Xamarin.Android.Build.Tasks\Xamarin.Android.Build.Tasks.csproj] This was happening because the `net8.0` build of `Java.Interop.Tools.JavaCallableWrappers.dll` was being used and should not be: we want the `netstandard2.0` version to be used for MSBuild task assemblies. To fix this, let's only build `netstandard2.0` when `$(XABuild)` is `true`. We should also avoid using `$(AppendTargetFrameworkToOutputPath)=false` when multi-targeting: stop using it inside XA's build.
I'm slightly confused by this.
Because If not, couldn't we use The existence of this PR suggests I don't understand something within MSBuild-land. :-) |
I wonder if it would be cleaner to create a |
|
src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers.csproj
Outdated
Show resolved
Hide resolved
Ah. What if we flipped the order, so that it was instead: <TargetFrameworks>$(DotNetTargetFramework);netstandard2.0</TargetFrameworks> Would that alter anything? |
This would maybe work: <TargetFrameworks>$(DotNetTargetFramework);netstandard2.0</TargetFrameworks> But incremental builds would run every time, because the two target frameworks copy on top of each other. I used |
|
Builds are done in parallel, so it is likely a race condition. It can also lead to this error:
|
<PropertyGroup> | ||
<!-- Only use $(ToolOutputFullPath) for netstandard2.0 --> | ||
<PropertyGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' "> | ||
<OutputPath>$(ToolOutputFullPath)</OutputPath> | ||
</PropertyGroup> |
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 think what I ended up with is simple enough. This prevents us from putting the net8.0
library in $(ToolOutputFullPath)
.
The build error is gone in dotnet/android#8751, but we still have some runtime test failures.
Context: https://github.com/xamarin/xamarin-android/pull/8748
Context: 56b7eeb771aceb7d47b31a4337f7b0e73ba74447
xamarin/xamarin-android#8748 attempts to bump xamarin-android to use
commit 56b7eeb7, and fails to build:
(CoreCompile target) ->
Xamarin.Android.Build.Tasks\Utilities\MamJsonParser.cs(92,43): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level [Xamarin.Android.Build.Tasks\Xamarin.Android.Build.Tasks.csproj]
Xamarin.Android.Build.Tasks\Utilities\MamJsonParser.cs(92,81): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level [Xamarin.Android.Build.Tasks\Xamarin.Android.Build.Tasks.csproj]
Xamarin.Android.Build.Tasks\Utilities\MavenExtensions.cs(26,32): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level [Xamarin.Android.Build.Tasks\Xamarin.Android.Build.Tasks.csproj]
The cause of the failure is that 56955d9a updated
`Java.Interop.Tools.JavaCallableWrappers.csproj` to multitarget both
netstandard2.0 and net8.0. This is nominally "fine", except that
`Java.Interop.Tools.JavaCallableWrappers.csproj` *also* sets
[`$(AppendTargetFrameworkToOutputPath)`][0]=false, which means that
both builds use the *same* output directory. This means that the
netstandard2.0 and net8.0 build outputs clobber each other -- in
parallel! -- which means that the `Xamarin.Android.Build.Tasks.csproj`
build doesn't reliably use the netstandard2.0 output assembly.
Fix this scenario by no longer overriding the
`$(AppendTargetFrameworkToOutputPath)` MSBuild property, and only
setting `$(OutputPath)` to `$(ToolOutputFullPath)` for netstandard2.0
builds.
Finally, remove use of `XAConfig.props`. This appears to be vestigial,
as we can't find any current code that would produce this file.
[0]: https://learn.microsoft.com/dotnet/core/project-sdk/msbuild-props#appendtargetframeworktooutputpath |
xamarin/xamarin-android's build began failing with:
This was happening because the
net8.0
build ofJava.Interop.Tools.JavaCallableWrappers.dll
was being used and should not be: we want thenetstandard2.0
version to be used for MSBuild task assemblies.To fix this:
$(OutputPath)
to$(ToolOutputFullPath)
fornetstandard2.0
This solves the build error above.
Other cleanup:
No need to import
XAConfig.props
, as it no longer existsNo need to set
$(AppendTargetFrameworkToOutputPath)
, as it is already inDirectory.Build.props