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 Mono.Cecil.dll to 0.11.5. #9078

Merged
merged 1 commit into from
Jul 3, 2024
Merged

Bump Mono.Cecil.dll to 0.11.5. #9078

merged 1 commit into from
Jul 3, 2024

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Jul 3, 2024

Context: #9043

While attempting to mark API-35 stable, we hit this error running all of the in-tree Windows smoke tests on CI:

D:\a\_work\1\s\bin\Release\dotnet\packs\Microsoft.Android.Sdk.Windows\35.0.0-ci.pr.gh9043.6\tools\
Xamarin.Android.Bindings.JavaDependencyVerification.targets(22,5): error MSB4062: The 
"Xamarin.Android.Tasks.GetMicrosoftNuGetPackagesMap" task could not be loaded from the assembly 
D:\a\_work\1\s\bin\Release\dotnet\packs\Microsoft.Android.Sdk.Windows\35.0.0-
ci.pr.gh9043.6\tools\Xamarin.Android.Build.Tasks.dll. Could not load file or assembly 'Mono.Cecil, Version=0.11.4.0, 
Culture=neutral, PublicKeyToken=50cebf1cceb9d05e'. The system cannot find the file specified. Confirm that the 
<UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a 
public class that implements Microsoft.Build.Framework.ITask. [D:\a\_work\1\a\TestRelease\07-
02_16.40.15\temp\DotNetBuildandroid-armFalseFalseFalse\UnnamedProject.csproj]

This appears to be caused by the fact that today we:

  • Build the ILLink* project(s) from runtime with their 0.11.5 Microsoft.DotNet.Cecil package.
  • Build the rest of XA with 0.11.4 Mono.Cecil package.
  • The 0.11.4 Mono.Cecil "wins" and ends up in the output directory and the sdk pack.

So long as ILLink* doesn't use any Cecil 0.11.5 API, this works, however it is risky because we don't know exactly what API ILLink* uses.

Removing our building of API-34 (by making API-35 stable), and no longer building API-35 as a preview, this order seems to get changed and the 0.11.5 Cecil "wins" and ends up in the output directory. This causes the assembly load error listed above.

We could try to fix the ordering and make 0.11.4 "win", but this leaves us vulnerable to missing some API that ILLink* needs. As such, it's better that we update the rest of XA to use the 0.11.5 version of Mono.Cecil.

This update breaks the FixAbstractMethodsStep_Explicit unit test. Specifically, this logic now returns null instead of being able to be resolved:

new MethodReference (iface_method.Name, void_type, iface).Resolve ();

It feels like this makes sense: a method name and return type doesn't seem like it would be enough to resolve, as parameters are not considered.

The fix is simply to use the existing MethodDefinition as the MethodReference, there is no reason to create a new one. This change fixes the test.

This change was pulled out of #9043 to ensure that it doesn't cause any breakage.

Note: Technically ILLink* references Microsoft.DotNet.Cecil version 0.11.4-alpha.24313.1, which implies that it is 0.11.4, however the Mono.Cecil.dll inside is versioned as 0.11.5.

@jpobst jpobst marked this pull request as ready for review July 3, 2024 19:44
@jonpryor jonpryor merged commit 54a9c8c into main Jul 3, 2024
58 checks passed
@jonpryor jonpryor deleted the update-cecil branch July 3, 2024 20:44
grendello added a commit that referenced this pull request Jul 5, 2024
* main:
  [tests] verify trimmer warnings where appropriate (#9076)
  Bump to jbevain/cecil@8c123e1 (#9078)
  [trimming] remove `$(NullabilityInfoContextSupport)` (#9069)
  [build] Bump `$(XABuildToolsVersion)`=35 (#9071)
  [ci] Move PR build to shared pool (#8854)
  Use AsyncTask from xamarin-android-tools (#9017)
  Bump to dotnet/sdk@02c06d398a 9.0.100-preview.7.24351.1 (#9067)
  [trimming] use public `$(MetricsSupport)` property (#9068)
  [ci] Update resourceManagement.yml (#9070)
  Bump to dotnet/android-api-docs@c14203771a (#8992)
  [trimming] remove `$(_AggressiveAttributeTrimming)` by default (#9062)
@github-actions github-actions bot locked and limited conversation to collaborators Aug 4, 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.

2 participants