Skip to content

Conversation

@dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Jun 14, 2017

The linker was not producing ppdb files.
What is supposed to happen is the
linker if it changes a dll should emit a new pdb file
along with it. Because none of the assemblies were being
loaded with debug symbols. Hence no updated pdb.

So this commit loads the Symbols as part of the
linker process to ensure that we are able to emit the new
pdb when required. This is only done for files which
have been changed by the linker.

@radekdoulik
Copy link
Member

I wonder what is the performance penalty of this change. Could you please try if not reading the symbols (as before) and adding following change would fix it? It might help to keep the performance nearly the same.

index b69dd41..61dbcd0 100644
--- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs
+++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs
@@ -28,6 +28,7 @@ namespace MonoDroid.Tuner
                        }
 
                        if (changed) {
+                               Context.SafeReadSymbols (assembly);
                                AssemblyAction action = Annotations.HasAction (assembly) ? Annotations.GetAction (assembly) : AssemblyAction.Skip;
                                if (action == AssemblyAction.Skip || action == AssemblyAction.Copy || action == AssemblyAction.Delete)
                                        Annotations.SetAction (assembly, AssemblyAction.Save);

@dellis1972
Copy link
Contributor Author

@radekdoulik ok I'll give that a go

…not be debugged.

The linker was not producing ppdb files.
What is supposed to happen is the
linker if it changes a dll should emit a new pdb file
along with it. Because none of the assemblies were being
loaded with debug symbols. Hence no updated pdb.

So this commit loads the Symbols as part of the
linker process to ensure that we are able to emit the new
pdb when required. This is only done for files which
have been changed by the linker.
@dellis1972
Copy link
Contributor Author

@radekdoulik your suggestion seemed to work. Well my new unit test still passed so it was doing the same thing

@radekdoulik
Copy link
Member

That's great, thanks!

@radekdoulik radekdoulik merged commit 7151179 into dotnet:master Jun 15, 2017
jonpryor pushed a commit that referenced this pull request May 20, 2020
Context: dotnet/java-interop@b00e644

Changes: dotnet/java-interop@186174c...b00e644

  * dotnet/java-interop@b00e644: [Java.Interop.Tools.Generator] Create a v2 version of map.csv (#646)
  * dotnet/java-interop@1708d8a: [XAT.Bytecode] Bind Kotlin internal interfaces as package-private (#645)

In dotnet/java-interop@b00e644e, we added support for a "v2" version
of the `map.csv` file format used to bundle Java constant fields into
enumerations.

Convert `src/Mono.Android/map.csv` to use this new format to make
enumification easier going forward.

This does not add any new enumifications.

Additionally, the new enum code standardizes on using `$` to denote
nested classes, e.g. `java/lang/Thread$State`, not
`java/lang/Thread.State`.  `$` is correct JNI syntax or nested types.

~Half of this existing file used `$` and half used `.`.  This resulted
in needing `acceptable-breakage` fixes as this signature changed in the
`[IntDefinition]` attribute.
jonpryor pushed a commit that referenced this pull request May 26, 2020
Context: dotnet/java-interop@b00e644

Changes: dotnet/java-interop@d6024f1...76d1ac7

  * dotnet/java-interop@76d1ac7: [Java.Interop.Tools.Generator] Create a v2 version of map.csv (#646)
  * dotnet/java-interop@b6c15399: [XAT.Bytecode] Bind Kotlin internal interfaces as package-private (#645)

In dotnet/java-interop@b00e644e, we added support for a "v2" version
of the `map.csv` file format used to bundle Java constant fields into
enumerations.

Convert `src/Mono.Android/map.csv` to use this new format to make
enumification easier going forward.

This does not add any new enumifications.

Additionally, the new enum code standardizes on using `$` to denote
nested classes, e.g. `java/lang/Thread$State`, not
`java/lang/Thread.State`.  `$` is correct JNI syntax or nested types.

~Half of this existing file used `$` and half used `.`.  This resulted
in needing `acceptable-breakage` fixes as this signature changed in the
`[IntDefinition]` attribute.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 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.

3 participants