Page MenuHomePhabricator

[shell] script to generate WASM database code
ClosedPublic

Authored by kamil on Jun 30 2023, 7:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 1, 2:24 AM
Unknown Object (File)
Sun, Sep 29, 12:09 PM
Unknown Object (File)
Thu, Sep 26, 5:23 AM
Unknown Object (File)
Mon, Sep 16, 2:02 AM
Unknown Object (File)
Sun, Sep 15, 8:02 PM
Unknown Object (File)
Sun, Sep 15, 4:48 PM
Unknown Object (File)
Sun, Sep 8, 10:47 PM
Unknown Object (File)
Sep 3 2024, 8:10 PM
Subscribers

Details

Summary

Script downloads SQLite code (if it does not exist) and generates two files:

  • CommQueryExecutor.js which is glue code in ES6 format, will be imported to JS code
  • CommQueryExecutor.wasm which is actual implementation
Test Plan
  • run script and check if files are created properly
  • run second time and check if SQLite assets are not re-downloaded

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
web/scripts/run_emscripten.sh
98 ↗(On Diff #28299)

each file responsible for adding @generated will also be ignored by phabricator, not sure what will be the best way to fix this

web/scripts/run_emscripten.sh
21–23 ↗(On Diff #28299)

version we are using on the web right now but we can consider updating to a newer

25–33 ↗(On Diff #28299)

inspired by tool we currently using on web (src)

58–59 ↗(On Diff #28299)

allows using memory managment

60–62 ↗(On Diff #28299)

we will need to read virtual file with database content to persist, this allows using C++ file system from JS level

65–70 ↗(On Diff #28299)

this allows importing .js file as regular lib and use with our config

kamil published this revision for review.Jun 30 2023, 8:37 AM
jon requested changes to this revision.Jun 30 2023, 4:33 PM
jon added inline comments.
web/scripts/run_emscripten.sh
10–13 ↗(On Diff #28299)

Please resolve the current directory, so you can call this from another directory other than just CWD

78–83 ↗(On Diff #28299)

-I is an include path. -L is for a library path. However, you seem to be using it as CFLAGS below

98 ↗(On Diff #28299)

to make this BSD/MacOS friendly

This revision now requires changes to proceed.Jun 30 2023, 4:33 PM
web/scripts/run_emscripten.sh
11 ↗(On Diff #28299)

Should this be moved out (eg. to shared/) now that it's no longer native-specific? Fine to handle this later, but feel share a link to a Linear task before landing!

12 ↗(On Diff #28299)

Given sqlite has a separate license, we should make sure to store it in a file named third-party, and make sure that the LICENSE file is included in that folder

An example is the native/cpp/third-party folder

web/scripts/run_emscripten.sh
103 ↗(On Diff #28412)
10–13 ↗(On Diff #28299)

right, my bad - thanks for suggestion

11 ↗(On Diff #28299)

Yeah, good call: ENG-4311

12 ↗(On Diff #28299)

Note from 1:1 meeting with @ashoat: SQLite is in the public domain and we not have to include LICENSE

98 ↗(On Diff #28299)

in that case, should I add a line to remove .js=.bak file?

Bash LGTM

web/scripts/run_emscripten.sh
29 ↗(On Diff #28412)

nit: whitespace alignment

This revision is now accepted and ready to land.Jul 6 2023, 3:40 PM
jon requested changes to this revision.Jul 7 2023, 11:56 AM
jon added inline comments.
web/scripts/run_emscripten.sh
101 ↗(On Diff #28412)

Sorry, i forgot this doesn't follow normal posix conventions. It will take everything immediately after -i

This revision now requires changes to proceed.Jul 7 2023, 11:56 AM
web/scripts/run_emscripten.sh
101 ↗(On Diff #28582)

I am not sure if I follow your idea here but file comm-query-executor.js.bak is still generated, should I add a line in the script to remove it or add it to .gitignore?

jon added inline comments.
web/scripts/run_emscripten.sh
101 ↗(On Diff #28582)

Unfortunately yes, I think ignoring a file is better than failing whether or not people do nix develop before running this file.

This revision is now accepted and ready to land.Jul 11 2023, 9:59 AM