Skip to content

enhance make #37097

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.

Sign up for GitHub

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

Draft
bircni wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −2
Draft

enhance make #37097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 6 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,12 @@ lint-json-fix: node_modules ## lint and fix json files
pnpm exec eslint -c eslint.json.config.ts --color --max-warnings=0 --concurrency $(ESLINT_CONCURRENCY) --fix

.PHONY: watch
watch: ## watch everything and continuously rebuild
@bash tools/watch.sh
watch: ## watch everything (optional: WATCH_DB=sqlite|mysql|pgsql|mssql WATCH_ACT=true)
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang Apr 3, 2026

Choose a reason for hiding this comment

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

  1. there is no other "DB" that needs to "watch".
  2. there is no "ACT" at the moment.

Off-topic: maybe it should really introduce this: WIP: Pure Go SQLite3 #32628 , then no build tag is needed anymore.

Copy link
Copy Markdown
Contributor

@TheFox0x7 TheFox0x7 Apr 3, 2026

Choose a reason for hiding this comment

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

afaik you were sort of against it - #20614 (comment)
I wouldn't mind having sqlite built-in without tags though.

Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang Apr 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The wasm one isn't like that CCGO based "pure Go" solution. The wasm one is 100% (at least >99%) complied from official SQLite C code, so I would consider it as safe and stable enough for production.

I was just against CCGO at that time 😄

To explain more for WASM ecosystem: mature compiler, mature VM. CCGO is cool and awesome, wasm is the future (IMHO)

Copy link
Copy Markdown
Contributor

@TheFox0x7 TheFox0x7 Apr 3, 2026

Choose a reason for hiding this comment

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

My bad! I assumed this was the same solution (modernrc) and did not look at what it's imported in your PR (ncruces).
seems decent. At least better for us since we don't have to support sqlite tags.

Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang Apr 3, 2026

Choose a reason for hiding this comment

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

Update: it seems that ncruces changed its solution.

  • Old ncruces: SQLite C -> WASM compiler -> WASM code -> WASM VM
  • New ncruces: SQLite C -> WASM compiler -> WASM code -> ncruces wasm2go -> Go code
  • modernrc: SQLite C -> modernrc CCGO compiler -> Go code

Still, modernrc CCGO is much more complicated than ncruces wasm2go 🤣

Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang Apr 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

cznic (native)

we call it "modernc", its package is modernc.org/sqlite

Copy link
Copy Markdown
Member

@silverwind silverwind Apr 3, 2026

Choose a reason for hiding this comment

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

modernc.org/sqlite

Right, the module path still is that: https://gitlab.com/cznic/sqlite/-/blob/master/go.mod#L1

Copy link
Copy Markdown
Contributor

@TheFox0x7 TheFox0x7 Apr 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

ncruces seems slower to compile from scratch. Not a big issue though. It requires one less workaround but the changes in how parameters are passed seem the same.

Size diff (same base, only changed enough to get it compiling and browsable):
+6MB on ncruces
+1.1MB on modernrc

Respective branches:
https://github.com/TheFox0x7/gitea/tree/modernrc-sqlite
https://github.com/TheFox0x7/gitea/tree/ncurces-sqlite

Let me know if you have something I should try or which one to start as draft

Copy link
Copy Markdown
Member

@silverwind silverwind Apr 3, 2026

Choose a reason for hiding this comment

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

Some benchmarks at https://github.com/cvilsmeier/go-sqlite-bench, modernc seems slightly faster on average.

Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang Apr 4, 2026

Choose a reason for hiding this comment

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

modernc +1

@WATCH_DB="$(WATCH_DB)" WATCH_ACT="$(WATCH_ACT)" bash tools/watch.sh

.PHONY: watch-full
watch-full: ## watch with WATCH_DB=sqlite preset (pass WATCH_ACT=true when supported)
@$(MAKE) watch WATCH_DB=sqlite

.PHONY: watch-frontend
watch-frontend: node_modules ## start vite dev server for frontend
Expand Down
24 changes: 24 additions & 0 deletions tools/watch.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,30 @@
#!/bin/bash
set -euo pipefail

# WATCH_DB selects build tags for the backend (see Makefile TAGS). sqlite sets
# TAGS for CGO sqlite support. Other drivers use the default binary without
# sqlite tags; caller-supplied TAGS (pam, bindata, etc.) are preserved. When
# WATCH_DB is unset, TAGS is left unchanged.
# WATCH_ACT=true is reserved for starting act_runner alongside watch (not yet implemented).
if [[ -n "${WATCH_DB:-}" ]]; then
case "${WATCH_DB}" in
sqlite)
export TAGS="sqlite sqlite_unlock_notify"
;;
mysql | pgsql | postgres | mssql)
# Do not touch TAGS so pam/bindata/other local tags from the caller are kept.
;;
*)
echo "watch.sh: unknown WATCH_DB=${WATCH_DB} (use sqlite, mysql, pgsql, postgres, or mssql)" >&2
exit 1
;;
esac
fi

if [[ "${WATCH_ACT:-}" == "true" ]]; then
echo "watch.sh: WATCH_ACT=true is not implemented yet (act_runner integration pending); ignoring." >&2
fi

make --no-print-directory watch-frontend &
make --no-print-directory watch-backend &

Expand Down