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
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/testing/tests.mobile.targets
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

<PropertyGroup Condition="'$(TargetOS)' == 'Browser'">
<!-- We need to set this in order to get extensibility on xunit category traits and other arguments we pass down to xunit via MSBuild properties -->
<RunScriptCommand>$HARNESS_RUNNER wasm test --engine=$(JSEngine) $(JSEngineArgs) --js-file=runtime.js -v --output-directory=$XHARNESS_OUT -- --enable-zoneinfo --run WasmTestRunner.dll $(AssemblyName).dll</RunScriptCommand>
<RunScriptCommand>$HARNESS_RUNNER wasm test --engine=$(JSEngine) $(JSEngineArgs) --js-file=runtime.js -v --output-directory=$XHARNESS_OUT -- --run WasmTestRunner.dll $(AssemblyName).dll</RunScriptCommand>
</PropertyGroup>

<!-- Generate a self-contained app bundle for Android with tests. -->
Expand Down
23 changes: 0 additions & 23 deletions src/mono/wasm/runtime-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,6 @@ while (true) {
} else if (args [0] == "--disable-on-demand-gc") {
enable_gc = false;
args = args.slice (1);
} else if (args [0] == "--enable-zoneinfo") {
enable_zoneinfo = true;
args = args.slice (1);
} else {
break;
}
Expand Down Expand Up @@ -173,26 +170,6 @@ var Module = {
if (!enable_gc) {
Module.ccall ('mono_wasm_enable_on_demand_gc', 'void', ['number'], [0]);
}
if (enable_zoneinfo) {
// The timezone file is generated by https://github.com/dotnet/blazor/tree/master/src/TimeZoneData.
// The file format of the TZ file look like so
//
// [4-byte magic number]
// [4 - byte length of manifest]
// [json manifest]
// [data bytes]
//
// The json manifest is an array that looks like so:
//
// [...["America/Fort_Nelson",2249],["America/Glace_Bay",2206]..]
//
// where the first token in each array is the relative path of the file on disk, and the second is the
// length of the file. The starting offset of a file can be calculated using the lengths of all files
// that appear prior to it.
var zonedata = new Uint8Array(read ("dotnet.timezones.blat", "binary"));
MONO.mono_wasm_load_data (zonedata, "/usr/share/zoneinfo");
}


config.loaded_cb = function () {
App.init ();
Expand Down
34 changes: 15 additions & 19 deletions src/mono/wasm/runtime/library_mono.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,6 @@ var MonoSupportLib = {
: virtualName;
if (fileName.startsWith("/"))
fileName = fileName.substr(1);

if (parentDirectory) {
if (ctx.tracing)
console.log ("MONO_WASM: Creating directory '" + parentDirectory + "'");
Expand All @@ -634,11 +633,12 @@ var MonoSupportLib = {
if (ctx.tracing)
console.log ("MONO_WASM: Creating file '" + fileName + "' in directory '" + parentDirectory + "'");

var fileRet = ctx.createDataFile (
parentDirectory, fileName,
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

var fileRet = ctx.createDataFile (
parentDirectory, fileName,
bytes, true /* canRead */, true /* canWrite */, true /* canOwn */
);
}
break;

default:
Expand Down Expand Up @@ -1101,12 +1101,12 @@ var MonoSupportLib = {
return className.replace(/\//g, '.').replace(/`\d+/g, '');
},

mono_wasm_load_data: function (data, prefix) {
mono_wasm_load_data_archive: function (data, prefix) {
var dataview = new DataView(data.buffer);
var magic = dataview.getUint32(0, true);
// get magic number
if (magic != 0x626c6174) {
throw new Error ("File is of wrong type");
return false;
}
var manifestSize = dataview.getUint32(4, true);
var manifestContent = Module.UTF8ArrayToString(data, 8, manifestSize);
Expand All @@ -1118,18 +1118,13 @@ var MonoSupportLib = {
// /usr/share/zoneinfo/Africa
// /usr/share/zoneinfo/Asia
// ..
var p = prefix.slice(1).split('/');
p.forEach((v, i) => {
FS.mkdir(v);
Module['FS_createPath']("/" + p.slice(0, i).join('/'), v, true, true);
})

var folders = new Set()
manifest.filter(m => {
m = m[0].split('/')
if (m!= null) {
if (m.length > 2) folders.add(m.slice(0,m.length-1).join('/'));
folders.add(m[0]);
}
var file = m[0];
var last = file.lastIndexOf ("/");
var directory = file.slice (0, last);
folders.add(directory);
});
folders.forEach(folder => {
Module['FS_createPath'](prefix, folder, true, true);
Expand All @@ -1139,9 +1134,10 @@ var MonoSupportLib = {
var name = row[0];
var length = row[1];
var bytes = data.slice(0, length);
Module['FS_createDataFile'](`${prefix}/${name}`, null, bytes, true, true);
Module['FS_createDataFile'](prefix, name, bytes, true, true);
data = data.slice(length);
}
return true;
}
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public override bool Execute ()
var sEnableRemote = enableRemote ? "true" : "false";

sw.WriteLine($"\t\t{{ behavior: \"icu\", name: \"icudt.dat\", load_remote: {sEnableRemote} }},");

sw.WriteLine($"\t\t{{ behavior: \"vfs\", name: \"dotnet.timezones.blat\", virtual_path: \"/usr/share/zoneinfo/\" }}");
sw.WriteLine ("\t],");

if (enableRemote) {
Expand Down