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

[wasm] Include data archives and timezone data by default #39586

Merged
merged 3 commits into from
Jul 20, 2020

Conversation

lewing
Copy link
Member

@lewing lewing commented Jul 18, 2020

No description provided.

@lewing lewing requested a review from marek-safar as a code owner July 18, 2020 04:08
@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.

@lewing lewing changed the title Clean [wasm] Include data archives and timezone data by default Jul 18, 2020
@lewing lewing added the arch-wasm WebAssembly architecture label Jul 18, 2020
@lewing lewing added this to the 5.0.0 milestone Jul 18, 2020
@lewing lewing requested review from tqiu8 and kg July 18, 2020 04:25
bytes, true /* canRead */, true /* canWrite */, true /* canOwn */
);

if (!this.mono_wasm_load_data_archive (bytes, parentDirectory)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there's a risk of getting mystery breakage here if a file happens to begin with the magic number, but it's probably fine for now.

Copy link
Member Author

@lewing lewing Jul 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree in principle and and I'm fine with adding a "behavior" but I think if should be a content-type instead of of a behavior and the loading function should be extensible off the type with the pre defined types having default handlers. I think this is a good step towards that goal.

Copy link
Member Author

@lewing lewing Jul 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add more than sufficient uniqueness we could verify that the manifest parses as json and that
the length of the manifest is less that then length of the data*

  • the ability to add another archive at the excess of the previous archive has proven valuable in the past

@lewing
Copy link
Member Author

lewing commented Jul 18, 2020

Looks like this is breaking some empty zip file tests.

@ghost
Copy link

ghost commented Jul 18, 2020

Tagging subscribers to this area: @tarekgh, @safern, @krwq
Notify danmosemsft if you want to be subscribed.

@lewing lewing merged commit 945beee into dotnet:master Jul 20, 2020
@lewing lewing deleted the clean branch July 20, 2020 16:40
lewing added a commit to lewing/runtime that referenced this pull request Jul 21, 2020
* Add data archive loading to the generic loading logic
lewing added a commit that referenced this pull request Jul 22, 2020
* ICU integration and asset loading overhaul (#37971)

This PR overhauls runtime startup/asset loading and adds support for ICU integration.

The mono-config.js format is reworked and simplified, with new functionality added:

    Individual assets can be loaded from one or more remote sources with configurable fallback behavior
    In addition to the existing support for loading assemblies, you can now pre-load arbitrary files into the native heap or into emscripten's virtual file system. VFS support previously only existed in runtime-test.js but now is available to any consumer of dotnet.js.
    Assets can have a virtual path set so that their application-facing path does not necessarily have to match their path on the server.
    One or more ICU data archives can be added to the assets list and will be automatically loaded and used to enable ICU-based globalization support.
    Many configuration knobs that previously required API calls can now be set declaratively in the configuration file (environment variables, etc.)

WasmAppBuilder is updated to add ICUDataFiles and RemoteSources parameters that can be used to add the associated information to the config file declaratively from a msbuild project.

Various adjustments are made to existing tests and test cases so that they will pass with the addition of ICU integration.

Co-authored-by: EgorBo <egorbo@gmail.com>
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
Co-authored-by: Larry Ewing <lewing@microsoft.com>

* [wasm] Include data archives and timezone data by default (#39586)

* Add data archive loading to the generic loading logic

* [mono] Update ICU version, disable some tests for Browser (#39596)

Co-authored-by: Katelyn Gadd <kg@luminance.org>
Co-authored-by: EgorBo <egorbo@gmail.com>
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
Jacksondr5 pushed a commit to Jacksondr5/runtime that referenced this pull request Aug 10, 2020
* Add data archive loading to the generic loading logic
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants