[WASM] Add satellite assemblies bundle and hook to register them #41358
Conversation
safern
requested review from
lambdageek,
marek-safar and
thaystg
as code owners
|
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. |
|
Tagging subscribers to this area: @CoffeeFlux |
|
@CoffeeFlux @lewing could you please review the fix |
|
@marek-safar yep I've been working with Santi on this, planning to review it first thing today |
There was a problem hiding this comment.
Thanks for doing this!
This is an initial review, but some of the changes I'm asking for are broad enough that I'll definitely take another full pass after this feedback has been addressed. Blocking the PR for now as a result.
I reviewed in two passes, the first for purely stylistic changes and the second looking at the logic, so you may see stylistic corrections on a section I later recommend changing up.
If you have any questions or disagree with any comments please do ping me; getting this landed is my main priority for today. Thanks again!
|
@safern did you forget to push? The issues are marked as resolved but I don't see any new commits or a force-push. |
I resolved them locally and adding an extra test. Should push shortly. |
CoffeeFlux
left a comment
CoffeeFlux
left a comment
There was a problem hiding this comment.
Looks good to me, thanks a lot!
|
/backport to release/5.0-rc2 |
|
Started backporting to release/5.0-rc2: https://github.com/dotnet/runtime/actions/runs/238409046 |
|
@CoffeeFlux this is now ready 😄 |
|
Waiting on a mono/mono lane and then we should be good to merge. |
|
mono/mono failures are known so we're good to merge this. |
| /** | ||
| * mono_image_open_a_lot_parameterized | ||
| * this API is not culture aware, so if we load a satellite assembly for one culture by name | ||
| * via this API, and then try to load it with another culture we will return the first one. | ||
| */ | ||
| static MonoImage * | ||
| mono_image_open_a_lot_parameterized (MonoLoadedImages *li, MonoAssemblyLoadContext *alc, const char *fname, MonoImageOpenStatus *status, gboolean refonly, gboolean load_from_context, gboolean *problematic) |
There was a problem hiding this comment.
I don't understand this comment. mono_image_open_a_lot_parametrized doesn't know anything about cultures because it's a low-level API that is only concerned with file paths, not assembly names. You should never have the same path name for two different satellite assemblies.
ghost
locked as resolved and limited conversation to collaborators
5 participants
This adds a satellite assemblies bundle for WASM and also adds a hook into the driver to be able to register satellite assemblies into the bundle before the runtime is initialized. This allows us to load a satellite assembly by name from the bundle for bundled runtimes.
Fixes: #39379
cc: @steveisok @marek-safar @lewing