Add SQLite port (sqlite3) #17297

tiran · 2022-06-22T13:23:00Z

The SQLite port adds -sUSE_SQLITE3 to Emscripten. Internally it uses
sqlite amalagation (single file distribution). The ports creates
libsqlite3.a (non-threaded) or libsqlite3-mt.a (multi-threaded).

SQLite is used by CPython and Pyodide. A port in Emscripten would make
our lives a bit easier and could be useful for other projects, too.

tiran · 2022-06-22T13:23:54Z

TODO

tests
documentation
naming (USE_SQLITE3 or USE_SQLITE?)
make some sqlite features optional?

CC @hoodmane @rth

hoodmane · 2022-06-22T13:54:57Z

It would be nice to merge my libffi-emscripten port into mainline libffi and add the libffi port to Emscripten too.

kripken

This makes sense to me. @sbc100 I know you're been thinking about a more pluggable port system, but I think adding this for now makes sense. In particular we could simplify some stuff while doing so, like remove our copy of sqlite in the test dir and switch test_sqlite to use the port instead? And sqlite is a very popular library.

kripken · 2022-06-22T19:36:00Z

tools/ports/sqlite3.py

+
+def get(ports, settings, shared):
+  # TODO: This is an emscripten-hosted mirror of the sqlite repo from sqlite.prg.
+  ports.fetch_project('sqlite3', 'https://www.sqlite.org/2022/sqlite-amalgamation-' + TAG + '.zip', 'sqlite-amalgamation-' + TAG, sha512hash=HASH)


We will be downloading this on every CI build, etc. - is that website the recommended place to do that? (Most other ports tend to download from github, but I don't know if that's better...)

Good point! So far I have not been able to locate a more suitable mirror for the release. The SQLite amalgamation release is a convenient single file bundle of SQlite (one C file, two header files). It would take more effort to build SQLite from the github source mirror of SQlite. Could Emscripten provide its own mirror, e.g. on emscripten-ports?

Given how easy it is to integrate sqlite (especially the amalgamation version) into a project.. how much value is there in emscripten adding this port? Can't folks who want to just use it? I suppose the configuration options are non-trivial?

Yes, the configuration options are non-trivial. For example SQLITE_OMIT_POPEN is easy to miss. The build matrix with standard/pic/lto/lto+pic combined with single-threaded/pthreads adds to the complexity, too.

It would take me more time to document everything than to write the rule file.

tools/ports/sqlite3.py

sbc100

My long plan for make it easier to add ports in a contrib directory has not yet happend. Given how simple this one is I'd be ok with landing it and moving it to the contrib directory later.

tiran · 2022-06-23T15:16:15Z

Do you have a ticket for ports contrib directory feature? I would be interested to assist and contribute.

tools/ports/sqlite3.py

The SQLite port adds `-sUSE_SQLITE3` to Emscripten. Internally it uses sqlite amalagation (single file distribution). The ports creates `libsqlite3.a` (non-threaded) or `libsqlite3-mt.a` (multi-threaded). SQLite is used by CPython and Pyodide. A port in Emscripten would make our lives a bit easier and could be useful for other projects, too.

- drop unnecessary function - use version instead of tag name - example version and version year

tiran · 2022-06-29T13:41:30Z

I have addressed your latest review comments, updated the port to latest sqlite 3.39.0, ported test_sqlite to the port, and rebased the branch on latest main.

sbc100 · 2022-06-29T14:06:50Z

tests/test_core.py

-       #define SQLITE_INT64_TYPE long long int
-       #define SQLITE_THREADSAFE 0
-    '''
-    src += read_file(test_file('third_party/sqlite/sqlite3.c'))


I guess we can completely remove the tests/third_party/sqlite/ directory now?

It is still used by test_benchmark test_zzz_sqlite.

In that case we could move it under tests/benchmark/.

(Perhaps we could also make the benchmark code us the port.)

I think it belongs in tests/third_party where it is, since it is third party test code.

tests/test_core.py

sbc100 · 2022-06-29T14:10:04Z

tests/test_core.py

-    src += read_file(test_file('third_party/sqlite/sqlite3.c'))
-    src += read_file(test_file('sqlite/benchmark.c'))
+    self.emcc_args += ['-sUSE_SQLITE3']
+    src = read_file(test_file('sqlite/benchmark.c'))
    self.do_run(src,


This can be changed to self.do_run_form_file(test_file('sqlite/benchmark.c'), test_file('sqlite/benchmark.txt'), force_c=True) which avoids reading those files into python.

sbc100 · 2022-06-29T14:11:38Z

tests/test_core.py

+  def test_sqlite(self, use_pthreads):
+    if use_pthreads:
+      self.set_setting('USE_PTHREADS')
+      self.setup_node_pthreads()
    self.set_setting('EXPORTED_FUNCTIONS', ['_main', '_sqlite3_open', '_sqlite3_close', '_sqlite3_exec', '_sqlite3_free'])
    if '-g' in self.emcc_args:
      print("disabling inlining") # without registerize (which -g disables), we generate huge amounts of code
      self.set_setting('INLINING_LIMIT')


I wonder if this block is still needed? Can you try core2g.test_sqlite without this .. if it passed in a reasonable amount of time I think we should remove this block... since it no longer effects the compilation of sqlite itself.. only the tiny benchmark code.

Is this a reasonable amount of time?

Using 2 parallel test processes test_sqlite_single (test_core.core0) ... ok (1.23s) test_sqlite_pthreads (test_core.core0) ... ok (1.39s) DONE: combining results on main thread test_sqlite_pthreads (test_core.core0) ... ok test_sqlite_single (test_core.core0) ... ok ---------------------------------------------------------------------- Ran 2 tests in 1.602s

tiran · 2022-06-29T14:18:44Z

One test case is failing:

  File "/root/project/tools/cache.py", line 154, in get
    raise Exception(f'FROZEN_CACHE is set, but cache file is missing: "{shortname}" (in cache root path "{self.dirname}")')
Exception: FROZEN_CACHE is set, but cache file is missing: "sysroot/lib/wasm32-emscripten/libsqlite3-mt.a" (in cache root path "/root/cache")

It looks like I need to include the multithreading variant in PORT_VARIANTS.

sbc100 · 2022-06-29T15:09:31Z

tests/test_core.py

@@ -6665,37 +6665,23 @@ def test_freetype(self):
  @no_asan('local count too large for VMs')
  @no_ubsan('local count too large for VMs')
  @is_slow_test
-  def test_sqlite(self):
+  @parameterized({
+    'single': (False,),


Can you just use the empty string here for the default case.

Also, it looks like you will need to add the two new libraries to MINIMAL_TASKS in embuilder.py.

kripken · 2022-06-29T17:30:19Z

It would be good to add this to the changlog.

tiran mentioned this pull request Jun 22, 2022

gh-84461: Fix test_sqlite for Emscripten/WASI python/cpython#94125

Merged

kripken reviewed Jun 22, 2022

View reviewed changes

sbc100 reviewed Jun 23, 2022

View reviewed changes

tools/ports/sqlite3.py Outdated Show resolved Hide resolved

sbc100 reviewed Jun 23, 2022

View reviewed changes

tools/ports/sqlite3.py Outdated Show resolved Hide resolved

sbc100 reviewed Jun 23, 2022

View reviewed changes

sbc100 approved these changes Jun 29, 2022

View reviewed changes

tools/ports/sqlite3.py Outdated Show resolved Hide resolved

tools/ports/sqlite3.py Outdated Show resolved Hide resolved

tiran added 4 commits June 29, 2022 15:23

Address review

e47669d

- drop unnecessary function - use version instead of tag name - example version and version year

update to 3.39, address review

dbe4506

Use sqlite3 port in test_sqlite

f51d380

tiran force-pushed the sqlite3_ports branch from 821c18a to f51d380 Compare June 29, 2022 13:38

tiran marked this pull request as ready for review June 29, 2022 13:50

sbc100 approved these changes Jun 29, 2022

View reviewed changes

register port variant

04b9bf8

simplify test case even further

27d7337

sbc100 approved these changes Jun 29, 2022

View reviewed changes

sbc100 reviewed Jun 29, 2022

View reviewed changes

tiran added 2 commits June 29, 2022 17:14

drop suffix for single thread test case

926b15a

build sqlite3 for CI

f1f330a

sbc100 approved these changes Jun 29, 2022

View reviewed changes

sbc100 enabled auto-merge (squash) June 29, 2022 16:18

sbc100 merged commit 1056be2 into emscripten-core:main Jun 29, 2022

tiran deleted the sqlite3_ports branch June 29, 2022 17:27

ryanking13 mentioned this pull request Jun 30, 2022

Move sqlite3 and libffi to emscripten ports pyodide/pyodide#2806

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SQLite port (sqlite3) #17297

Add SQLite port (sqlite3) #17297

tiran commented Jun 22, 2022

tiran commented Jun 22, 2022 •

edited

Loading

hoodmane commented Jun 22, 2022

kripken left a comment

kripken Jun 22, 2022

tiran Jun 23, 2022

sbc100 Jun 23, 2022

tiran Jun 23, 2022

sbc100 left a comment

tiran commented Jun 23, 2022

tiran commented Jun 29, 2022

sbc100 Jun 29, 2022

tiran Jun 29, 2022

kripken Jun 29, 2022

sbc100 Jun 29, 2022

sbc100 Jun 29, 2022

sbc100 Jun 29, 2022

tiran Jun 29, 2022

tiran commented Jun 29, 2022

sbc100 Jun 29, 2022

kripken commented Jun 29, 2022

Add SQLite port (sqlite3) #17297

Add SQLite port (sqlite3) #17297

Conversation

tiran commented Jun 22, 2022

tiran commented Jun 22, 2022 • edited Loading

hoodmane commented Jun 22, 2022

kripken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbc100 left a comment

Choose a reason for hiding this comment

tiran commented Jun 23, 2022

tiran commented Jun 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiran commented Jun 29, 2022

Choose a reason for hiding this comment

kripken commented Jun 29, 2022

tiran commented Jun 22, 2022 •

edited

Loading