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.

Sign up for GitHub

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

Jump to bottom

XA0105 should be an error, not an warning #6356

Open
jonpryor opened this issue Oct 1, 2021 · 0 comments
Open

XA0105 should be an error, not an warning #6356

jonpryor opened this issue Oct 1, 2021 · 0 comments
Assignees
@dellis1972 @jonathanpeppers
Labels
Area: App+Library Build Issues when building Library projects or Application projects.
Milestone
Under Consideration

Comments

@jonpryor
Copy link
Member

jonpryor commented Oct 1, 2021

Context: #6328 (comment)
Context: #6328 (comment)

Steps to Reproduce

Build and run the following app: Scratch.xa0105.zip

Expected Behavior

"It runs" and/or it doesn't build. (The latter is easier to achieve.)

Actual Behavior

TypeLoadException at runtime:

android.runtime.JavaProxyThrowable: System.TypeLoadException: Could not load type of field 'Library.Class1:p' (0) due to: Could not resolve type with token 01000011 from typeref (expected class 'Android.Util.Proto.ProtoOutputStream' in assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065') assembly:Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065 type:Android.Util.Proto.ProtoOutputStream member:(null)
  at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_savedInstanceState) [0x0000f] in <a7b7e1debecb4c31a65b5c5045f9f603>:0

Version Information

Happens with Xamarin.Android 12.0.0 via 25821c6 (so, lol? version doesn't really matter though. All current "Xammie" versions!)

Discussion

Warning XA0105 is emitted when the App assembly has a $(TargetFrameworkVersion) which is lower than that of a referenced assembly.

A warning is better than nothing, but not everybody reads or fixes warnings.

Thus, the repro:

  • Library.dll has a $(TargetFrameworkVersion)=v11.0, and uses ProtoOutputStream, introduced in API-30.
  • App.dll has a $(TargetFrameworkVersion)=v9.0, and references Library.dll.

This is exactly the scenario that XA0105 checks for.

The problem is that this can't possibly work at runtime: currently, the Mono.Android.dll included in the .apk is based on the $(TargetFrameworkVersion) of the App, i.e. v9.0. The v9.0 Mono.Android.dll can't possibly have types specific to the v11.0 Mono.Android.dll. Consequently, if/when App.dll attempts to use an "inappropriate" type from Library.dll, a TypeLoadException is thrown.

There are two "plausibly sane" fixes:

  1. When this scenario is detected, we override the Mono.Android.dll included in the .apk/Fast Deployment to be that of the highest detected $(TargetFrameworkVersion), i.e. v11.0 in this case. -or-
  2. We turn XA0105 into an error, and require that this scenario be fixed.
@jonpryor jonpryor added Area: App+Library Build Issues when building Library projects or Application projects. needs-triage Issues that need to be assigned. labels Oct 1, 2021
@jonpryor jonpryor changed the title XA0105 should be a warning, not an error XA0105 should be an error, not an warning Oct 1, 2021
@jonathanpeppers jonathanpeppers added this to the Under Consideration milestone Oct 1, 2021
@jonathanpeppers jonathanpeppers removed the needs-triage Issues that need to be assigned. label Oct 1, 2021
jonpryor pushed a commit that referenced this issue Oct 1, 2021
@jonathanpeppers
[One .NET] <LinkAssembliesNoShrink/> can skip the main assembly (#6328)
d7c0578 
Context: #6356
Context: #6357

We found an incremental build in a `dotnet new android` app with C#
code changes has this duration:

	100 ms  LinkAssembliesNoShrink                     1 calls

It turns out we don't actually need to run `<LinkAssembliesNoShrink/>`
against the "main" assembly in .NET 6.  After passing in
`$(TargetName)` and simply copying the file, this time improved to:

	  9 ms  LinkAssembliesNoShrink                     1 calls

This change should improve incremental C# changes by ~91ms in
`dotnet new android` projects.

In "Xammie" Xamarin.Android, it is possible for an application project
to reference a class library with a higher `$(TargetFrameworkVersion)`;
see [warning XA0105 docs][0].

We cannot apply this optimization for Xamarin.Android.

However, in thinking about and revisiting this code, we've made two
determinations for future work:

  * XA0105 being a warning is a terrible idea, as it can allow apps
    to crash via `TypeLoadException`.

    See #6356.

  * .NET 6 builds don't emit [XA2000 warnings][1] when a referenced
    assembly references `AppDomain.CreateDomain()`.  This isn't
    desirable, as `AppDomain.CreateDomain()` is still part of the
    .NET Standard 2.0 API contract, and thus could still be used,
    even though it won't work.  *Something* should warn if
    `AppDomain.CreateDomain()` is referenced, to reduce the need for
    manual app testing.

    See #6357.

[0]: https://github.com/xamarin/xamarin-android/blob/af60da7f13e613faa8ded5ef734e8c6d91445acc/Documentation/guides/messages/xa0105.md
[1]: https://github.com/xamarin/xamarin-android/blob/03d4e40d310255f80cc5ffb6475f03a02a561bc5/Documentation/guides/messages/xa2000.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dellis1972 dellis1972

@jonathanpeppers jonathanpeppers

Labels
Area: App+Library Build Issues when building Library projects or Application projects.
Projects
None yet
Milestone
Under Consideration
Development

No branches or pull requests
3 participants
@jonpryor @dellis1972 @jonathanpeppers