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

[mtouch][linker] Remove type forwarders. Fixes #51805 (#1589) #1600

Merged
merged 1 commit into from Jan 31, 2017

Conversation

spouliot
Copy link
Contributor

a. System.Net.Http.Primitives.dll is user code and contains type
forwarders (it's like a facade) to another facade assembly,
System.Net.Primitives.dll, that ships with the SDK;

b. The former, System.Net.Http.Primitives.dll, is not processed by
the linker, e.g. no code is removed and the assembly cannot be deleted.
However we save back (as much as we can [1]) the result of any type
being resolved;

c. It also means the later, System.Net.Primitives.dll, is fully linked
and (in many cases) can be removed from the final application (as it's
mostly forwarders).

d. This means the final, re-saved, System.Net.Http.Primitives.dll binary
could point to non-existing metadata, i.e. the removed
System.Net.Primitives.dll, because of [1].

Because we resolve (and save) the forwarders and because we do not
allow code downloads or generation (Apple restriction) it is possible to
remove the forwarders, which will fix the issue for XI.

[1] The scope of exported types cannot be updated
https://github.com/jbevain/cecil/blob/abb4e902da25fb4c23b6a9bfc6e5f9a034d04b3e/Mono.Cecil/ExportedType.cs#L41

There is also a enhancement bug, #11165, about this but it predated our
PCL support and the resolve-n-save that we now do for forwarders. This
is now fully fixed.

References:

a. System.Net.Http.Primitives.dll is user code *and* contains type
forwarders (it's like a facade) to another facade assembly,
System.Net.Primitives.dll, that ships with the SDK;

b. The former, System.Net.Http.Primitives.dll, is not processed by
the linker, e.g. no code is removed and the assembly cannot be deleted.
However we save back (as much as we can [1]) the result of any type
being resolved;

c. It also means the later, System.Net.Primitives.dll, is fully linked
and (in many cases) can be removed from the final application (as it's
mostly forwarders).

d. This means the final, re-saved, System.Net.Http.Primitives.dll binary
could point to non-existing metadata, i.e. the removed
System.Net.Primitives.dll, because of [1].

Because we resolve (and save) the forwarders *and* because we do not
allow code downloads or generation (Apple restriction) it is possible to
remove the forwarders, which will fix the issue for XI.

[1] The scope of exported types cannot be updated
https://github.com/jbevain/cecil/blob/abb4e902da25fb4c23b6a9bfc6e5f9a034d04b3e/Mono.Cecil/ExportedType.cs#L41

There is also a enhancement bug, xamarin#11165, about this but it predated our
PCL support and the resolve-n-save that we now do for forwarders. This
is now _fully_ fixed.

References:
* https://bugzilla.xamarin.com/show_bug.cgi?id=11165 (enhancement)
* https://bugzilla.xamarin.com/show_bug.cgi?id=51805
@monojenkins
Copy link
Collaborator

Build failure

@spouliot
Copy link
Contributor Author

Unrelated failures on intro-mac classic/32 bits - pending PR #1585

@spouliot spouliot merged commit c633bd3 into xamarin:master Jan 31, 2017
@spouliot spouliot deleted the bug51805 branch January 31, 2017 21:22
spouliot added a commit to spouliot/xamarin-macios that referenced this pull request Feb 3, 2017
spouliot added a commit that referenced this pull request Feb 3, 2017
* Revert "[mtouch][linker] Remove type forwarders. Fixes #51805 (#1589) (#1600)"

This reverts commit c633bd3.

* [mono] Bump mono to get latest cecil (cycle9) and fix linker's ExternalType.Scope. Fixes #52187 and #51805

Original fix for bug #51805 was reverted since it caused a regression [2]
when type forwarders are used thru reflection, which happens when
serializing some types.

[1] https://bugzilla.xamarin.com/show_bug.cgi?id=51805
[2] https://bugzilla.xamarin.com/show_bug.cgi?id=52187
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants