Bug 59886 - Android Linker not skipping specified assemblies
Summary: Android Linker not skipping specified assemblies
Status: RESOLVED UPSTREAM
Alias: None
Product: Android
Classification: Xamarin
Component: Linker (show other bugs)
Version: unspecified
Hardware: Macintosh Mac OS
: --- normal
Target Milestone: ---
Assignee: Radek Doulik
URL: https://github.com/garypope/LinkerPro...
Depends on:
Blocks:
 
Reported: 2017-10-02 09:03 UTC by Gary
Modified: 2017-10-02 20:56 UTC (History)
2 users (show)

See Also:
Tags:
Is this bug a regression?: No
Last known good build:


Attachments
Partial build output (434.08 KB, image/png)
2017-10-02 09:03 UTC, Gary
Details

Description Gary 2017-10-02 09:03:56 UTC
Created attachment 25050 [details]
Partial build output

When building a Xamarin Android project that has a reference to a net standard 2.0 library, which in turn references Microsoft.Identity.Client, the linker seems to have at least one, potentially two bugs. 

First is that it's unable to link Microsoft.Identity.Client (not sure whether it should be even attempting to in first place?). Second is that when I add the AndroidLinkSkip for that lib, the linker still attempts (and fails) to link that assembly.
Comment 1 Gary 2017-10-02 09:05:15 UTC
I've created a repo that shows this problem here: 
https://github.com/garypope/LinkerProblem

I'm using Xamarin.Android 8.0.0.35
Comment 2 Jon Douglas [MSFT] 2017-10-02 14:34:50 UTC
This bug is related to the following issue that has been fixed already:

https://bugzilla.xamarin.com/show_bug.cgi?id=59109 (iOS Specific)

This was also filed under the Android product:

https://bugzilla.xamarin.com/show_bug.cgi?id=59822 (Android Specific)

Specifically the following comment provides insights for the cause of this issue:

https://bugzilla.xamarin.com/show_bug.cgi?id=59822#c2

There is then a comment of where the actual fix is located:

https://bugzilla.xamarin.com/show_bug.cgi?id=59822#c4

In short, you will need Xamarin.Android 8.0.0.35 which should hopefully make 15.4 Preview 4.

Thus I am marking this issue as a DUPLICATE of 59822.

