Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Sep 3, 2020

Backport of #41358 to release/5.0-rc2

/cc @CoffeeFlux @safern

Customer Impact

The tests outlined in #39379 uncovered various issues in our handling of satellite assemblies that would affect customers' abilities to use them. Satellite assemblies were not corrected added to the bundle and instead relied on the VFS to load. Even if added to the bundle, nothing would disambiguate them based on culture. Finally, attempting to load a satellite assembly for two different cultures would return the wrong assembly for the second load.

Testing

This was prompted by a test failure that is now functional, and as part of my investigation to ensure this would be safe to backport an additional test was added to verify the functionality of LoadFromAssemblyPath.

Risk

This makes some significant changes to loader behavior, especially related to how we register satellite assemblies, but this is inevitable given the previous handling was fairly broken. The riskiest component is the changes to MonoImage registration, but I went through the code in detail and the tests now cover the various scenarios we care about for wasm, so I think this is unlikely to regress any other part while enabling a previously-broken customer scenario.

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@CoffeeFlux CoffeeFlux added area-AssemblyLoader-mono Servicing-consider Issue for next servicing release review labels Sep 3, 2020
@ghost
Copy link

ghost commented Sep 3, 2020

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

@CoffeeFlux
Copy link
Contributor

cc: @marek-safar

@CoffeeFlux
Copy link
Contributor

CoffeeFlux commented Sep 3, 2020

Apparently Blazor has to delay the satellite assembly registration, so an additional change would be required on our side with a new API. That should end up on the PR this is coming from later today, but given the increasing complexity of this I'm a bit hesitant to take it for 5.0, especially since Pranav is out on the Blazor side (which will also require changes in how they register the assemblies). If you think this is worth it, I can reopen the backport tomorrow after the final changes are in, but otherwise I'm just going to delay this to 6.0.

@CoffeeFlux CoffeeFlux closed this Sep 3, 2020
@jkotas jkotas deleted the backport/pr-41358-to-release/5.0-rc2 branch September 4, 2020 04:08
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-AssemblyLoader-mono Servicing-consider Issue for next servicing release review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants