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

New error in .NET 8 Android projects #3165

Open
jonathanpeppers opened this issue Dec 16, 2022 · 16 comments
Open

New error in .NET 8 Android projects #3165

jonathanpeppers opened this issue Dec 16, 2022 · 16 comments

Comments

@jonathanpeppers
Copy link
Member

Starting in this Maestro bump: xamarin/xamarin-android#7630

We started seeing:

ILLink error IL1034: Root assembly 'MyAndroidApp, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' does not have entry point.

Might be caused by: #3124 (comment)

Android apps are class libraries, because there is no static void Main() method.

To work around this I did:

  <Target Name="_FixRootAssembly" AfterTargets="PrepareForILLink">
    <ItemGroup>
      <TrimmerRootAssembly Update="@(TrimmerRootAssembly)" Condition=" '%(RootMode)' == 'EntryPoint' " RootMode="Library" />
    </ItemGroup>
  </Target>

Filing an issue here, to see if this is what we should actually do -- or something else.

jonathanpeppers added a commit to xamarin/xamarin-android that referenced this issue Dec 16, 2022
Changes: dotnet/installer@243326d...167a4ed
Changes: dotnet/linker@13b8d6d...27ce032
Changes: dotnet/runtime@dd7fdb7...1a37caf
Changes: dotnet/emsdk@b6656f5...96351a7

Updates:

* Microsoft.Dotnet.Sdk.Internal: from 8.0.100-alpha.1.22602.5 to 8.0.100-alpha.1.22611.1
* Microsoft.NET.ILLink.Tasks: from 7.0.100-1.22564.1 to 8.0.100-1.22609.1
* Microsoft.NETCore.App.Ref: from 8.0.0-alpha.1.22559.2 to 8.0.0-alpha.1.22605.1
* Microsoft.NET.Workload.Emscripten.net7.Manifest-8.0.100: from 8.0.0-alpha.1.22554.1 to 8.0.0-alpha.1.22558.2

Other changes:

* Update `.apkdesc` files

* [illink] avoid `IL1034` for `.exe` files

Context: dotnet/linker#3124 (comment)

Android projects are class libraries, and this currently triggers:

    ILLink error IL1034: Root assembly 'MyAndroidApp, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' does not have entry point.

Issue filed at: dotnet/linker#3165

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
@vitek-karas
Copy link
Member

@sbomer please take a look.

@sbomer
Copy link
Member

sbomer commented Dec 20, 2022

I think the workaround is doing the right thing (edit: see my next comment), and I think it would make sense to put the fix for this into the SDK. In #3131 we decided against it, but I think this issue shows that we really do already have non-exe projects as a supported scenario - I just didn't know it.

I think we should fix this by using RootMode="library" RootMode="visible" when the OutputType is Library. From xamarin/xamarin-android#4604, I think this will work for the Android scenario. Does that sound right to you @jonathanpeppers?

Note @vitek-karas @marek-safar @MichalStrehovsky this will make "library" "visible" root mode an officially supported feature for the first time - are we OK with this? If not, the alternative would be to use "all" to get the behavior from before #3124.

@sbomer
Copy link
Member

sbomer commented Dec 20, 2022

Sorry, I just remembered that "library" has extra behavior which disables optimizations for when we are only trimming a single library against reference assemblies. I think what we actually want is "visible". @jonathanpeppers you might want to use RootMode="Visible" (or "All", to get the previous behavior) in your workaround instead, until we resolve this issue.

@sbomer
Copy link
Member

sbomer commented Dec 20, 2022

Another option, instead of using the assembly RootMode, would be to let the Android SDK tell the linker via XML descriptor which entry points to keep. Presumably there is an effective entry point (or multiple) for Android apps that is statically known, even if it's not encoded as such in the PE file. This option has slightly cleaner trimming behavior, but I don't like that it relies on the XML.

@MichalStrehovsky
Copy link
Member

Note @vitek-karas @marek-safar @MichalStrehovsky this will make "library" "visible" root mode an officially supported feature for the first time - are we OK with this? If not, the alternative would be to use "all" to get the behavior from before #3124.

It's not clear to me what "publishing a self-contained library" actually means. (We only support trimming for selfcontained deployments.)

E.g. if we want to support dotnet/runtime#79377 with IL Linker, we would actually only want to root things with [UnmanagedCallersOnly(EntryPoint = "Foo")]. This is a definition for "publishing a self-contained library" that makes sense.

@jonathanpeppers what are the trimming roots in an Android application?

@marek-safar
Copy link
Contributor

Note @vitek-karas @marek-safar @MichalStrehovsky this will make "library" "visible" root mode an officially supported feature for the first time - are we OK with this? If not, the alternative would be to use "all" to get the behavior from before #3124.

I'm not sure that's what we really want. My understanding of the scenario is that this is not library mode trimming but trimming with a custom entry point name that is hardcoded somewhere in XA interop. I think the expectation is that all public methods in such library will be trimmed.

@jonathanpeppers
Copy link
Member Author

Android has many roots, where Android/Java will call into managed code, such as:

  • Android.App.Application
  • Android.App.Activity's
  • Services
  • Broadcast receivers
  • Content providers
  • Views created from Android .xml
  • Probably more, I'm not thinking of

I think Android tried really hard to be "decoupled", where these types don't necessarily have references to each other.

The solution we brought over from Xamarin.Android, is anything that subclasses Java.Lang.Object is preserved by a custom linker step. So, I would expect most Android apps have most of the main assembly preserved. It sounds like [UnmanagedCallersOnly] would do something similar.

@jonathanpeppers
Copy link
Member Author

/cc @jonpryor you might find this thread interesting.

@MichalStrehovsky
Copy link
Member

If there's a custom step that roots everything necessary, maybe the fix would be not to root anything and leave it to the custom step? Basically any extra rooting the TrimmerRootAssembly can bring would probably be just garbage we could have collected during trimming.

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Dec 21, 2022
Context: dotnet/linker#3165 (comment)

In 2a10e96, I added a workaround for new linker behavior.

It turns out I should have used `All` instead of `Library` for now.

The original issue is still under discussion.
jonathanpeppers added a commit to xamarin/xamarin-android that referenced this issue Dec 21, 2022
…#7651)

Context: dotnet/linker#3165 (comment)

In 2a10e96, I added a workaround for new linker behavior.

It turns out I should have used `All` instead of `Library` for now.

The original issue is still under discussion.
@vitek-karas
Copy link
Member

Just for my understanding:
I thought that by default we don't trim the app's assembly in Xamarin/Mobile scenarios, so this should not be a problem there, right?
I assume this only happens if the app opts into aggressive trimming.

I would obviously prefer more "precise" solution than either "all" or "visible", as both are very generic. I would also much prefer to not expose a "library"/"visible" mode as officially supported. And I 100% agree with Michal that it's currently basically undefined what "trimmed" library means - since we don't support FDD trimming and SDK doesn't have a concept of a self-contained library (well not yet really, NativeAOT is AFAIK the first case where it's officially supported).

So if we can determine the roots some other way - either a custom step or XML, that would be preferable.

As for immediate workaround - I would use the RootMode=all which gets us back the current behavior before the change.

@jonathanpeppers
Copy link
Member Author

I thought that by default we don't trim the app's assembly in Xamarin/Mobile scenarios, so this should not be a problem there, right?

Customers would have to opt into more aggressive trimming by either using our old setting AndroidLinkMode=Full or setting %(TrimMode) on assemblies. By default we only trim the BCL and our assembly with Android APIs and not the user's code.

SDK doesn't have a concept of a self-contained library

Android apps (actually libraries) are self-contained in .NET 6 and 7. We found when we enabled SelfContained=true, all the MSBuild plumbing seems to work fine for a library. But yes, maybe this isn't officially supported.

@vitek-karas
Copy link
Member

We found when we enabled SelfContained=true, all the MSBuild plumbing seems to work fine for a library. But yes, maybe this isn't officially supported.

We discussed this couple of years ago and at least back then it was "accidentally mostly working", so it probably works for relatively simple cases but it's not tested. But aside from that - it's not clear what it "should" do in most cases. For example, if I just "dotnet new classlib" and then "dotnet build --self-contained true" the output is basically useless. The runtime doesn't have a concept of a self-contained library/component, so even if the files on disk look OK, it won't run or anything. The SDK in this case behaves as if it's publishing an app, and in the case of Android that matches relatively closely what we want, but in other scenarios it's just plain wrong.

@rolfbjarne
Copy link
Member

This hits iOS as well, where app extensions are like Android projects: they're libraries and don't have a standard Main function.

We also brought over the same solution from the Xamarin days: we have a custom linker step that roots API that iOS will call.

This seems rather simple to solve: an executable is really a library with a well-defined entry point / root. Couldn't the trimmers support a library with custom entry points as well - RootMode=custom-roots? And then any roots in the library would have to be specified somehow (xml file, custom linker step), otherwise the entire library would be linked away.

@vitek-karas
Copy link
Member

Couldn't the trimmers support a library with custom entry points as well - RootMode=custom-roots? And then any roots in the library would have to be specified somehow (xml file, custom linker step), otherwise the entire library would be linked away.

This should be possible already. Don't specify root mode, just mark everything as action=link so that trimmer does normal trimming on everything. And then use descriptor XML to "root" the entry points necessary for a given project.

What would be different even if we did add a feature for this?

@rolfbjarne
Copy link
Member

Couldn't the trimmers support a library with custom entry points as well - RootMode=custom-roots? And then any roots in the library would have to be specified somehow (xml file, custom linker step), otherwise the entire library would be linked away.

This should be possible already. Don't specify root mode, just mark everything as action=link so that trimmer does normal trimming on everything. And then use descriptor XML to "root" the entry points necessary for a given project.

What would be different even if we did add a feature for this?

Yes, that seems to work, as long as I use an xml descriptor (our custom linker step never seems to be hit for what we need to preserve though, which is probably because we use a MarkDispatcher subclass, and that only processes assemblies that are already marked somehow).

Also I have to undo this:

<!-- Root the main assembly entry point. -->
<ItemGroup>
  <TrimmerRootAssembly Include="@(IntermediateAssembly)" RootMode="EntryPoint" />
</ItemGroup>

Would it make sense to make it conditional on the project not being a library instead?

<!-- Root the main assembly entry point. -->
<ItemGroup>
  <TrimmerRootAssembly Include="@(IntermediateAssembly)" Condition="'$(OutputType)' != 'Library'" RootMode="EntryPoint" />
</ItemGroup>

@vitek-karas
Copy link
Member

This all followes form the fact that trimmer was not made to work on "library" projects. Granted, it doesn't defend against that (probably should have). If iOS and other targets use it on a Library project it would make sense to add better support for that in a more "opfficial" way. That said, I think it should be limited to the targets we know how to handle. As mentioned above, we don't know how to handle normal "classlib" projects with trimming currently, so ideally those would provide a good UX (warning/error something like that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants