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

Bump to xamarin/java.interop/master@a8f68e56 #5497

Merged
merged 1 commit into from
Jan 18, 2021

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Jan 13, 2021

Changes: dotnet/java-interop@3894cd7...a8f68e5

  • Use const instead of static readonly
  • [generator] Disable [SupportedOSPlatform] until .NET 5/6.
  • [generator] Wrap SupportedOSPlatformAttribute in NET
  • [build] use $(RollForward)=Major for console apps
  • [Java.Interop] Conditionally call ManagedPeer.Init()
  • [generator] Add [SupportedOSPlatform] in assemblies using ApiSince
  • [global.json] Update Microsoft.Build.NoTargets to 2.0.1.

@jonpryor
Copy link
Member

Looks like we do need this: dotnet/java-interop#779

because of the DotNetBuild failures, e.g. https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4370096&view=ms.vss-test-web.build-test-results-tab&runId=18107834&resultId=100055&paneView=attachments

/Users/runner/work/1/s/bin/TestRelease/temp/DotNetBuildandroid.21-armandroid.21-arm64android.21-x86android.21-x64False/obj/Debug/net6.0-android/UnnamedProject.AssemblyInfo.cs(21,38):
warning CS0436: The type 'SupportedOSPlatformAttribute' in '/Users/runner/work/1/s/bin/TestRelease/temp/DotNetBuildandroid.21-armandroid.21-arm64android.21-x86android.21-x64False/obj/Debug/net6.0-android/generated/src/__NamespaceMapping__.cs'
conflicts with the imported type 'SupportedOSPlatformAttribute' in 'System.Runtime, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.
Using the type defined in '/Users/runner/work/1/s/bin/TestRelease/temp/DotNetBuildandroid.21-armandroid.21-arm64android.21-x86android.21-x64False/obj/Debug/net6.0-android/generated/src/__NamespaceMapping__.cs'.

@jonathanpeppers jonathanpeppers changed the title Bump to xamarin/java.interop/master@44192fab Bump to xamarin/java.interop/master@a33084b3 Jan 14, 2021
Copy link
Member

@radekdoulik radekdoulik left a comment

Choose a reason for hiding this comment

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

There's another issue with SupportedOSPlatformAttribute.

/Users/runner/work/1/s/tests/Mono.Android-Tests/Java.Interop-Tests/obj/Release/net6.0-android/Java.Interop-Tests.NET.AssemblyInfo.cs(21,38): error CS0433: The type 'SupportedOSPlatformAttribute' exists in both 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065' and 'System.Runtime, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' [/Users/runner/work/1/s/tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.NET.csproj] [/Users/runner/work/1/s/tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Mono.Android.NET-Tests.csproj]

@radekdoulik
Copy link
Member

radekdoulik commented Jan 14, 2021

I think the problem here is that the attribute class is also added to Mono.Android assembly and we are still building it with netcoreapp3.1 TFM.

@jpobst do we need it generated in multiple places? Would it be possible to just add it to legacy Mono.Android assembly and don't generate it elsewhere?

@jonathanpeppers
Copy link
Member Author

So... Mono.Android.csproj should probably be net6.0, right?

But we can't do that because msbuild with Mono can't build anything net5.0 or higher...

So that leaves us needing to build this entire repo with dotnet build?

@jpobst
Copy link
Contributor

jpobst commented Jan 14, 2021

I think this is a special case because we use [InternalsVisibleTo]:

https://github.com/xamarin/xamarin-android/blob/master/src/Mono.Android/Properties/AssemblyInfo.cs.in#L32

Normally the Mono.Android.dll version will not be visible to other assemblies.

What was the fix for the other place we were seeing this? Can it be applied to Java.Interop-Tests?

@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Jan 14, 2021

I added NET to $(DefineConstants) for the netcoreapp3.1 build. Let's see if that works.

@jpobst
Copy link
Contributor

jpobst commented Jan 14, 2021

We can also just comment out this code until we are building with net6.0 if this is blocking people.

@jonathanpeppers
Copy link
Member Author

@jpobst yeah, it didn't work, got a bunch of errors like:

error CS0234: The type or namespace name 'SupportedOSPlatformAttribute' does not exist in the namespace 'System.Runtime.Versioning' 

Could we put it behind a feature flag? It seems like it work work in apps, but not Mono.Android.csproj.

@jpobst
Copy link
Contributor

jpobst commented Jan 14, 2021

We can comment it out for now, until we are compiling .NET 5/6 properly. It's really only needed for Mono.Android.dll. No user assemblies are going to have api-since data.

jonpryor pushed a commit to dotnet/java-interop that referenced this pull request Jan 14, 2021
Context: da12df4
Context: a33084b
Context: dotnet/android#5497 (review)

We cannot currently build `net5.0` or `net6.0` assemblies in our
xamarin-android tree; we instead do some workaround that involves
compiling with `netcoreapp3.1` while referencing the 5.0 BCL.

This creates a conflict between the local `[SupportedOSPlatform]` we
create in `Mono.Android.dll` built for `netcoreapp3.1` , and the real
one in the 5.0 BCL.

	error CS0433: The type 'SupportedOSPlatformAttribute' exists in both
	'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065' and
	'System.Runtime, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
	[/Users/runner/work/1/s/tests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.NET.csproj]

Disable generating `SupportedOSPlatformAttribute` until we can build
`Mono.Android.dll` with `NET` defined, in which case it will not
contain the local attribute, and we can re-enable da12df4.
@radekdoulik
Copy link
Member

Weird error, trying rebuild.

/var/folders/9k/brhlbh9d729gxd3m_9j0t23w0000gn/T/tmpddf75c38a8ce438aaf930877db7fd94d.exec.cmd: line 2: 35321 Illegal instruction: 4  /Users/builder/Library/Android/dotnet/dotnet pack -p:Configuration=Release -p:NuGetLicense=/Users/builder/azdo/_work/1/s/xamarin-android/external/monodroid/tools/scripts/License.txt -p:AndroidRID=android.21-arm -p:AndroidABI=armeabi-v7a "/Users/builder/azdo/_work/1/s/xamarin-android/build-tools/create-packs/Microsoft.Android.Runtime.proj"

Changes: dotnet/java-interop@3894cd7...a8f68e5

* Use `const` instead of `static readonly`
* [generator] Disable [SupportedOSPlatform] until .NET 5/6.
* [generator] Wrap SupportedOSPlatformAttribute in NET
* [build] use $(RollForward)=Major for console apps
* [Java.Interop] Conditionally call ManagedPeer.Init()
* [generator] Add [SupportedOSPlatform] in assemblies using ApiSince
* [global.json] Update Microsoft.Build.NoTargets to 2.0.1.
@jonathanpeppers jonathanpeppers changed the title Bump to xamarin/java.interop/master@a33084b3 Bump to xamarin/java.interop/master@a8f68e56 Jan 15, 2021
@radekdoulik radekdoulik merged commit 7db5be8 into dotnet:master Jan 18, 2021
@jonathanpeppers jonathanpeppers deleted the java.interop-44192fab branch January 18, 2021 16:43
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants