-
Notifications
You must be signed in to change notification settings - Fork 528
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
[build] Stop producing Xamarin.Android.Cecil.dll #8489
Conversation
Context: xamarin/monodroid@b934949 The Xamarin.Android.Cecil.dll assembly was created to to avoid potential conflicts and versioning issues with the copy of Cecil that Xamarin Studio was using at the time. Considering our migration to .NET, and how infrequently we change our Mono.Cecil reference these days, these potential issues are likely no longer a concern. Our unique Xamarin.Android.Cecil.dll copy has been removed in favor of Mono.Cecil package references in all projects that depend on it.
Adding the do-not-merge label as we will likely want some more external validation in the IDEs before merging this. |
@pjcollins the main concern around this was not our extensions, but other third party extensions like that Refectoring one. If they used a Cecil and were loaded first it would break out builds. @tondat any thoughts on this? |
<_MSBuildFiles Include="$(MicrosoftAndroidSdkOutDir)Mono.Cecil.dll" /> | ||
<_MSBuildFiles Include="$(MicrosoftAndroidSdkOutDir)Mono.Cecil.pdb" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first concern was that MAUI ships Mono.Cecil
, will we conflict?
So, I downloaded the package and inspected it: https://www.nuget.org/packages/Mono.Cecil/0.11.4
It looks to be strong named and appropriately versioned:
So, what may happen is that if MAUI ships Mono.Cecil
, we may use theirs or vice-versa if the version & strong name key match. Although, it will likely "just work" if the version is identical.
I don't think there is a problem here -- it should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem used to be with VS extensions. It was never with other products like iOS etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.NET framework should know how to respect strong named + versioned assemblies and load new instances of them.
VS Mac would have had an issue on Mono, but I believe it will be out of support and never be able to build .NET 9 projects. VS Mac also made it to .NET 6, in its latest version?
Agreed that this feels like it could potentially conflict with other copies of But it also feels like strong naming should protect us from that? Unless someone else makes local changes to their copy and doesn't change the version number? |
Related: #3132 |
I spent some time testing in a Windows VM today and had no issues with some somewhat trivial build, deploy, debug, and hot reload scenarios with Android and MAUI apps using a VS 17.9 preview and WSA. I think we should be able to merge this, but am wondering if it makes sense to wait for the .NET 9 PR to land first @jonathanpeppers ? |
I don't think we should wait on the .NET 9 PR to merge this. It might be better to merge sooner than later, to find out if find any possible issue loading different |
@pjcollins: please also try removing
It doesn't look like removing |
Fixes: #3132
Context: https://github.com/xamarin/monodroid/commit/b9349491a141b5265e3d48eec9af589c8c5cc793
The Xamarin.Android.Cecil.dll assembly was created to to avoid potential
conflicts and versioning issues with the copy of Cecil that Xamarin
Studio was using at the time. Considering our migration to .NET, and
how infrequently we change our Mono.Cecil reference these days, these
potential issues are likely no longer a concern.
Our unique Xamarin.Android.Cecil.dll copy has been removed in favor of
Mono.Cecil package references in all projects that depend on it.