*** This bug has been marked as a duplicate of bug 59822 ***
Comment 3 Jon Douglas [MSFT] 2017-10-02 15:51:56 UTC
Taking a second look at this bug, I actually believe this is a different linking error that had very similar parameters to the known item I had marked as duplicate(F#, Microsoft.Identity.Client, Linker Problem). I am going to set the status to REOPENED given my earlier mistake. Apologies in advance.

Here is the output from the linker error that I believe differs from the issue in https://bugzilla.xamarin.com/show_bug.cgi?id=59822

2>C:\Program Files (x86)\Microsoft Visual Studio\Dogfood\Enterprise\MSBuild\Xamarin\Android\Xamarin.Android.Common.targets(1628,5): error MSB4018: The "LinkAssemblies" task failed unexpectedly.
2>Mono.Linker.MarkException: Error processing method: 'System.Void Microsoft.Identity.Client.AuthenticationActivity::OnResume()' in assembly: 'Microsoft.Identity.Client.dll' ---> Mono.Cecil.ResolutionException: Failed to resolve System.Void Android.Support.CustomTabs.CustomTabsIntent::LaunchUrl(Android.App.Activity,Android.Net.Uri)
2>   at Mono.Linker.Steps.MarkStep.HandleUnresolvedMethod(MethodReference reference)
2>   at Mono.Linker.Steps.MarkStep.MarkMethod(MethodReference reference)
2>   at Mono.Linker.Steps.MarkStep.MarkInstruction(Instruction instruction)
2>   at Mono.Linker.Steps.MarkStep.MarkMethodBody(MethodBody body)
2>   at Mono.Linker.Steps.MarkStep.ProcessMethod(MethodDefinition method)
2>   at Mono.Linker.Steps.MarkStep.ProcessQueue()
2>   --- End of inner exception stack trace ---
2>   at Mono.Linker.Steps.MarkStep.ProcessQueue()
2>   at Mono.Linker.Steps.MarkStep.Process()
2>   at Mono.Linker.Steps.MarkStep.Process(LinkContext context)
2>   at Mono.Linker.Pipeline.Process(LinkContext context)
2>   at MonoDroid.Tuner.Linker.Process(LinkerOptions options, LinkContext& context)
2>   at Xamarin.Android.Tasks.LinkAssemblies.Execute(DirectoryAssemblyResolver res)
2>   at Xamarin.Android.Tasks.LinkAssemblies.Execute()
2>   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
2>   at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext()
2>Done building project "LinkerProblem.Droid.fsproj" -- FAILED.

https://developer.android.com/reference/android/support/customtabs/CustomTabsIntent.html#launchUrl(android.content.Context, android.net.Uri) indicates that this was added in 25.0.0. Interestingly we see that the trace is expecting a different method that is of launchUrl(android.app.Activity, android.net.Uri). I am still investigating this issue and will add updates to this issue when I figure the source cause.
Comment 4 Jon Douglas [MSFT] 2017-10-02 15:58:15 UTC
The main cause is that Microsoft.Identity.Client has a dependency here:

Xamarin.Android.Support.CustomTabs (>= 23.3.0)

This of course installs a 23.3.0 version of this assembly which has the following definition:

LaunchUrl(Activity context, Uri url)

This of course does not exist for what is expected of the new definition in newer versions of this library. i.e.

LaunchUrl(Context context, Uri url)

Thus to fix this issue, you would need to explicitly use a newer version of Xamarin.Android.Support.CustomTabs. Whoever maintains this library should also bump the >= NuGet dependency to a newer version such as 25.4.0.2.
Comment 5 Gary 2017-10-02 16:11:50 UTC
Thanks Jon, I'll explicitly reference version 25.4.0.2 or higher for now and try the linker again. Thanks for your help, I'll let you know how I get on with it.
Comment 6 Jon Douglas [MSFT] 2017-10-02 16:15:35 UTC
Small correction and clarification on the previous comment. The dependency on 23.3.0 is *expecting* the method definition of LaunchUrl(Activity context, Uri url) whereas you might have a newer version installed such as 25.4.0.2+ which has the up to date method definition: LaunchUrl(Context context, Uri url). As you can imagine, the linker is trying to find a method that does not contain those parameters and thus the error is thrown. 

The library in question is located here: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet

The problematic lines are here:

https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/master/src/Microsoft.Identity.Client/Microsoft.Identity.Client.csproj#L134-L135

I have gone ahead and filed an issue on your behalf:

https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/476
Comment 7 Jon Douglas [MSFT] 2017-10-02 16:16:37 UTC
The current workaround for you since you do not control this library would be using Xamarin.Android.Support versions 23.3.0 as if you install the latest versions, it will have the new definition that the library does not expect.
Comment 8 Jon Douglas [MSFT] 2017-10-02 16:17:47 UTC
It looks like there is a fix here:

https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/443#issuecomment-321022300

Thus I am marking this issue as RESOLVED UPSTREAM
Comment 9 Gary 2017-10-02 16:47:23 UTC
Hey Jon, on that last Github the last action there seems to close the issue with a resolution of bumping the support libraries up to 25.4.0.2+ to workaround the issue. I'm going to go with your solution of bringing them down to 23.3.
Comment 10 Gary 2017-10-02 16:55:04 UTC
My apologies, my interpretation was off there. I see now KPanwar is talking about bumping Microsoft.Identity.Client references up to 25.4.0.2+ not that I should bump my dependencies up to that version. Thanks for logging that issue with the guys managing that library for me by the way!
Comment 11 Jon Douglas [MSFT] 2017-10-02 17:26:02 UTC
(In reply to Gary from comment #10)
> My apologies, my interpretation was off there. I see now KPanwar is talking
> about bumping Microsoft.Identity.Client references up to 25.4.0.2+ not that
> I should bump my dependencies up to that version. Thanks for logging that
> issue with the guys managing that library for me by the way!

No problem, thank you for reporting this and making our product better!
Comment 12 Gary 2017-10-02 19:30:42 UTC
Hi Jon, I'm so swamped in issues right now with Xamarin/Paket/Net Core that I've not noticed that this bug I logged is solely about the linker skip not working. 

If the AndroidSkipLink would work correctly then I should be able to avoid this issue by way of not linking Microsoft.Client.Identity without rolling back my support libraries to 23.3.0, is that not right?

As it stands, this issue and many more like it are piling up and I think we're going to have to ditch Xamarin now for good. It was a good platform a few years back but my team have been swamped with issues this past few weeks and we can't lose any more time to Xamarin bugs. (Walmart Labs).
Comment 13 Jon Douglas [MSFT] 2017-10-02 20:17:49 UTC
Hey Gary,

I do not believe this is Linker related in this case per-say. I rather believe it's a problem of NuGet dependency resolution and the linker catches the issue. See: 

https://docs.microsoft.com/en-us/nuget/consume-packages/dependency-resolution

I believe this is a red herring to the actual issue because even if the LinkAssemblies task ignored this skipped assembly, you would fail at runtime if this code got invoked. Secondly I believe that the LinkAssemblies task needs to load the assembly in order for it to be marked to be skipped. This error happens prior to that I believe which makes it seem like <AndroidLinkSkip> does not work.

However these items are added to a delimited list and then added to the options.SkippedAssemblies item to be processed by the linker:

https://github.com/xamarin/xamarin-android/blob/5e5e178769945ce87a0da6ea21637ea633f0d2c7/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs#L132-L144

You can then look to see how these are skipped: https://github.com/mono/linker/blob/master/tuner/Mono.Tuner/CustomizeActions.cs

I apologize for your trouble with the tooling, I believe this whole issue happened because of a bad dependency in the Microsoft.Identity.Client library and is resolved in their alpha version on myget. Many of these items are fairly new and need some time to iron out the issues such as netstandard2.0 support and paket items(Although paket has been around awhile).

Please let me know if there are any painpoints that I can help resolve. I know many of these items around nuget dependency resolution and the mono linker tend to be confusing.

Were you able to use the myget version of the Microsoft.Identity.Client package and then keep your Xamarin.Android.Support libraries at 25.3.1+?
Comment 14 Gary 2017-10-02 20:39:18 UTC
Hi Jon, thanks for your comprehensive response, I appreciate it. 

Yeah, I hear what you're saying and it makes sense. 

Our team has been moving to Xamarin native from Xamarin forms for the past month and we've hit so many issues with tooling now that I think ultimately this one issue is, or could be the straw that broke the camels back. From VS for Mac to the linker, designer, Multi-Dex issues, this is all costing us the same time that a regular native app would have in the first place.

No, I've not got the myget version yet. Hopefully that will resolve one problem when I do. I tried setting Paket to explicitly take 23.3.0 for the support libraries and all I got from that was a StackOverflow. Till then I'm just forced to have Multi-dex on with the linker off.
Comment 15 Jon Douglas [MSFT] 2017-10-02 20:56:21 UTC
(In reply to Gary from comment #14)
> Hi Jon, thanks for your comprehensive response, I appreciate it. 
> 
> Yeah, I hear what you're saying and it makes sense. 
> 
> Our team has been moving to Xamarin native from Xamarin forms for the past
> month and we've hit so many issues with tooling now that I think ultimately
> this one issue is, or could be the straw that broke the camels back. From VS
> for Mac to the linker, designer, Multi-Dex issues, this is all costing us
> the same time that a regular native app would have in the first place.
> 
> No, I've not got the myget version yet. Hopefully that will resolve one
> problem when I do. I tried setting Paket to explicitly take 23.3.0 for the
> support libraries and all I got from that was a StackOverflow. Till then I'm
> just forced to have Multi-dex on with the linker off.

No problem Gary, If you have any suggestions for anything that we can improve or areas that are real pain points that we can look into making less painful please let me know. For multidex and linker type issues, I have quite a few blogs on my personal blog that might help demystify these topics:

https://www.jon-douglas.com/archive/

Please be aware that any and all feedback will be put in the right hands to improve the product overall. Thanks again!

Note You need to log in before you can comment on or make changes to this bug.