dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.93k stars 532 forks source link

Set `AssemblyVersion` for `Mono.Android.dll` to `(ApiLevel).0.0.0` #6903

Open jpobst opened 2 years ago

jpobst commented 2 years ago

Context: https://github.com/xamarin/java.interop/issues/962

Like Java.Interop.dll, traditionally we have not versioned Mono.Android.dll to allow binding nugets made with ex: d17-2 to be used with d17-1 applications. In a .NET world, we should restrict along the .NET major version. That is, bindings nugets made for net7.0 should not work in net6.0.

To enforce this, we should version Mono.Android.dll with 7.0.0.0. Additional versioning information can be placed in [AssemblyFileVersion].

Additional consideration

We may want to incorporate the API level as well. For example, a net7.0-android33.0 Mono.Android.dll should not be consumable by a net7.0-android32.0 application. Perhaps we really want something like 7.33.0.0?

Note this is not a silver bullet, as assemblies aren't intended to be versioned on 2 independent dimensions. For example, under this scheme, a net8.0-android32.0 application would be able to consume a net7.0-android33.0 assembly which is not intended. (Because 8.32.0.0 > 7.33.0.0)

We should think about the implications of this.

As an aside, we should also verify every assembly we ship in .NET 7 is versioned correctly.

jonpryor commented 2 years ago

We may want to incorporate the API level as well. For example, a net7.0-android33.0 Mono.Android.dllshould not be consumable by a net7.0-android32.0 application. Perhaps we really want something like 7.33.0.0?

@jonathanpeppers & @pjcollins: please correct me if I'm wrong, but I believe that dotnet build won't allow a net6.0-android32.0 assembly to reference net6.0-android33.0 assembly. Consequently, I don't think we need to set the assembly version to "further" enforce this; "just" an assembly version of 7.0.0.0 should be sufficient.

jpobst commented 2 years ago

I think you get some guarantees from NuGet, but if you're not using NuGet it would be nice to get some protection from assembly version. I am also thinking more about bindings assemblies than Mono.Android.dll itself.

For example, if my binding library was built against API-33 and has a reference to Mono.Android.dll 7.0.0.0, it would be usable by an application targeting net7.0-android32.0 if it was added as a <Reference>. (<PackageReference> should be blocked via NuGet protection, <ProjectReference> should be blocked by TFM.)

jonathanpeppers commented 2 years ago

Here is what the spec says on this:

https://github.com/dotnet/designs/blob/main/accepted/2020/net5/net5.md#consuming-a-library-with-a-higher-targetplatformversion

I think if we put 7.0.0.0 that would be sufficient for Mono.Android.dll.

jpobst commented 2 years ago

Here is what the spec says on this:

error NU1202: Package 'Monkey.FizzBuzz' is not compatible with 'net5.0-ios13.0'. Package 'Monkey.FizzBuzz' supports: net5.0-ios14.0

This is a NuGet error, it will not prevent the situation if this is a <Reference> rather than a <PackageReference>.

jonathanpeppers commented 2 years ago

If someone puts:

<Reference Include="Mono.Android.dll" HintPath="C:\Program Files\dotnet\packs\Microsoft.Android.Runtime.32.android-arm64\32.0.300-rc.1.4\runtimes\android-arm64\lib\net6.0\Mono.Android.dll" />

If something is supposed to stop them, then the dotnet/sdk should do it. This could happen with any assembly in the BCL?

jpobst commented 2 years ago

This is less about our "BCL" assemblies and more about bindings libraries built against Mono.Android.dll.

If I create a binding library with net8.0-android32.0, it will have an assembly reference to Mono.Android 8.0.0.0.

If I try to use that library in my net7.0-android32.0 application via <Reference Include='MyBindingLib.dll' />, it will give me some error that is roughly:

Cannot resolve assembly Mono.Android.dll with version 8.0.0.0.  Only Mono.Android.dll 7.0.0.0 could be found.

This is the behavior we want.


However, not including a platform version leaves the door open to platform mismatches.

If I were to build my binding library with net7.0-android33.0, it will have an assembly reference to Mono.Android 7.0.0.0.

If I try to use that library in a net7.0-android32.0 application with a <Reference>, there will be no error even though it is not valid.

If we were using {.net version}.{platform version}.0.0 as a version number, then we would get the desired error:

Cannot resolve assembly Mono.Android.dll with version 7.33.0.0.  Only Mono.Android.dll 7.32.0.0 could be found.

This is better(?), though it still falls apart if the binding library is compiled against net7.0-android33.0 and is consumed in a net8.0-android32.0 application.

The library would reference Mono.Android 7.33.0.0. The .NET 8 BCL would have Mono.Android 8.32.0.0 which would match for versioning purposes, instead of throwing an error that the platform version is not high enough.


I'm don't actually think there is a good solution here. I just think we should be aware of the shortcomings and do the best we can. 🤷‍♂️

jonathanpeppers commented 2 years ago

In .NET 6, Mono.Android.dll is:

[assembly: AssemblyVersion("0.0.0.0")]

Do we need to change this to 6.0.0.0 for GA? It would prevent people from building .NET 6 assemblies and trying to use them in Xamarin.Android.

jonathanpeppers commented 2 years ago

@jonpryor and I discussed that we'll just move things to 7.0.0.0 in .NET 7.

jonpryor commented 2 years ago

"just move things to 7.0.0.0 in .NET 7" doesn't actually address or resolve @jpobst 's concerns.

Should we bump to 7.something for .NET 7? Yes.

Should we bump to 7.0.0.0 vs. 7.{api-level}.0.0? I don't think we've actually answered this.

Maybe we should try 7.{api-level}.0.0 and see if that breaks anything?

jpobst commented 2 years ago

For some additional context, Microsoft.iOS.dll only uses the platform version:

[assembly: AssemblyVersion("15.4.100.125")]

We definitely add new APIs in each platform version. Adding new APIs in net8.0-android33.0 that aren't in net7.0-android33.0 is probably rarer. 🤔

jonpryor commented 2 years ago

@jpobst wrote:

Adding new APIs in net8.0-android33.0 that aren't in net7.0-android33.0 is probably rarer. 🤔

Rarer, but that rare? Adding new APIs doesn't result in our API compatibility checks erroring out, so whenever we add APIs, e.g. to fix enumification issues (582adfd388e9b8b0f28ce51c47f19349a3f1aaa8? 0e2321d36734a280efd0ffb41e6af1938657186c? etc.), we're implicitly creating "something different", and arguably should do e.g. net6.0-android32.0, net6.0-android32.1, etc., which would almost certainly result in a state of affairs wherein net7.0-android32.0 is "most compatible with" net6.0-android32.2, if we did things "properly". Or, as you said, could result in net8.0-android33.0 having more APIs than net7.0-android33.0.

We need to figure out what, if anything, we do to address this. An assembly version of 7.33.0.0 won't help. An assembly version of 7.33.1.0, 7.33.2.0, etc. might help -- does anything actually care about the build value, in major.minor.build? -- but doesn't immediately answer the question of how we produce that build number in a way that is meaningful/useful. (Maybe the number of commits just in src/Mono.Android since some reference commit? Update API compat checks to error out on added APIs? Something else?)

jonpryor commented 2 years ago

Current plan is to follow iOS and use API levels as our assembly versions: 32.0.0.0, 33.0.0.0, etc.