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

[Java.Interop.Tools.Cecil] Change DirectoryAssemblyResolver Warning to Diagnostic Output #622

Merged
merged 1 commit into from Apr 13, 2020

Conversation

dellis1972
Copy link
Contributor

Context xamarin/xamarin-android#2120

When building an app and using an Obfuscator you wne up with allot
of warnings like.

warning : Failed to read 'obj\Release\90\android\assets\DotfuscatorForms.Android.dll' with debugging symbols. Retrying to load it without it. Error details are logged below.
warning : Mono.Cecil.Cil.SymbolsNotMatchingException: Symbols were found but are not matching the assembly

These are probably the result of the Obfuscation process since the
assembly changed. However the warning was NOT useful or more
importantly... actionable by the user. It is just noise. It might
be useful for the developers writing the Java.Interop tooling itself.

So rather than a warning , lets change it to be Verbose. This will
allow it up show up in full diagnostic output, but NOT in a normal
build. Also it won't be a warning so users will not feel the need
to fix it.

…o Diagnostic Output

Context xamarin/xamarin-android#2120

When building an app and using an Obfuscator you wne up with allot
of warnings like.

	warning : Failed to read 'obj\Release\90\android\assets\DotfuscatorForms.Android.dll' with debugging symbols. Retrying to load it without it. Error details are logged below.
	warning : Mono.Cecil.Cil.SymbolsNotMatchingException: Symbols were found but are not matching the assembly

These are probably the result of the Obfuscation process since the
assembly changed. However the warning was NOT useful or more
importantly... actionable by the user. It is just noise. It might
be useful for the developers writing the Java.Interop tooling itself.

So rather than a warning , lets change it to be `Verbose`. This will
allow it up show up in full diagnostic output, but NOT in a normal
build. Also it won't be a warning so users will not feel the need
to fix it.
@jonpryor jonpryor merged commit 1032c0e into xamarin:master Apr 13, 2020
@dellis1972 dellis1972 deleted the removeredundantwarn branch April 14, 2020 10:25
@brendanzagaeski
Copy link
Member

brendanzagaeski commented Apr 19, 2020

Draft release note

Here's a candidate release note for this change.

Questions

Does the reasoning in the last sentence make sense?

To align more closely with the behavior of other common .NET project types that do not warn about mismatched debugger symbols, Xamarin.Android projects now log these as informational messages instead of warnings.

I was thinking about why other project types didn't have the same issue with obfuscation and realized this difference in warning behavior was at least somewhat related. (I verified that a .NET Core app produced no warnings when I intentionally provided a mismatched .pdb file.)

Draft note

Java.Interop GitHub 622: warning : Failed to read ... with debugging symbols. Retrying to load it without it. and Mono.Cecil.Cil.SymbolsNotMatchingException: Symbols were found but are not matching the assembly could appear during builds, for example for projects using obfuscation tools. To align more closely with the behavior of other common .NET project types that do not warn about mismatched debugger symbols, Xamarin.Android projects now log these as informational messages instead of warnings.

Other comments

Initially, I was also wondering if the warnings might be valuable to help expose managed linker bugs. Some of the comments on xamarin/xamarin-android#2120 over the years sound like they might have been related to linker issues. But I think any user-relevant effects on debugging symbols would always surface in other high visibility ways anyway, like breakpoints not working. (Let me know if that doesn't sound right though.)

jonpryor pushed a commit that referenced this pull request Apr 22, 2020
…o Diagnostic Output (#622)

Context: xamarin/xamarin-android#2120

When building an app and using an Obfuscator you wind up with a lot
of warnings such as:

	warning : Failed to read 'obj\Release\90\android\assets\DotfuscatorForms.Android.dll' with debugging symbols. Retrying to load it without it. Error details are logged below.
	warning : Mono.Cecil.Cil.SymbolsNotMatchingException: Symbols were found but are not matching the assembly

These are probably the result of the Obfuscation process since the
assembly changed.  However the warning was *not* useful or -- more
importantly -- actionable by the user.  It is just noise.  It might
be useful for the developers writing the Java.Interop tooling itself.

Rather than a warning, change the message to be `Verbose`. This will
allow it up show up in full diagnostic output, but NOT in a normal
build.  It won't be a warning so users will not feel a need to fix it.
@jpobst jpobst added this to the 10.4 (16.7 / 8.7) milestone Apr 23, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 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.

None yet

4 participants